This is already looking good, just a few comments.
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); This method is actually in the StaticMarkingVisitor, so I would just remove the class prefix in this tail call to VisitNativeContext. https://chromiumcodereview.appspot.com/11368137/diff/2001/src/incremental-marking.cc#newcode632 src/incremental-marking.cc:632: void IncrementalMarking::MarkObject(Map* map, HeapObject* obj) { 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) { Let's rename that to ProcessMarkingDeque, that's more descriptive. https://chromiumcodereview.appspot.com/11368137/diff/2001/src/incremental-marking.cc#newcode665 src/incremental-marking.cc:665: void IncrementalMarking::ProcessMarking() { Likewise. 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(); Add an empty newline in front of these three method declarations. 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, Remove the declaration of the VisitNativeContextIncremental declaration, there is no such method in this base class. https://chromiumcodereview.appspot.com/11368137/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
