https://chromiumcodereview.appspot.com/11368137/diff/2001/src/incremental-marking.cc
File src/incremental-marking.cc (right):

https://chromiumcodereview.appspot.com/11368137/diff/2001/src/incremental-marking.cc#newcode207
src/incremental-marking.cc:207:
IncrementalMarkingMarkingVisitor::VisitNativeContext(map, context);
On 2012/11/08 11:10:39, Michael Starzinger wrote:
This method is actually in the StaticMarkingVisitor, so I would just
remove the
class prefix in this tail call to VisitNativeContext.

Done.

https://chromiumcodereview.appspot.com/11368137/diff/2001/src/incremental-marking.cc#newcode632
src/incremental-marking.cc:632: void IncrementalMarking::MarkObject(Map*
map, HeapObject* obj) {
I added a size parameter to the VisitObject method. That keeps the
flexibility.

Done.

On 2012/11/08 11:10:39, Michael Starzinger wrote:
This should actually be called "ProcessObject" or "VisitObject",
because the
marking already happened when doing white->grey. Now that we do
grey->black we
are actually visiting the object and iterating it.

Also I checked the disassembly and this is currently not inlined into
the two
processing loops. It has to be marked as INLINE(...) in the header.

Also even in the inlined version, SizeFromMap() is called twice in the
stepping
case. We could pass in the size as an argument. But at this point I am
inclined
to actually manually inline it into the two processing loops. It
shouldn't be
too much code duplication in this case. WDYT?

https://chromiumcodereview.appspot.com/11368137/diff/2001/src/incremental-marking.cc#newcode648
src/incremental-marking.cc:648: void
IncrementalMarking::ProcessMarking(intptr_t bytes_to_process) {
On 2012/11/08 11:10:39, Michael Starzinger wrote:
Let's rename that to ProcessMarkingDeque, that's more descriptive.

Done.

https://chromiumcodereview.appspot.com/11368137/diff/2001/src/incremental-marking.cc#newcode665
src/incremental-marking.cc:665: void
IncrementalMarking::ProcessMarking() {
On 2012/11/08 11:10:39, Michael Starzinger wrote:
Likewise.

Done.

https://chromiumcodereview.appspot.com/11368137/diff/2001/src/incremental-marking.h
File src/incremental-marking.h (right):

https://chromiumcodereview.appspot.com/11368137/diff/2001/src/incremental-marking.h#newcode261
src/incremental-marking.h:261: void ProcessMarking();
On 2012/11/08 11:10:39, Michael Starzinger wrote:
Add an empty newline in front of these three method declarations.

Done.

https://chromiumcodereview.appspot.com/11368137/diff/2001/src/objects-visiting.h
File src/objects-visiting.h (right):

https://chromiumcodereview.appspot.com/11368137/diff/2001/src/objects-visiting.h#newcode409
src/objects-visiting.h:409: static inline void
VisitNativeContextIncremental(Map* map,
On 2012/11/08 11:10:39, Michael Starzinger wrote:
Remove the declaration of the VisitNativeContextIncremental
declaration, there
is no such method in this base class.

Done.

https://chromiumcodereview.appspot.com/11368137/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to