[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment methods

[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for (real) non-contiguous backing memory, this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced by lookups for object tags and page headers, where
possible.

Details:
- Use the fast checks (page header lookups) for containment in regular code.
- Use the slow version that masks out the page start adress and iterates all
  pages of a space for debugging/verification.
- The slow version works for off-heap/unmapped memory.
- Encapsulate all checks for the old->new barrier in Heap::RecordWrite().

BUG=chromium:581412
LOG=N

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

[Michael Lippautz, 2016-01-26 14:11:18]

Description was changed from

==========
[heap] Prepare SemiSpace for non-contiguous memory

- Remove masks that require semispace to be contigous in memory. Replace them
  with page lookups.

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

to

==========
[heap] Prepare semi space for non-contiguous backing memory

- Remove masks that require semispace to be contigous in memory. Replace them
  with page lookups.

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

[Michael Lippautz, 2016-01-26 14:42:12]

Patchset #1 (id:1) has been deleted

[Michael Lippautz, 2016-01-26 18:55:56]

Description was changed from

==========
[heap] Prepare semi space for non-contiguous backing memory

- Remove masks that require semispace to be contigous in memory. Replace them
  with page lookups.

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

to

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

- Remove masks that require semispace to be contigous in memory. Replace them
  with page lookups.

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

[Michael Lippautz, 2016-01-26 18:58:43]

Description was changed from

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

- Remove masks that require semispace to be contigous in memory. Replace them
  with page lookups.

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

to

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced with a new mechanism that leverages the fact that
heap objects are tagged and have appropriate page headers to look at and falls
back to a slow mechanism for raw addresses.

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

[Michael Lippautz, 2016-01-26 19:03:18]

Description was changed from

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced with a new mechanism that leverages the fact that
heap objects are tagged and have appropriate page headers to look at and falls
back to a slow mechanism for raw addresses.

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

to

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced with a new mechanism that leverages the fact that
heap objects are tagged and have appropriate page headers to look at and falls
back to a slow mechanism for raw addresses.

BUG=chromium: 581412
LOG=N
==========

[Michael Lippautz, 2016-01-26 19:03:27]

Description was changed from

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced with a new mechanism that leverages the fact that
heap objects are tagged and have appropriate page headers to look at and falls
back to a slow mechanism for raw addresses.

BUG=chromium: 581412
LOG=N
==========

to

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced with a new mechanism that leverages the fact that
heap objects are tagged and have appropriate page headers to look at and falls
back to a slow mechanism for raw addresses.

BUG=chromium:581412
LOG=N
==========

[Michael Lippautz, 2016-01-26 19:05:34]

Description was changed from

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced with a new mechanism that leverages the fact that
heap objects are tagged and have appropriate page headers to look at and falls
back to a slow mechanism for raw addresses.

BUG=chromium:581412
LOG=N
==========

to

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced by lookups for object tags and page headers, and
a fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

[Michael Lippautz, 2016-01-26 19:05:58]

Description was changed from

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced by lookups for object tags and page headers, and
a fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

to

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

[Michael Lippautz, 2016-02-05 08:58:40]

Patchset #8 (id:160001) has been deleted

[Michael Lippautz, 2016-02-05 09:53:51]

Patchset #2 (id:40001) has been deleted

[Michael Lippautz, 2016-02-05 09:53:58]

Patchset #3 (id:80001) has been deleted

[Michael Lippautz, 2016-02-05 09:54:05]

Patchset #2 (id:60001) has been deleted

[Michael Lippautz, 2016-02-05 12:27:22]

Description was changed from

==========
[heap] Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

to

==========
[heap] Do page lookups for SemiSpace, NewSpace, and Heap containments

Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

[Michael Lippautz, 2016-02-05 12:27:51]

Description was changed from

==========
[heap] Do page lookups for SemiSpace, NewSpace, and Heap containments

Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

to

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

[Michael Lippautz, 2016-02-05 12:31:04]

Description was changed from

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Change SemiSpace, NewSpace, and Heap containment methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

to

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

[Michael Lippautz, 2016-02-05 12:31:26]

Description was changed from

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

to

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a
slow fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

[Michael Lippautz, 2016-02-05 12:31:40]

Description was changed from

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a
slow fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

to

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a slow
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

[Michael Lippautz, 2016-02-05 12:32:25]

Description was changed from

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Preparing the young generation for non-contiguous backing memory, this change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a slow
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

to

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Preparing the young generation for (real) non-contiguous backing memory, this
change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a slow
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

[Michael Lippautz, 2016-02-05 12:44:39]

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

[Michael Lippautz, 2016-02-05 12:44:40]

This is missing the mips/ppc ports but I would already like to get some general
feedback.

Ulan, Hannes: Whoever is not on vacation/leave , PTAL at everything  ;)
Yang: PTAL at the serializer

Performance wise I benchmarked this locally (20 runs) on Splay[Latency] in 
deterministic mode:
  Splay: +0.3% 
  SplayLatency: -1% (might still be a flake)

https://codereview.chromium.org/1632913003/diff/260001/src/heap/slots-buffer.cc
File src/heap/slots-buffer.cc (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/slots-buffer....
src/heap/slots-buffer.cc:60:
heap->InNewSpaceSlow(reinterpret_cast<Address>(object)) ||
Since we only do the check for the live object later (line below), we need to
use the slow version here. (Slot could be in dead memory.)

(Might be gone anyways with a new slots buffer.)

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces.cc
File src/heap/spaces.cc (left):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces.cc#old...
src/heap/spaces.cc:1848: SemiSpace tmp = *from;
Let's move this to explicit code (std::swap), to avoid the confusing dance
between to and from space id_s (switching to/from, only to switch them back in
FlipPages).

https://codereview.chromium.org/1632913003/diff/260001/src/snapshot/serialize.cc
File src/snapshot/serialize.cc (right):

https://codereview.chromium.org/1632913003/diff/260001/src/snapshot/serialize...
src/snapshot/serialize.cc:1015: isolate->heap()->RecordWriteSlow(               
                        \
I tried switching this to the fast check, which assumes that we have a valid
Object* already, but unfortunately some tests crashed.

So I assume that current_object_address could also be some off-heap address at
this point (which requires the slow version to be filtered out).  Is this the
case?

[Yang, 2016-02-05 12:54:05]

https://codereview.chromium.org/1632913003/diff/260001/src/snapshot/serialize.cc
File src/snapshot/serialize.cc (right):

https://codereview.chromium.org/1632913003/diff/260001/src/snapshot/serialize...
src/snapshot/serialize.cc:1015: isolate->heap()->RecordWriteSlow(               
                        \
On 2016/02/05 12:44:40, Michael Lippautz wrote:
> I tried switching this to the fast check, which assumes that we have a valid
> Object* already, but unfortunately some tests crashed.
> 
> So I assume that current_object_address could also be some off-heap address at
> this point (which requires the slow version to be filtered out).  Is this the
> case?

Yes I think this is the case. The deserializer is an object visitor that
iterates the heap. There we also iterate addresses that are not on the heap, for
example builtins code list.

[Hannes Payer (out of office), 2016-02-08 16:05:40]

First round of comments.

https://codereview.chromium.org/1632913003/diff/260001/src/arm/macro-assemble...
File src/arm/macro-assembler-arm.h (right):

https://codereview.chromium.org/1632913003/diff/260001/src/arm/macro-assemble...
src/arm/macro-assembler-arm.h:222: Label* branch) {
I think this one should be ne and the one below eq.

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.cc#newco...
src/heap/heap.cc:4368: (new_space_.ToSpaceContainsSlow(addr) ||
old_space_->Contains(addr) ||
If ToSpaceContains takes an object, this check will be fast again.

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h#newcod...
src/heap/heap.h:1086: inline void RecordWriteSlow(Address address, int offset);
As discussed offline, this one should become Object... or disappear if not
necessary anymore.

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h#newcod...
src/heap/heap.h:1135: inline bool InNewSpaceSlow(Address address);
InNewSpaceSlow seems to be not necessary anymore. It's only caller is in
RemoveInvalidSlots (after removing the serializer caller). There you cast to
address, but you have object. Just call the object version.

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h#newcod...
src/heap/heap.h:1143: bool Contains(Address addr);
Can we delete Contains(Address addr)? It seems like we are able to survive with
the heap object interface (I did a quick check over the code base...).

Overall, it would be good to get rid of as many Address interfaces as possible.

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces-inl.h
File src/heap/spaces-inl.h (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces-inl.h#...
src/heap/spaces-inl.h:194: bool SemiSpace::ContainsSlow(Address a) {
This one is really slow and should be just used for verification purpose. As
mentioned in other comments we should really try to get rid of the Address
interfaces.

If we have to provide Address, we can always check the PageHeaderTag for faster
lookup.

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1800: void SemiSpace::FixPages(intptr_t flags, intptr_t mask)
{
FixPagesFlags?

[Michael Lippautz, 2016-02-08 18:00:39]

Thanks a lot for the detailed comments!

General:
- Most "Address" interfaces are now versions that can take arbitrary (even
off-heap) addresses and almost only used for verification/debugging. They are
named *Slow(). For example, we use the Heap::ContainsSlow() version in the
simulator during debugging.
- The write barrier is now only a single interface which also does the value
check.

(I left most DCHECKs at the fast version if possible. The CL is getting big
already.)

https://codereview.chromium.org/1632913003/diff/260001/src/arm/macro-assemble...
File src/arm/macro-assembler-arm.h (right):

https://codereview.chromium.org/1632913003/diff/260001/src/arm/macro-assemble...
src/arm/macro-assembler-arm.h:222: Label* branch) {
On 2016/02/08 16:05:40, Hannes Payer wrote:
> I think this one should be ne and the one below eq.

I don't think so. I think that the code is confusing though...

The code does:
  mask = IN_FROM_SPACE | IN_TO_SPACE
  TST flags, mask => does an AND, and sets Z(ero) flag according.
  Beq => jumps on Z set, which is when neither IN_FROM_SPACE or IN_TO_SPACE is
set

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.cc#newco...
src/heap/heap.cc:4368: (new_space_.ToSpaceContainsSlow(addr) ||
old_space_->Contains(addr) ||
On 2016/02/08 16:05:40, Hannes Payer wrote:
> If ToSpaceContains takes an object, this check will be fast again.

Done.

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h#newcod...
src/heap/heap.h:1086: inline void RecordWriteSlow(Address address, int offset);
On 2016/02/08 16:05:40, Hannes Payer wrote:
> As discussed offline, this one should become Object... or disappear if not
> necessary anymore.

The only remaining interface now is
  // Write barrier support for object[offset] = o;
  inline void RecordWrite(Object* object, int offset, Object* o);

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h#newcod...
src/heap/heap.h:1135: inline bool InNewSpaceSlow(Address address);
On 2016/02/08 16:05:40, Hannes Payer wrote:
> InNewSpaceSlow seems to be not necessary anymore. It's only caller is in
> RemoveInvalidSlots (after removing the serializer caller). There you cast to
> address, but you have object. Just call the object version.

Unfortunately I think this one needs to stay right now because
RemoveInvalidSlots only checks for whether the slot is contained in a live
object later on. So I think we do not have an Object there but in fact just any
Address (potentially an unboxed double).

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h#newcod...
src/heap/heap.h:1143: bool Contains(Address addr);
On 2016/02/08 16:05:40, Hannes Payer wrote:
> Can we delete Contains(Address addr)? It seems like we are able to survive
with
> the heap object interface (I did a quick check over the code base...).
> 
> Overall, it would be good to get rid of as many Address interfaces as
possible.

"Done." See general description above.

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces-inl.h
File src/heap/spaces-inl.h (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces-inl.h#...
src/heap/spaces-inl.h:194: bool SemiSpace::ContainsSlow(Address a) {
On 2016/02/08 16:05:40, Hannes Payer wrote:
> This one is really slow and should be just used for verification purpose. As
> mentioned in other comments we should really try to get rid of the Address
> interfaces.
> 
> If we have to provide Address, we can always check the PageHeaderTag for
faster
> lookup.

Acknowledged.

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1800: void SemiSpace::FixPages(intptr_t flags, intptr_t mask)
{
On 2016/02/08 16:05:40, Hannes Payer wrote:
> FixPagesFlags?

Done.

https://codereview.chromium.org/1632913003/diff/260001/src/snapshot/serialize.cc
File src/snapshot/serialize.cc (right):

https://codereview.chromium.org/1632913003/diff/260001/src/snapshot/serialize...
src/snapshot/serialize.cc:1015: isolate->heap()->RecordWriteSlow(               
                        \
On 2016/02/05 12:54:05, Yang wrote:
> On 2016/02/05 12:44:40, Michael Lippautz wrote:
> > I tried switching this to the fast check, which assumes that we have a valid
> > Object* already, but unfortunately some tests crashed.
> > 
> > So I assume that current_object_address could also be some off-heap address
at
> > this point (which requires the slow version to be filtered out).  Is this
the
> > case?
> 
> Yes I think this is the case. The deserializer is an object visitor that
> iterates the heap. There we also iterate addresses that are not on the heap,
for
> example builtins code list.

Actually, we are in a path where we can cast to Object*, as we've ruled out
builtins already above. I was lacking the pointer tag though, leading to misses
entries.

https://codereview.chromium.org/1632913003/diff/280001/src/frames.cc
File src/frames.cc (left):

https://codereview.chromium.org/1632913003/diff/280001/src/frames.cc#oldcode1522
src/frames.cc:1522: !heap->lo_space()->Contains(inner_pointer)) {
This was already a slow version. The name now reflects the kind of lookup.

[Hannes Payer (out of office), 2016-02-08 21:33:51]

https://codereview.chromium.org/1632913003/diff/260001/src/arm/macro-assemble...
File src/arm/macro-assembler-arm.h (right):

https://codereview.chromium.org/1632913003/diff/260001/src/arm/macro-assemble...
src/arm/macro-assembler-arm.h:222: Label* branch) {
On 2016/02/08 18:00:38, Michael Lippautz wrote:
> On 2016/02/08 16:05:40, Hannes Payer wrote:
> > I think this one should be ne and the one below eq.
> 
> I don't think so. I think that the code is confusing though...
> 
> The code does:
>   mask = IN_FROM_SPACE | IN_TO_SPACE
>   TST flags, mask => does an AND, and sets Z(ero) flag according.
>   Beq => jumps on Z set, which is when neither IN_FROM_SPACE or IN_TO_SPACE is
> set

Acknowledged.

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h#newcod...
src/heap/heap.h:1135: inline bool InNewSpaceSlow(Address address);
On 2016/02/08 18:00:38, Michael Lippautz wrote:
> On 2016/02/08 16:05:40, Hannes Payer wrote:
> > InNewSpaceSlow seems to be not necessary anymore. It's only caller is in
> > RemoveInvalidSlots (after removing the serializer caller). There you cast to
> > address, but you have object. Just call the object version.
> 
> Unfortunately I think this one needs to stay right now because
> RemoveInvalidSlots only checks for whether the slot is contained in a live
> object later on. So I think we do not have an Object there but in fact just
any
> Address (potentially an unboxed double).

I think you mentioned this already last week: if you re-arrange the new space
and live object check then you know for sure that it must be an object. Hence,
you can get rid of the address interface.

[Michael Lippautz, 2016-02-09 08:03:06]

PTAL. I kept the *Slow versions around for debugging and verifying.

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/heap.h#newcod...
src/heap/heap.h:1135: inline bool InNewSpaceSlow(Address address);
On 2016/02/08 21:33:51, Hannes Payer wrote:
> On 2016/02/08 18:00:38, Michael Lippautz wrote:
> > On 2016/02/08 16:05:40, Hannes Payer wrote:
> > > InNewSpaceSlow seems to be not necessary anymore. It's only caller is in
> > > RemoveInvalidSlots (after removing the serializer caller). There you cast
to
> > > address, but you have object. Just call the object version.
> > 
> > Unfortunately I think this one needs to stay right now because
> > RemoveInvalidSlots only checks for whether the slot is contained in a live
> > object later on. So I think we do not have an Object there but in fact just
> any
> > Address (potentially an unboxed double).
> 
> I think you mentioned this already last week: if you re-arrange the new space
> and live object check then you know for sure that it must be an object. Hence,
> you can get rid of the address interface. 

Done. The live object check is quite expensive (although it has a lot of
bailouts and only operates on mark bits. It's probably still faster than doing
different page lookups (in the slow version).

[Yang, 2016-02-09 09:54:26]

https://codereview.chromium.org/1632913003/diff/320001/src/snapshot/serialize.cc
File src/snapshot/serialize.cc (right):

https://codereview.chromium.org/1632913003/diff/320001/src/snapshot/serialize...
src/snapshot/serialize.cc:1017: /* Builtins have been filtered above, so we are
on-heap. */              \
This comment makes no sense to me. Maybe you mean the else-clase in line 982

DCHECK(where == kBuiltin);

This has nothing to do with whether we are iterating the builtins list. This
merely means that the serialized object refers to a builtin, and we serialized
that reference via the builtin's index.

There are other cases where we iterate off-heap data structures that we consider
roots, when we deserialize, for example:
- the root list (when we deserialize the isolate instead of initializing the
heap from scratch).
- the dispatch table for the interpreter.
- the partial snapshot cache.

[Michael Lippautz, 2016-02-09 13:28:54]

https://codereview.chromium.org/1632913003/diff/320001/src/snapshot/serialize.cc
File src/snapshot/serialize.cc (right):

https://codereview.chromium.org/1632913003/diff/320001/src/snapshot/serialize...
src/snapshot/serialize.cc:1017: /* Builtins have been filtered above, so we are
on-heap. */              \
On 2016/02/09 09:54:26, Yang wrote:
> This comment makes no sense to me. Maybe you mean the else-clase in line 982
> 
> DCHECK(where == kBuiltin);
> 
> This has nothing to do with whether we are iterating the builtins list. This
> merely means that the serialized object refers to a builtin, and we serialized
> that reference via the builtin's index.
> 
> There are other cases where we iterate off-heap data structures that we
consider
> roots, when we deserialize, for example:
> - the root list (when we deserialize the isolate instead of initializing the
> heap from scratch).
> - the dispatch table for the interpreter.
> - the partial snapshot cache.

Thanks for the clarification!

As discussed offline, we are on heap in this branch as we cannot have any
references to new space from off heap roots/addresses right now. I added a
DCHECK (which is exact, but slow) here so we can revisit the code once we do not
have this invariant anymore.

[ulan, 2016-02-09 13:36:50]

lgtm, two nits

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1841: std::swap(from->current_capacity_,
to->current_capacity_);
Let's extract it into separate function and put it near the constructor.

https://codereview.chromium.org/1632913003/diff/340001/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/1632913003/diff/340001/src/frames.cc#newcode1520
src/frames.cc:1520: !heap->lo_space()->ContainsSlow(inner_pointer)) {
We check the large space again below, so here we could avoid ContainsSlow by
moving the if statement after the large space check.

[Hannes Payer (out of office), 2016-02-09 14:40:23]

lgtm, with nits

https://codereview.chromium.org/1632913003/diff/340001/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):

https://codereview.chromium.org/1632913003/diff/340001/src/arm/simulator-arm....
src/arm/simulator-arm.cc:396: current_heap->ContainsSlow(obj->address())) {
Why ContainsSlow here? This is not a verifier method.

https://codereview.chromium.org/1632913003/diff/340001/src/arm64/simulator-ar...
File src/arm64/simulator-arm64.cc (right):

https://codereview.chromium.org/1632913003/diff/340001/src/arm64/simulator-ar...
src/arm64/simulator-arm64.cc:3544: current_heap->ContainsSlow(obj->address())) {
Why ContainsSlow here? This is not a verifier method.

https://codereview.chromium.org/1632913003/diff/340001/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/1632913003/diff/340001/src/frames.cc#newcode1520
src/frames.cc:1520: !heap->lo_space()->ContainsSlow(inner_pointer)) {
On 2016/02/09 13:36:50, ulan wrote:
> We check the large space again below, so here we could avoid ContainsSlow by
> moving the if statement after the large space check.

+1

https://codereview.chromium.org/1632913003/diff/340001/src/heap/slots-buffer.cc
File src/heap/slots-buffer.cc (right):

https://codereview.chromium.org/1632913003/diff/340001/src/heap/slots-buffer....
src/heap/slots-buffer.cc:62: heap->InNewSpace(object) ||
Can you leave a todo(mlippautz) here, which says that you are going to move the
new space check back where it was before when double unboxed slots are filtered
using the new method?

https://codereview.chromium.org/1632913003/diff/340001/src/snapshot/serialize.cc
File src/snapshot/serialize.cc (right):

https://codereview.chromium.org/1632913003/diff/340001/src/snapshot/serialize...
src/snapshot/serialize.cc:1019: reinterpret_cast<Object*>(current_object_address
+ 1),               \
use HeapObject::FromAddress

https://codereview.chromium.org/1632913003/diff/340001/src/snapshot/serialize...
src/snapshot/serialize.cc:1257: reinterpret_cast<Object*>(current_object_address
+ 1),
use HeapObject::FromAddress

[Michael Lippautz, 2016-02-09 14:50:28]

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1632913003/diff/260001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1841: std::swap(from->current_capacity_,
to->current_capacity_);
On 2016/02/09 13:36:50, ulan wrote:
> Let's extract it into separate function and put it near the constructor.

As discussed offline: This is already Swap(), and it's already right next to the
constructor ;)

https://codereview.chromium.org/1632913003/diff/340001/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):

https://codereview.chromium.org/1632913003/diff/340001/src/arm/simulator-arm....
src/arm/simulator-arm.cc:396: current_heap->ContainsSlow(obj->address())) {
On 2016/02/09 14:40:22, Hannes Payer wrote:
> Why ContainsSlow here? This is not a verifier method.

As discussed offline: There's a reinterpret_cast above (line: 392) and we are
iterating over raw memory, which could contain anything.

https://codereview.chromium.org/1632913003/diff/340001/src/arm64/simulator-ar...
File src/arm64/simulator-arm64.cc (right):

https://codereview.chromium.org/1632913003/diff/340001/src/arm64/simulator-ar...
src/arm64/simulator-arm64.cc:3544: current_heap->ContainsSlow(obj->address())) {
On 2016/02/09 14:40:22, Hannes Payer wrote:
> Why ContainsSlow here? This is not a verifier method.

As above.

https://codereview.chromium.org/1632913003/diff/340001/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/1632913003/diff/340001/src/frames.cc#newcode1520
src/frames.cc:1520: !heap->lo_space()->ContainsSlow(inner_pointer)) {
On 2016/02/09 13:36:50, ulan wrote:
> We check the large space again below, so here we could avoid ContainsSlow by
> moving the if statement after the large space check.

Done.

https://codereview.chromium.org/1632913003/diff/340001/src/frames.cc#newcode1520
src/frames.cc:1520: !heap->lo_space()->ContainsSlow(inner_pointer)) {
On 2016/02/09 14:40:22, Hannes Payer wrote:
> On 2016/02/09 13:36:50, ulan wrote:
> > We check the large space again below, so here we could avoid ContainsSlow by
> > moving the if statement after the large space check.
> 
> +1

Done.

https://codereview.chromium.org/1632913003/diff/340001/src/heap/slots-buffer.cc
File src/heap/slots-buffer.cc (right):

https://codereview.chromium.org/1632913003/diff/340001/src/heap/slots-buffer....
src/heap/slots-buffer.cc:62: heap->InNewSpace(object) ||
On 2016/02/09 14:40:22, Hannes Payer wrote:
> Can you leave a todo(mlippautz) here, which says that you are going to move
the
> new space check back where it was before when double unboxed slots are
filtered
> using the new method?

Done.

https://codereview.chromium.org/1632913003/diff/340001/src/snapshot/serialize.cc
File src/snapshot/serialize.cc (right):

https://codereview.chromium.org/1632913003/diff/340001/src/snapshot/serialize...
src/snapshot/serialize.cc:1019: reinterpret_cast<Object*>(current_object_address
+ 1),               \
On 2016/02/09 14:40:22, Hannes Payer wrote:
> use HeapObject::FromAddress

Done.

https://codereview.chromium.org/1632913003/diff/340001/src/snapshot/serialize...
src/snapshot/serialize.cc:1257: reinterpret_cast<Object*>(current_object_address
+ 1),
On 2016/02/09 14:40:23, Hannes Payer wrote:
> use HeapObject::FromAddress

Done.

[Michael Lippautz, 2016-02-09 14:50:59]

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

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

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

[commit-bot: I haz the power, 2016-02-09 14:55:50]

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

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

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

[Yang, 2016-02-09 15:37:54]

On 2016/02/09 14:55:51, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   v8_linux_mipsel_compile_rel on tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)

serializer lgtm.

[Michael Lippautz, 2016-02-09 20:03:37]

Patchset #15 (id:380001) has been deleted

[Michael Lippautz, 2016-02-09 20:18:02]

An update on the asan crahser:

tl;dr We crashed because we were looking up a page header for a stale store
buffer entry.

Details:
1) We wrote a constructor in the shared constructor/backpointer field,
triggering a WB.
2) We wrote a backpointer to some other map without WB.
3) The map behind the back pointer died.
4) In fact, the whole page died, triggering the unmapping of the page directly
after starting sweeping.
5) Pointer updates later on tried to call InNewSpace(), which now requires page
headers to be present.
6) We crashed, iff the unmapping task was faster then the main thread in giving
up the page reservations.

Why it previously worked:
(a) Before the new slot set we filtered all spaces (including map space) for
invalid pointers. We currently only do this for old space, because the filtering
should only catch unboxed doubles.
(b) We relied on the object masks for new space containment checks. Those checks
were also safe for unmapped memory.

(b) was the quite subtle safety net for (a), which I removed in this CL.

Possible solutions:
(1) Add map space again to filtering invalid slots.
(2) Delay unmapping of empty pages.

We decided to go with (2) and not (1) because filtering should really only be
required for unboxed doubles and we want to get rid of it anyways. Additionally,
updating pointers for the new slots buffer will also require the page headers to
be present.

[Michael Lippautz, 2016-02-10 09:08:11]

Description was changed from

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Preparing the young generation for (real) non-contiguous backing memory, this
change
removes object masks that are used to compute containment in semi and new space.
The masks are replaced by lookups for object tags and page headers, and a slow
fallback for raw addresses.

BUG=chromium:581412
LOG=N
==========

to

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Preparing the young generation for (real) non-contiguous backing memory, this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced by lookups for object tags and page headers, where
possible.

Details:
- Use the fast checks (page header lookups) for containment in regular code.
- Use the slow version that masks out the page start adress and iterates all
  pages of a space for debugging/verification.
- The slow version works for off-heap/unmapped memory.
- Encapsulate all checks for the old->new barrier in Heap::RecordWrite().

BUG=chromium:581412
LOG=N
==========

[ulan, 2016-02-10 09:12:25]

On 2016/02/09 20:18:02, Michael Lippautz wrote:
> An update on the asan crahser:
> 
> tl;dr We crashed because we were looking up a page header for a stale store
> buffer entry.
> 
> Details:
> 1) We wrote a constructor in the shared constructor/backpointer field,
> triggering a WB.
> 2) We wrote a backpointer to some other map without WB.
> 3) The map behind the back pointer died.
> 4) In fact, the whole page died, triggering the unmapping of the page directly
> after starting sweeping.
> 5) Pointer updates later on tried to call InNewSpace(), which now requires
page
> headers to be present.
> 6) We crashed, iff the unmapping task was faster then the main thread in
giving
> up the page reservations.
> 
> Why it previously worked:
> (a) Before the new slot set we filtered all spaces (including map space) for
> invalid pointers. We currently only do this for old space, because the
filtering
> should only catch unboxed doubles.
> (b) We relied on the object masks for new space containment checks. Those
checks
> were also safe for unmapped memory.
> 
> (b) was the quite subtle safety net for (a), which I removed in this CL.
> 
> Possible solutions:
> (1) Add map space again to filtering invalid slots.
> (2) Delay unmapping of empty pages.
> 
> We decided to go with (2) and not (1) because filtering should really only be
> required for unboxed doubles and we want to get rid of it anyways.
Additionally,
> updating pointers for the new slots buffer will also require the page headers
to
> be present.

