New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 915233 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

LargeObjectSpace::FindPage is slowing down write barrier

Project Member Reported by u...@chromium.org, Dec 14

Issue description

LargeObjectSpace::FindPage shows up as the hottest function in performance profile of microbenchmarks (discovered by jarin@).

The function is used during store-buffer draining to look up the memory chunk by slot address:
https://cs.chromium.org/chromium/src/v8/src/heap/store-buffer.cc?rcl=886f3c63813ac72c9228a4362d667e0d1fe33213&l=170

We can optimize this by additionally storing the host address in the write barrier.
 
Cc: verwa...@chromium.org
Toon suggested a cool idea to use a fake object pointer in the page header to distinguish actual page header from the slots in the middle of a large object.

The fake object pointer should point to the page start, i.e. it will be page aligned.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 11

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/fd49c8bb43a144fbaa9139ed8fa3e634179d6b89

commit fd49c8bb43a144fbaa9139ed8fa3e634179d6b89
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Fri Jan 11 10:59:49 2019

[heap] Optimize MemoryChunk::FromAnyPointerAddress

Currently this function requires the caller to hold a mutex for the
large page chunk hashtable and performs a hashtable lookup.

This patch adds a header sentinel field in each MemoryChunk. The field
is then used to distinguish large object slots from ordinary slots.

Bug: chromium:915233
Change-Id: I9fbeeb4f07f49573d0a21f9a2cc934370e417d68
Reviewed-on: https://chromium-review.googlesource.com/c/1391752
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58732}
[modify] https://crrev.com/fd49c8bb43a144fbaa9139ed8fa3e634179d6b89/src/heap/heap-inl.h
[modify] https://crrev.com/fd49c8bb43a144fbaa9139ed8fa3e634179d6b89/src/heap/heap.h
[modify] https://crrev.com/fd49c8bb43a144fbaa9139ed8fa3e634179d6b89/src/heap/spaces-inl.h
[modify] https://crrev.com/fd49c8bb43a144fbaa9139ed8fa3e634179d6b89/src/heap/spaces.cc
[modify] https://crrev.com/fd49c8bb43a144fbaa9139ed8fa3e634179d6b89/src/heap/spaces.h
[modify] https://crrev.com/fd49c8bb43a144fbaa9139ed8fa3e634179d6b89/src/heap/store-buffer.cc
[modify] https://crrev.com/fd49c8bb43a144fbaa9139ed8fa3e634179d6b89/tools/v8heapconst.py

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 11

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/9c9682d05ad5895c6ad67546d304c8edaff85bc6

commit 9c9682d05ad5895c6ad67546d304c8edaff85bc6
Author: Michael Achenbach <machenbach@chromium.org>
Date: Fri Jan 11 12:15:09 2019

Revert "[heap] Optimize MemoryChunk::FromAnyPointerAddress"

This reverts commit fd49c8bb43a144fbaa9139ed8fa3e634179d6b89.

Reason for revert: Speculative revert for:
https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/24672

Original change's description:
> [heap] Optimize MemoryChunk::FromAnyPointerAddress
> 
> Currently this function requires the caller to hold a mutex for the
> large page chunk hashtable and performs a hashtable lookup.
> 
> This patch adds a header sentinel field in each MemoryChunk. The field
> is then used to distinguish large object slots from ordinary slots.
> 
> Bug: chromium:915233
> Change-Id: I9fbeeb4f07f49573d0a21f9a2cc934370e417d68
> Reviewed-on: https://chromium-review.googlesource.com/c/1391752
> Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#58732}

TBR=ulan@chromium.org,mlippautz@chromium.org

Change-Id: I232729fdfd55baef7de99ea2fd14fbc0a2f71d27
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:915233
Reviewed-on: https://chromium-review.googlesource.com/c/1406671
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58738}
[modify] https://crrev.com/9c9682d05ad5895c6ad67546d304c8edaff85bc6/src/heap/heap-inl.h
[modify] https://crrev.com/9c9682d05ad5895c6ad67546d304c8edaff85bc6/src/heap/heap.h
[modify] https://crrev.com/9c9682d05ad5895c6ad67546d304c8edaff85bc6/src/heap/spaces-inl.h
[modify] https://crrev.com/9c9682d05ad5895c6ad67546d304c8edaff85bc6/src/heap/spaces.cc
[modify] https://crrev.com/9c9682d05ad5895c6ad67546d304c8edaff85bc6/src/heap/spaces.h
[modify] https://crrev.com/9c9682d05ad5895c6ad67546d304c8edaff85bc6/src/heap/store-buffer.cc
[modify] https://crrev.com/9c9682d05ad5895c6ad67546d304c8edaff85bc6/tools/v8heapconst.py

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 17 (5 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/c0994d3ffbb33a864630cc4d44826e34abfc3686

commit c0994d3ffbb33a864630cc4d44826e34abfc3686
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Thu Jan 17 11:25:59 2019

Reland "[heap] Optimize MemoryChunk::FromAnyPointerAddress"

This is a reland of fd49c8bb43a144fbaa9139ed8fa3e634179d6b89

Original change's description:
> [heap] Optimize MemoryChunk::FromAnyPointerAddress
> 
> Currently this function requires the caller to hold a mutex for the
> large page chunk hashtable and performs a hashtable lookup.
> 
> This patch adds a header sentinel field in each MemoryChunk. The field
> is then used to distinguish large object slots from ordinary slots.
> 
> Bug: chromium:915233
> Change-Id: I9fbeeb4f07f49573d0a21f9a2cc934370e417d68
> Reviewed-on: https://chromium-review.googlesource.com/c/1391752
> Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#58732}

Bug: chromium:915233
Change-Id: I10d23a928328169a2dc6bab78d2b7d2c5d00ebb6
Reviewed-on: https://chromium-review.googlesource.com/c/1406672
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58876}
[modify] https://crrev.com/c0994d3ffbb33a864630cc4d44826e34abfc3686/src/heap/heap-inl.h
[modify] https://crrev.com/c0994d3ffbb33a864630cc4d44826e34abfc3686/src/heap/heap.h
[modify] https://crrev.com/c0994d3ffbb33a864630cc4d44826e34abfc3686/src/heap/spaces-inl.h
[modify] https://crrev.com/c0994d3ffbb33a864630cc4d44826e34abfc3686/src/heap/spaces.cc
[modify] https://crrev.com/c0994d3ffbb33a864630cc4d44826e34abfc3686/src/heap/spaces.h
[modify] https://crrev.com/c0994d3ffbb33a864630cc4d44826e34abfc3686/src/heap/store-buffer.cc
[modify] https://crrev.com/c0994d3ffbb33a864630cc4d44826e34abfc3686/tools/v8heapconst.py

Sign in to add a comment