LGTM if final comments are addresed. I like this!

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

https://chromiumcodereview.appspot.com/11782028/diff/34001/src/isolate.cc#newcode1737
src/isolate.cc:1737: for (int i = 0; i < FLAG_sweeper_threads; i++) {
Can we merge the two for-loops, I think that's easier to read.

https://chromiumcodereview.appspot.com/11782028/diff/34001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://chromiumcodereview.appspot.com/11782028/diff/34001/src/mark-compact.cc#newcode3478
src/mark-compact.cc:3478: if (mode ==
MarkCompactCollector::SWEEP_SEQUENTIALLY) {
You could factor out this logic and move it into the static Free()
helper, templatize the helper and hope that GCC inlines it. Would make
the source cleaner and produce the same code. I would prefer that over
the current implementation, but I'll leave the decision whether to do
that or not up to you.

https://chromiumcodereview.appspot.com/11782028/diff/34001/src/mark-compact.cc#newcode3652
src/mark-compact.cc:3652: PrintF("Prepare conservatively parallel
sweeping 0x%" V8PRIxPTR ".\n",
Let's rephrase this log line to "Sweeping 0x??? conservatively in
parallel.", I think that's easier to understand.

https://chromiumcodereview.appspot.com/11782028/diff/34001/src/mark-compact.cc#newcode3706
src/mark-compact.cc:3706:
I think you have white-space on that empty line. Should trip a presubmit
failure.

https://chromiumcodereview.appspot.com/11782028/diff/34001/src/mark-compact.cc#newcode3715
src/mark-compact.cc:3715: // The starting of the sweeper threads should
be after SweepSpace
Let's turn this comment into a TODO().

https://chromiumcodereview.appspot.com/11782028/diff/34001/src/mark-compact.h
File src/mark-compact.h (right):

https://chromiumcodereview.appspot.com/11782028/diff/34001/src/mark-compact.h#newcode859
src/mark-compact.h:859: void PrepareParallelSweeping(PagedSpace* space);
The implementation of this method is gone, let's also drop the
declaration.

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

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to