[v8-dev] Re: Parallel and concurrent sweeping. (issue 11782028)

[mstarzinger]

This is going into the right direction. A first round of high level  
comments.

https://chromiumcodereview.appspot.com/11782028/diff/19002/src/flag-definitions.h
File src/flag-definitions.h (right):

https://chromiumcodereview.appspot.com/11782028/diff/19002/src/flag-definitions.h#newcode416
src/flag-definitions.h:416: DEFINE_bool(concurrent_sweeping, false,
"enable concurrent sweeping")
As discussed offline, for now it makes sense to have both flags until we
stabilize the implementation. Later we should unify them into one.

https://chromiumcodereview.appspot.com/11782028/diff/19002/src/heap.h
File src/heap.h (right):

https://chromiumcodereview.appspot.com/11782028/diff/19002/src/heap.h#newcode1091
src/heap.h:1091: bool IsConcurrentSweepingPending();
We should try to move most of that functionality into MarkCompact and
minimize the entry points in Heap. IMHO parallel sweeping should be
considered a subsystem of MarkCompact.

https://chromiumcodereview.appspot.com/11782028/diff/19002/src/heap.h#newcode1578
src/heap.h:1578: void SweepInParallel(PagedSpace* paged_space,
This dispatcher should not be necessary.

https://chromiumcodereview.appspot.com/11782028/diff/19002/src/heap.h#newcode1588
src/heap.h:1588: bool AdvanceSweepers(int step_size) {
This is still being called from the idle notification. For both modes it
should be a NOP.

https://chromiumcodereview.appspot.com/11782028/diff/19002/src/isolate.cc
File src/isolate.cc (right):

https://chromiumcodereview.appspot.com/11782028/diff/19002/src/isolate.cc#newcode1733
src/isolate.cc:1733: if (FLAG_concurrent_sweeping ||
FLAG_parallel_sweeping) {
Move this above the stopping of the optimizing compiler so that de-init
order is the reverse of init order.

https://chromiumcodereview.appspot.com/11782028/diff/19002/src/isolate.cc#newcode2106
src/isolate.cc:2106: if (FLAG_sweeper_threads < 1) {
Just set it to one if it's smaller than one.

https://chromiumcodereview.appspot.com/11782028/diff/19002/src/spaces.cc
File src/spaces.cc (right):

https://chromiumcodereview.appspot.com/11782028/diff/19002/src/spaces.cc#newcode2464
src/spaces.cc:2464: if (heap()->IsConcurrentSweepingActivated()) {
The SlowAllocateRaw() function is getting too complicated. Would it be
possible to move all the sweeper-related code into a separate
EnsureSweeperProgress() function and basically keep the original
implementation of SlowAllocateRaw() almost untouched?

https://chromiumcodereview.appspot.com/11782028/

--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev