[heap] Fix rescanning live objects when aborting compaction.

[heap] Fix rescanning live objects when aborting compaction.

Fix aborting compaction for pages by doing two separate passes, one that scans
all live objects, and one that later on sweeps the page.

Detailed description see bug report.

BUG=chromium:539356, chromium:524425
LOG=N

Committed: https://crrev.com/5cf6ee3447fbc63a030ab180079ba93902cce057
Cr-Commit-Position: refs/heads/master@{#31526}

[Michael Lippautz, 2015-10-23 11:49:15]

Patchset #1 (id:1) has been deleted

[Michael Lippautz, 2015-10-23 11:50:02]

Description was changed from

==========
[heap] Fix rescanning live objects when aborting compaction.

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

to

==========
[heap] Fix rescanning live objects when aborting compaction.

BUG=chromium:539356,chromium:524425
LOG=N
==========

[Michael Lippautz, 2015-10-23 11:51:19]

Description was changed from

==========
[heap] Fix rescanning live objects when aborting compaction.

BUG=chromium:539356,chromium:524425
LOG=N
==========

to

==========
[heap] Fix rescanning live objects when aborting compaction.

Fix aborting compaction for pages by doing two separate passes, one that scans
all live objects, and one that later on sweeps the page.

Detailed description see bug report.

BUG=chromium:539356,chromium:524425
LOG=N
==========

[Michael Lippautz, 2015-10-23 11:58:33]

Description was changed from

==========
[heap] Fix rescanning live objects when aborting compaction.

Fix aborting compaction for pages by doing two separate passes, one that scans
all live objects, and one that later on sweeps the page.

Detailed description see bug report.

BUG=chromium:539356,chromium:524425
LOG=N
==========

to

==========
[heap] Fix rescanning live objects when aborting compaction.

Fix aborting compaction for pages by doing two separate passes, one that scans
all live objects, and one that later on sweeps the page.

Detailed description see bug report.

BUG=chromium:539356,chromium:524425
LOG=N
==========

[Michael Lippautz, 2015-10-23 11:58:33]

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

[Hannes Payer (out of office), 2015-10-23 13:09:18]

looking good, a few comments. this seems to be the easiest way to fix this bug.

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

https://codereview.chromium.org/1413763011/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3777: {
This function is getting super long (and complicated). Can we at least move code
into descriptive functions to have a better overview what is going on here.

https://codereview.chromium.org/1413763011/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3855: GCTracer::Scope gc_scope(heap()->tracer(),
This timer scope now also covers the sweeping.

https://codereview.chromium.org/1413763011/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3867: // Second pass on aborted pages.
Please add a separate timer scope here.

https://codereview.chromium.org/1413763011/diff/20001/src/heap/spaces.h
File src/heap/spaces.h (right):

https://codereview.chromium.org/1413763011/diff/20001/src/heap/spaces.h#newco...
src/heap/spaces.h:331: //   has been aborted to to lack of memory and needs
special handling by
just drop the "to to lack of memory"

[Michael Lippautz, 2015-10-23 13:27:05]

Not super happy with the complexity too. Factored out the new code into
functions.

In general, I am not too happy with the fix, but it seems to be the least
involving way to get those pages back into a regular state.

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

https://codereview.chromium.org/1413763011/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3777: {
On 2015/10/23 13:09:18, Hannes Payer wrote:
> This function is getting super long (and complicated). Can we at least move
code
> into descriptive functions to have a better overview what is going on here.

Factored out a few pieces.

https://codereview.chromium.org/1413763011/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3855: GCTracer::Scope gc_scope(heap()->tracer(),
On 2015/10/23 13:09:18, Hannes Payer wrote:
> This timer scope now also covers the sweeping.

Fixed to only track updating the pointers.

https://codereview.chromium.org/1413763011/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3867: // Second pass on aborted pages.
On 2015/10/23 13:09:18, Hannes Payer wrote:
> Please add a separate timer scope here.

Done.

https://codereview.chromium.org/1413763011/diff/20001/src/heap/spaces.h
File src/heap/spaces.h (right):

https://codereview.chromium.org/1413763011/diff/20001/src/heap/spaces.h#newco...
src/heap/spaces.h:331: //   has been aborted to to lack of memory and needs
special handling by
On 2015/10/23 13:09:18, Hannes Payer wrote:
> just drop the "to to lack of memory"

Done.

[Hannes Payer (out of office), 2015-10-23 13:53:42]

If we implement slot recording for evacuation candidates, the code may get
simpler again.

[Hannes Payer (out of office), 2015-10-23 14:00:48]

LGTM. Since this is fixing a current regression and writing a test for OOM cases
is hard, we can land this without a test. However, we should figure out how we
can write tests for such cases efficiently.

[Hannes Payer (out of office), 2015-10-23 14:01:57]

The CQ bit was checked by hpayer@chromium.org

[commit-bot: I haz the power, 2015-10-23 14:02:00]

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

[commit-bot: I haz the power, 2015-10-23 14:24:05]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-10-23 14:24:23]

Patchset 2 (id:??) landed as
https://crrev.com/5cf6ee3447fbc63a030ab180079ba93902cce057
Cr-Commit-Position: refs/heads/master@{#31526}