New page local store buffer.

New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

Design doc: https://goo.gl/sMKCf7

BUG=chromium:578883
LOG=NO

Committed: https://crrev.com/bb883395a814d10cee927eabe689a8070a40a86a
Cr-Commit-Position: refs/heads/master@{#33806}

[ulan, 2016-01-18 18:58:52]

Description was changed from

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

BUG=
==========

to

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

BUG=chromium:578883
==========

[ulan, 2016-01-18 18:59:06]

Description was changed from

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

BUG=chromium:578883
==========

to

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

BUG=chromium:578883
LOG=NO
==========

[ulan, 2016-01-18 19:02:30]

Description was changed from

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

BUG=chromium:578883
LOG=NO
==========

to

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

Design doc: go/rfyqw (todo: cleanup and make world visible).

BUG=chromium:578883
LOG=NO
==========

[ulan, 2016-01-18 19:13:31]

ulan@chromium.org changed reviewers:
+ hpayer@chromium.org, mlippautz@chromium.org

[ulan, 2016-01-18 19:13:32]

PTAL

[Hannes Payer (out of office), 2016-01-20 19:43:01]

This is awesome! First round of comments.

https://codereview.chromium.org/1608583002/diff/20001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1608583002/diff/20001/BUILD.gn#newcode1069
BUILD.gn:1069: "src/heap/slot-set.h",
alphabetic order

https://codereview.chromium.org/1608583002/diff/20001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/heap.cc#newcod...
src/heap/heap.cc:6086: void
Heap::FilterStoreBufferEntriesOnAboutToBeFreedPages() {
This method should not be needed anymore because:

https://codereview.chromium.org/1608583002/diff/20001/src/heap/heap.cc#newcod...
src/heap/heap.cc:6092: chunk->ReleaseOldToNewSlots();
They should be released in MemoryChunk::ReleaseAllocatedMemory() which you
already do.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/heap.cc#newcod...
src/heap/heap.cc:6093: chunk->SetFlag(MemoryChunk::ABOUT_TO_BE_FREED);
ABOUT_TO_BE_FREED should not be needed anymore.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/incremental-ma...
File src/heap/incremental-marking.cc (left):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/incremental-ma...
src/heap/incremental-marking.cc:369:
chunk->SetFlag(MemoryChunk::SCAN_ON_SCAVENGE);
Nice!

https://codereview.chromium.org/1608583002/diff/20001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:2885: // or in map spaces.
Why map space?

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h
File src/heap/slot-set.h (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:14: // Data structure for maintaining a set of slots in a
standard (non-large)
Why don't we also support large pages right away, i.e. allow a slot set of
arbitrary size? The constructor could take care of that.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:36: void SetPageStart(Address page_start) { page_start_ =
page_start; }
If we allow arbitrary size slot sets, we would not need that method. As far as I
see, it is just needed to set up the large object case.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:75: uint32_t end_mask = (1u << end_bit) - 1;
~(1u << end_bit) - 1) will give you the right end_mask right away.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:163: uint32_t* result = new uint32_t[kCellsPerBucket];
This is a nice way to safe memory, but it may be potentially bad for cache
locality. A similar approach would be to have all the cells virtually allocated
but unmap unused physical cells. That would require kCellsPerBucket = 128 to
unmap physical 4K pages.
WDYT? Let's discuss this offline.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:187: int* bit_index) {
Can we DCHECK that slot_offset is within the page?

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.cc
File src/heap/store-buffer.cc (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.c...
src/heap/store-buffer.cc:120:
heap_->isolate()->counters()->store_buffer_compactions()->Increment();
It is not really "compactions" anymore. Maybe overflows? Or Evacuations?

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.c...
src/heap/store-buffer.cc:179: chunk->ReleaseOldToNewSlots();
Why are we doing that here? We should not process slots of white large objects.

[Michael Lippautz, 2016-01-21 10:07:32]

Good bye SCAN_ON_SCAVENGE, you will not be missed ;)

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h
File src/heap/slot-set.h (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:20: class SlotSet {
Make it inherit from public Malloced, so that we get proper V8 OOMs when
allocation using _new_ fails.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:68: void RemoveRange(int start_offset, int end_offset) {
Amazing function :)

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:75: uint32_t end_mask = (1u << end_bit) - 1;
On 2016/01/20 19:43:00, Hannes Payer wrote:
> ~(1u << end_bit) - 1) will give you the right end_mask right away.

Probably doesn't matter too much, but you could also move start_mask and
end_mask into the branch.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:115: enum CallbackResult { KEEP_SLOT, REMOVE_SLOT };
nit: Move to top

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:163: uint32_t* result = new uint32_t[kCellsPerBucket];
nit: If you use NewArray<uint32_t>(kCellsPerBucket) you get the benefit of
checking the memory against null. I guess the performance impact should be
negligible.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:179: if (cells != nullptr && cells[cell_index] != 0) {
AFAIC this could just be DCHECKs, e.g.,
  DCHECK_NOT_NULL(cells);
  DCHECK_NE(cells[cell_index], 0);

(I would do the masking unconditionally of the value of cells[cell_index])

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:188: DCHECK(slot_offset % kPointerSize == 0);
nit: DCHECK_EQ

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:196: static const int kMaxSlots = (1 << kPageSizeBits) /
kPointerSize;
nit: Move right below the "private:" section.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.cc
File src/heap/store-buffer.cc (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.c...
src/heap/store-buffer.cc:20: : heap_(heap), start_(NULL), limit_(NULL),
virtual_memory_(NULL) {}
nit: nullptr

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.h
File src/heap/store-buffer.h (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.h...
src/heap/store-buffer.h:57: // The store buffer is divided up into a new buffer
that is constantly being
nit: This comment is now out of date.

[Michael Lippautz, 2016-01-22 08:03:49]

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer-i...
File src/heap/store-buffer-inl.h (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer-i...
src/heap/store-buffer-inl.h:15: uint32_t
StoreBuffer::SlotAddressToOffset(Address addr, SlotSet** slots) {
Maybe rename to AddressToSlotSetAndOffset, as it's not only computing the offset
but also handling the slot set(s).

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer-i...
src/heap/store-buffer-inl.h:18: if (owner == NULL) {
nullptr

https://codereview.chromium.org/1608583002/diff/20001/tools/gyp/v8.gyp
File tools/gyp/v8.gyp (right):

https://codereview.chromium.org/1608583002/diff/20001/tools/gyp/v8.gyp#newcod...
tools/gyp/v8.gyp:839: '../../src/heap/slot-set.h',
Order

[Michael Lippautz, 2016-01-26 09:12:24]

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h
File src/heap/slot-set.h (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:46: bucket[bucket_index][cell_index] |= 1u << bit_index;
Maybe we can additionally have a method {InsertSynchronized} that can be used
for parallel insertion? If we divide up memory upon compaction tasks, they may
end up using non-private pages, for which we would still require a local buffer.

The method would only need to handle concurrency with competing calls of
InsertSynchronized.

E.g. (omitting all necessary casting):

void InsertSynchronized(int slot_offset) {
  int bucket_index, cell_index, bit_index;
  SlotToIndices(slot_offset, &bucket_index, &cell_index, &bit_index);
  
  AtomicWord bucket_ptr = base::Acquire_Load(&bucket[bucket_index]);
  if (bucket_ptr == nullptr) {
    uint32_t* new_bucket = AllocateBucket();
    if (!base::Release_CompareAndSwap(bucket_ptr, nullptr, new_bucket)) {
      delete new_bucket;
      bucket_ptr = base::Acquire_Load(&bucket[bucket_index]);
    }
    bucket_ptr = new_bucket;
  }

  AtomicWord old;
  do {
    old = base::Acquire_Load(&bucket_ptr[cell_index]);
  } while (!base::Release_CompareAndSwap(&bucket_ptr[cell_index], old, 1u <<
bit_index));
}

[ulan, 2016-01-28 19:07:23]

Thank you for the comments!

I uploaded new PS.

https://codereview.chromium.org/1608583002/diff/20001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1608583002/diff/20001/BUILD.gn#newcode1069
BUILD.gn:1069: "src/heap/slot-set.h",
On 2016/01/20 19:43:00, Hannes Payer wrote:
> alphabetic order

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/heap.cc#newcod...
src/heap/heap.cc:6086: void
Heap::FilterStoreBufferEntriesOnAboutToBeFreedPages() {
On 2016/01/20 19:43:00, Hannes Payer wrote:
> This method should not be needed anymore because:

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/heap.cc#newcod...
src/heap/heap.cc:6092: chunk->ReleaseOldToNewSlots();
On 2016/01/20 19:43:00, Hannes Payer wrote:
> They should be released in MemoryChunk::ReleaseAllocatedMemory() which you
> already do.

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/heap.cc#newcod...
src/heap/heap.cc:6093: chunk->SetFlag(MemoryChunk::ABOUT_TO_BE_FREED);
On 2016/01/20 19:43:00, Hannes Payer wrote:
> ABOUT_TO_BE_FREED should not be needed anymore.

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:2885: // or in map spaces.
On 2016/01/20 19:43:00, Hannes Payer wrote:
> Why map space?

Clarified the comment.
Since we only filter invalid slots in old space, the store buffer can
still contain stale pointers in large object and in map spaces. Ignore
these pointers here.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h
File src/heap/slot-set.h (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:14: // Data structure for maintaining a set of slots in a
standard (non-large)
On 2016/01/20 19:43:00, Hannes Payer wrote:
> Why don't we also support large pages right away, i.e. allow a slot set of
> arbitrary size? The constructor could take care of that.

The large page takes care of that by using an array of SlotSets.

I decided to not make SlotSet generic so that the common path (normal pages)
stay fast.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:20: class SlotSet {
On 2016/01/21 10:07:32, Michael Lippautz wrote:
> Make it inherit from public Malloced, so that we get proper V8 OOMs when
> allocation using _new_ fails.

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:36: void SetPageStart(Address page_start) { page_start_ =
page_start; }
On 2016/01/20 19:43:00, Hannes Payer wrote:
> If we allow arbitrary size slot sets, we would not need that method. As far as
I
> see, it is just needed to set up the large object case.

It is also used in the Iterate callback to convert slot offset to addresses.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:46: bucket[bucket_index][cell_index] |= 1u << bit_index;
On 2016/01/26 09:12:24, Michael Lippautz wrote:
> Maybe we can additionally have a method {InsertSynchronized} that can be used
> for parallel insertion? If we divide up memory upon compaction tasks, they may
> end up using non-private pages, for which we would still require a local
buffer.
> 
> The method would only need to handle concurrency with competing calls of
> InsertSynchronized.
> 
> E.g. (omitting all necessary casting):
> 
> void InsertSynchronized(int slot_offset) {
>   int bucket_index, cell_index, bit_index;
>   SlotToIndices(slot_offset, &bucket_index, &cell_index, &bit_index);
>   
>   AtomicWord bucket_ptr = base::Acquire_Load(&bucket[bucket_index]);
>   if (bucket_ptr == nullptr) {
>     uint32_t* new_bucket = AllocateBucket();
>     if (!base::Release_CompareAndSwap(bucket_ptr, nullptr, new_bucket)) {
>       delete new_bucket;
>       bucket_ptr = base::Acquire_Load(&bucket[bucket_index]);
>     }
>     bucket_ptr = new_bucket;
>   }
> 
>   AtomicWord old;
>   do {
>     old = base::Acquire_Load(&bucket_ptr[cell_index]);
>   } while (!base::Release_CompareAndSwap(&bucket_ptr[cell_index], old, 1u <<
> bit_index));
> }

I will add this method in a separate CL as this CL is getting large.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:75: uint32_t end_mask = (1u << end_bit) - 1;
On 2016/01/20 19:43:00, Hannes Payer wrote:
> ~(1u << end_bit) - 1) will give you the right end_mask right away.

The masks are used below too.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:75: uint32_t end_mask = (1u << end_bit) - 1;
On 2016/01/20 19:43:00, Hannes Payer wrote:
> ~(1u << end_bit) - 1) will give you the right end_mask right away.

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:115: enum CallbackResult { KEEP_SLOT, REMOVE_SLOT };
On 2016/01/21 10:07:32, Michael Lippautz wrote:
> nit: Move to top

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:163: uint32_t* result = new uint32_t[kCellsPerBucket];
On 2016/01/21 10:07:32, Michael Lippautz wrote:
> nit: If you use NewArray<uint32_t>(kCellsPerBucket) you get the benefit of
> checking the memory against null. I guess the performance impact should be
> negligible.

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:163: uint32_t* result = new uint32_t[kCellsPerBucket];
On 2016/01/20 19:43:00, Hannes Payer wrote:
> This is a nice way to safe memory, but it may be potentially bad for cache
> locality. A similar approach would be to have all the cells virtually
allocated
> but unmap unused physical cells. That would require kCellsPerBucket = 128 to
> unmap physical 4K pages.
> WDYT? Let's discuss this offline.
Ack. Let's discuss offline.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:179: if (cells != nullptr && cells[cell_index] != 0) {
On 2016/01/21 10:07:32, Michael Lippautz wrote:
> AFAIC this could just be DCHECKs, e.g.,
>   DCHECK_NOT_NULL(cells);
>   DCHECK_NE(cells[cell_index], 0);
> 
> (I would do the masking unconditionally of the value of cells[cell_index])

The cells and cell[cell_index] can be 0.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:187: int* bit_index) {
On 2016/01/20 19:43:00, Hannes Payer wrote:
> Can we DCHECK that slot_offset is within the page?

The equivalent DCHECK is performed below for slot.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:188: DCHECK(slot_offset % kPointerSize == 0);
On 2016/01/21 10:07:32, Michael Lippautz wrote:
> nit: DCHECK_EQ

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/slot-set.h#new...
src/heap/slot-set.h:196: static const int kMaxSlots = (1 << kPageSizeBits) /
kPointerSize;
On 2016/01/21 10:07:32, Michael Lippautz wrote:
> nit: Move right below the "private:" section.

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer-i...
File src/heap/store-buffer-inl.h (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer-i...
src/heap/store-buffer-inl.h:15: uint32_t
StoreBuffer::SlotAddressToOffset(Address addr, SlotSet** slots) {
On 2016/01/22 08:03:49, Michael Lippautz wrote:
> Maybe rename to AddressToSlotSetAndOffset, as it's not only computing the
offset
> but also handling the slot set(s).

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer-i...
src/heap/store-buffer-inl.h:18: if (owner == NULL) {
On 2016/01/22 08:03:49, Michael Lippautz wrote:
> nullptr

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.cc
File src/heap/store-buffer.cc (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.c...
src/heap/store-buffer.cc:20: : heap_(heap), start_(NULL), limit_(NULL),
virtual_memory_(NULL) {}
On 2016/01/21 10:07:32, Michael Lippautz wrote:
> nit: nullptr

Done.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.c...
src/heap/store-buffer.cc:120:
heap_->isolate()->counters()->store_buffer_compactions()->Increment();
On 2016/01/20 19:43:00, Hannes Payer wrote:
> It is not really "compactions" anymore. Maybe overflows? Or Evacuations?

Removed the counter completely since it is equivalent to
isolate->counters()->store_buffer_overflows() now.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.c...
src/heap/store-buffer.cc:179: chunk->ReleaseOldToNewSlots();
On 2016/01/20 19:43:00, Hannes Payer wrote:
> Why are we doing that here? We should not process slots of white large
objects.

Good point, removed it.

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.h
File src/heap/store-buffer.h (right):

https://codereview.chromium.org/1608583002/diff/20001/src/heap/store-buffer.h...
src/heap/store-buffer.h:57: // The store buffer is divided up into a new buffer
that is constantly being
On 2016/01/21 10:07:32, Michael Lippautz wrote:
> nit: This comment is now out of date.

Done.

https://codereview.chromium.org/1608583002/diff/20001/tools/gyp/v8.gyp
File tools/gyp/v8.gyp (right):

https://codereview.chromium.org/1608583002/diff/20001/tools/gyp/v8.gyp#newcod...
tools/gyp/v8.gyp:839: '../../src/heap/slot-set.h',
On 2016/01/22 08:03:49, Michael Lippautz wrote:
> Order

Done.

[ulan, 2016-02-01 15:44:11]

Description was changed from

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

Design doc: go/rfyqw (todo: cleanup and make world visible).

BUG=chromium:578883
LOG=NO
==========

to

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

Design doc: https://goo.gl/sMKCf7

BUG=chromium:578883
LOG=NO
==========

[Michael Lippautz, 2016-02-01 17:36:13]

lgtm from my side!

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer.cc
File src/heap/store-buffer.cc (right):

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer....
src/heap/store-buffer.cc:184: if (heap->InNewSpace(object) &&
object->IsHeapObject()) {
nit: As far as I see, Heap::InNewSpace(Object*) compares against
NewSpace::object_mask_, which already includes the check for IsHeapObject
(because it also contains the tag in the mask).

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer.h
File src/heap/store-buffer.h (right):

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer....
src/heap/store-buffer.h:39: void IteratePointersToNewSpace(ObjectSlotCallback
callback);
nit: Maybe add a comment describing that the slot only remains in the store
buffer when callback actually moves the value from from space to to space.

[Hannes Payer (out of office), 2016-02-02 12:40:37]

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer.cc
File src/heap/store-buffer.cc (right):

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer....
src/heap/store-buffer.cc:102: Page* last_page = nullptr;
You are currently not assigning to last_page and last_slot_set.

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer.h
File src/heap/store-buffer.h (right):

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer....
src/heap/store-buffer.h:24: // between spaces.
What is the plan for this class? We should at least re-name it to something more
expressive.

[ulan, 2016-02-02 13:27:28]

Thanks for the comments.

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer.cc
File src/heap/store-buffer.cc (right):

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer....
src/heap/store-buffer.cc:102: Page* last_page = nullptr;
On 2016/02/02 12:40:37, Hannes Payer wrote:
> You are currently not assigning to last_page and last_slot_set.

Done.

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer....
src/heap/store-buffer.cc:184: if (heap->InNewSpace(object) &&
object->IsHeapObject()) {
On 2016/02/01 17:36:13, Michael Lippautz wrote:
> nit: As far as I see, Heap::InNewSpace(Object*) compares against
> NewSpace::object_mask_, which already includes the check for IsHeapObject
> (because it also contains the tag in the mask).

Done.

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer.h
File src/heap/store-buffer.h (right):

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer....
src/heap/store-buffer.h:24: // between spaces.
On 2016/02/02 12:40:37, Hannes Payer wrote:
> What is the plan for this class? We should at least re-name it to something
more
> expressive.

The class will stay, but I agree about naming. Once this CL sticks and I will
move the class to remembered-set.h and rename it to OldToNewRememberedSet (along
with OldToOldRememberedSet that will replace SlotsBuffer).

https://codereview.chromium.org/1608583002/diff/240001/src/heap/store-buffer....
src/heap/store-buffer.h:39: void IteratePointersToNewSpace(ObjectSlotCallback
callback);
On 2016/02/01 17:36:13, Michael Lippautz wrote:
> nit: Maybe add a comment describing that the slot only remains in the store
> buffer when callback actually moves the value from from space to to space.

Done.

[Hannes Payer (out of office), 2016-02-07 15:46:03]

LGTM

[ulan, 2016-02-08 07:45:58]

The CQ bit was checked by ulan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-08 07:46:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1608583002/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1608583002/280001

[commit-bot: I haz the power, 2016-02-08 07:52:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-08 07:52:28]

Dry run: Try jobs failed on following builders:
  v8_win_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/2472)

[ulan, 2016-02-08 08:22:03]

The CQ bit was checked by ulan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-08 08:22:16]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1608583002/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1608583002/300001

[commit-bot: I haz the power, 2016-02-08 08:48:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-08 08:48:45]

Dry run: This issue passed the CQ dry run.

[ulan, 2016-02-08 08:49:20]

The CQ bit was checked by ulan@chromium.org

[ulan, 2016-02-08 08:49:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlippautz@chromium.org, hpayer@chromium.org
Link to the patchset: https://codereview.chromium.org/1608583002/#ps300001
(title: "Rebase and fix signed unsigned conversion")

[commit-bot: I haz the power, 2016-02-08 08:49:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1608583002/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1608583002/300001

[commit-bot: I haz the power, 2016-02-08 08:51:09]

Description was changed from

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

Design doc: https://goo.gl/sMKCf7

BUG=chromium:578883
LOG=NO
==========

to

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

Design doc: https://goo.gl/sMKCf7

BUG=chromium:578883
LOG=NO
==========

[commit-bot: I haz the power, 2016-02-08 08:51:10]

Committed patchset #16 (id:300001)

[commit-bot: I haz the power, 2016-02-08 08:51:48]

Description was changed from

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

Design doc: https://goo.gl/sMKCf7

BUG=chromium:578883
LOG=NO
==========

to

==========
New page local store buffer.

This replaces the global remembered set with per-page remembered sets.

Each page in the old space, map space, and large object space keeps track of
the set of slots in the page pointing to the new space.

The data structure for storing slot sets is a two-level bitmap, which allows
us to remove the store buffer overflow and SCAN_ON_SCAVENGE logic.

Design doc: https://goo.gl/sMKCf7

BUG=chromium:578883
LOG=NO

Committed: https://crrev.com/bb883395a814d10cee927eabe689a8070a40a86a
Cr-Commit-Position: refs/heads/master@{#33806}
==========

[commit-bot: I haz the power, 2016-02-08 08:51:49]

Patchset 16 (id:??) landed as
https://crrev.com/bb883395a814d10cee927eabe689a8070a40a86a
Cr-Commit-Position: refs/heads/master@{#33806}