Adds a scavenge GC pass to collect unmodified references

Adds a scavenge GC pass to collect unmodified references

Adds a scavenge GC pass that collects unmodified references instead of
processing object groups. This mode can be controlled by setting
FLAG_scavenge_reclaim_unmodified_objects. By default this is turned off.
Also, modified a test case to suit the handle the new GC pass.

BUG=v8:4421
LOG=N

Committed: https://crrev.com/959e050c1dd943f89bec53dcae52f9279243d23e
Cr-Commit-Position: refs/heads/master@{#31599}

[mythria, 2015-10-20 12:52:10]

mythria@chromium.org changed reviewers:
+ jochen@chromium.org, rmcilroy@chromium.org

[mythria, 2015-10-20 12:52:10]

Hi,

I uploaded a new cl for the scavenge optimization to remove unmodified
references. It is mostly same as the patch I sent last time. The only difference
is I changed the flag from inactive to active. There was no reason I chose
inactive earlier, so I changed it to active. Could you please review the
changes.

Thanks and Regards,
Mythri

[jochen (gone - plz use gerrit), 2015-10-21 13:35:31]

(still waiting for an update to the blink side before proceeding here)

[rmcilroy, 2015-10-21 13:36:51]

https://codereview.chromium.org/1410593005/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/1410593005/diff/1/include/v8.h#newcode5944
include/v8.h:5944: 
nit - remove extra newline

https://codereview.chromium.org/1410593005/diff/1/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/1410593005/diff/1/src/flag-definitions.h#newc...
src/flag-definitions.h:684: DEFINE_BOOL(scavenge_reclaim_unmodified_objects,
true,
Set this to false for now (and set it to true from the Blink CL in
renderer_process_impl)

https://codereview.chromium.org/1410593005/diff/1/src/global-handles.cc
File src/global-handles.cc (right):

https://codereview.chromium.org/1410593005/diff/1/src/global-handles.cc#newco...
src/global-handles.cc:393: uint8_t flags_;
Maybe we could just make this a uint16_t and store Unmodified in the flag rather
than a new bool field. Or can we steal another bit from elsewhere? WDYT Jochen?

https://codereview.chromium.org/1410593005/diff/1/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1410593005/diff/1/src/heap/heap.cc#newcode1260
src/heap/heap.cc:1260: //     (gc_type == kGCTypeScavenge))) {
Remove comments

https://codereview.chromium.org/1410593005/diff/1/src/heap/heap.cc#newcode1348
src/heap/heap.cc:1348: //      (gc_type == kGCTypeScavenge))) {
Remove comments

[mythria, 2015-10-21 14:43:51]

Hi,

I addressed review comments from Ross. I will wait for comments from Jochen to
merge the unmodified_flag with flags.

Thanks and Regards,
Mythri

https://codereview.chromium.org/1410593005/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/1410593005/diff/1/include/v8.h#newcode5944
include/v8.h:5944: 
On 2015/10/21 13:36:50, rmcilroy wrote:
> nit - remove extra newline

Done.

https://codereview.chromium.org/1410593005/diff/1/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/1410593005/diff/1/src/flag-definitions.h#newc...
src/flag-definitions.h:684: DEFINE_BOOL(scavenge_reclaim_unmodified_objects,
true,
On 2015/10/21 13:36:50, rmcilroy wrote:
> Set this to false for now (and set it to true from the Blink CL in
> renderer_process_impl)

Done.

https://codereview.chromium.org/1410593005/diff/1/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1410593005/diff/1/src/heap/heap.cc#newcode1260
src/heap/heap.cc:1260: //     (gc_type == kGCTypeScavenge))) {
On 2015/10/21 13:36:51, rmcilroy wrote:
> Remove comments 

Done.

https://codereview.chromium.org/1410593005/diff/1/src/heap/heap.cc#newcode1348
src/heap/heap.cc:1348: //      (gc_type == kGCTypeScavenge))) {
On 2015/10/21 13:36:51, rmcilroy wrote:
> Remove comments

Done.

[jochen (gone - plz use gerrit), 2015-10-22 12:12:02]

https://codereview.chromium.org/1410593005/diff/40001/src/global-handles.cc
File src/global-handles.cc (right):

https://codereview.chromium.org/1410593005/diff/40001/src/global-handles.cc#n...
src/global-handles.cc:744: node->set_active(false);
why not just mark modified nodes as active, then you wouldn't need the
"unmodified" flag. no?

[mythria, 2015-10-27 11:22:52]

I removed unmodified flag and use active flag instead. Can you please review my
changes. I tried layout tests and browser tests and all of them pass (except for
two in browser tests which fail with original code too).

https://codereview.chromium.org/1410593005/diff/40001/src/global-handles.cc
File src/global-handles.cc (right):

https://codereview.chromium.org/1410593005/diff/40001/src/global-handles.cc#n...
src/global-handles.cc:744: node->set_active(false);
On 2015/10/22 12:12:02, jochen wrote:
> why not just mark modified nodes as active, then you wouldn't need the
> "unmodified" flag. no?

Done.

[jochen (gone - plz use gerrit), 2015-10-27 12:09:13]

lgtm

[rmcilroy, 2015-10-27 12:12:05]

lgtm!

[mythria, 2015-10-27 12:20:59]

Thanks for your reviews. I will land this change and once it lands, I will run
trybots on the blink side and if everything goes well will land the changes on
blink side too.

[mythria, 2015-10-27 12:21:03]

The CQ bit was checked by mythria@chromium.org

[commit-bot: I haz the power, 2015-10-27 12:21:08]

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

[commit-bot: I haz the power, 2015-10-27 12:22:28]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-10-27 12:22:36]

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