Round of comments.
https://chromiumcodereview.appspot.com/10386046/diff/1/src/incremental-marking.cc File src/incremental-marking.cc (right): https://chromiumcodereview.appspot.com/10386046/diff/1/src/incremental-marking.cc#newcode660 src/incremental-marking.cc:660: Map* map = obj->map(); Hurry should also treat maps in a special way. https://chromiumcodereview.appspot.com/10386046/diff/1/src/incremental-marking.cc#newcode827 src/incremental-marking.cc:827: } else if (map->instance_type() == JS_MAP_TYPE) { s/JS_MAP_TYPE/MAP_TYPE/ https://chromiumcodereview.appspot.com/10386046/diff/2001/src/incremental-marking-inl.h File src/incremental-marking-inl.h (right): https://chromiumcodereview.appspot.com/10386046/diff/2001/src/incremental-marking-inl.h#newcode131 src/incremental-marking-inl.h:131: bool IncrementalMarking::MarkObject(HeapObject* obj) { I suggest naming it MarkObjectAndPush https://chromiumcodereview.appspot.com/10386046/diff/2001/src/incremental-marking-inl.h#newcode145 src/incremental-marking-inl.h:145: MarkBlackOrKeepGrey(mark_bit); Can it be grey at this point? I don't think so. https://chromiumcodereview.appspot.com/10386046/diff/2001/src/incremental-marking.cc File src/incremental-marking.cc (right): https://chromiumcodereview.appspot.com/10386046/diff/2001/src/incremental-marking.cc#newcode836 src/incremental-marking.cc:836: if (FLAG_collect_maps && map->instance_type() >= FIRST_JS_RECEIVER_TYPE) { long line https://chromiumcodereview.appspot.com/10386046/diff/2001/src/mark-compact-inl.h File src/mark-compact-inl.h (right): https://chromiumcodereview.appspot.com/10386046/diff/2001/src/mark-compact-inl.h#newcode55 src/mark-compact-inl.h:55: bool MarkCompactCollector::MarkObject(HeapObject* obj) { Usually we avoid to use function overloading (per style guide). In this case it's especially dangerous as MarkObject(HeapObject*) and MarkObject(HeapObject*, MarkBit) do _completely_ different things. https://chromiumcodereview.appspot.com/10386046/diff/2001/src/mark-compact.cc File src/mark-compact.cc (right): https://chromiumcodereview.appspot.com/10386046/diff/2001/src/mark-compact.cc#newcode286 src/mark-compact.cc:286: if (FLAG_collect_maps) ClearNonLiveTransitions(); It seems that running with --nocollect-maps would really cause maps tree to be retained in both directions (from leaves to root and from root to leaves). https://chromiumcodereview.appspot.com/10386046/diff/2001/src/mark-compact.cc#newcode665 src/mark-compact.cc:665: clear_map_transitions_ = true; Why does not it depend on FLAG_collect_maps? Flag and this field have some very strange relation. Having something that is always true does not help. If you change it to false then potentially you might get broken heap structure (back pointer from live map pointing to a dead map). So for now I suggest we should either remove it or make it work. https://chromiumcodereview.appspot.com/10386046/diff/2001/src/mark-compact.cc#newcode1808 src/mark-compact.cc:1808: // Mark the Object* fields of the Map. Since the descriptor array has been Try to move this to MarkMapContents to further share code? https://chromiumcodereview.appspot.com/10386046/diff/2001/src/mark-compact.h File src/mark-compact.h (right): https://chromiumcodereview.appspot.com/10386046/diff/2001/src/mark-compact.h#newcode404 src/mark-compact.h:404: MarkCompactCollector* mark_compact_collector() { empty line before definition https://chromiumcodereview.appspot.com/10386046/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