mips ports and the fix lgtm! :)

[Michael Lippautz, 2016-02-10 09:20:59]

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

[commit-bot: I haz the power, 2016-02-10 09:21:03]

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

[commit-bot: I haz the power, 2016-02-10 09:42:38]

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

[commit-bot: I haz the power, 2016-02-10 09:42:39]

Dry run: This issue passed the CQ dry run.

[Michael Lippautz, 2016-02-10 09:43:02]

The CQ bit was checked by mlippautz@chromium.org

[Michael Lippautz, 2016-02-10 09:43:03]

The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org,
yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/1632913003/#ps420001
(title: "mips ports")

[commit-bot: I haz the power, 2016-02-10 09:43:16]

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

[commit-bot: I haz the power, 2016-02-10 09:46:36]

Committed patchset #16 (id:420001)

[commit-bot: I haz the power, 2016-02-10 09:47:29]

Description was changed from

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Preparing the young generation for (real) non-contiguous backing memory, this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced by lookups for object tags and page headers, where
possible.

Details:
- Use the fast checks (page header lookups) for containment in regular code.
- Use the slow version that masks out the page start adress and iterates all
  pages of a space for debugging/verification.
- The slow version works for off-heap/unmapped memory.
- Encapsulate all checks for the old->new barrier in Heap::RecordWrite().

BUG=chromium:581412
LOG=N
==========

to

==========
[heap] Move to page lookups for SemiSpace, NewSpace, and Heap containment
methods

Preparing the young generation for (real) non-contiguous backing memory, this
change removes object masks that are used to compute containment in semi and new
space. The masks are replaced by lookups for object tags and page headers, where
possible.

Details:
- Use the fast checks (page header lookups) for containment in regular code.
- Use the slow version that masks out the page start adress and iterates all
  pages of a space for debugging/verification.
- The slow version works for off-heap/unmapped memory.
- Encapsulate all checks for the old->new barrier in Heap::RecordWrite().

BUG=chromium:581412
LOG=N

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

[commit-bot: I haz the power, 2016-02-10 09:47:30]

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