http://codereview.chromium.org/8099/diff/1/8 File src/mark-compact.cc (right):
http://codereview.chromium.org/8099/diff/1/8#newcode455 Line 455: Object *object = Memory::Object_at( On 2008/10/23 11:33:58, Kevin Millikin wrote: > Object* object ... Done. http://codereview.chromium.org/8099/diff/1/8#newcode457 Line 457: if (object->IsHeapObject()) { On 2008/10/23 11:33:58, Kevin Millikin wrote: > We should try to avoid the situation where adding a field to maps breaks this > code. Maybe we should consider putting the descriptors last and all the other > pointers together and looping over them (the pointers), then documenting why > that has to be the case in the layout part of class Map. Yeah, maybe we should. http://codereview.chromium.org/8099/diff/1/8#newcode478 Line 478: d->SetMark(); On 2008/10/23 11:33:58, Kevin Millikin wrote: > Consider moving this after the marking of the contents, and adding a comment > that pushing it will cause the *unmarked* children (only) to be recursively > marked. Done. http://codereview.chromium.org/8099/diff/1/8#newcode488 Line 488: // If the pair (details, value) at index 2i, 2i+1 is not On 2008/10/23 11:33:58, Kevin Millikin wrote: > i, i+1 ? Done. http://codereview.chromium.org/8099/diff/1/8#newcode869 Line 869: Object* next = map; On 2008/10/23 11:33:58, Kevin Millikin wrote: > For some reason, I think "parent" is a more evocative name.... Done. http://codereview.chromium.org/8099/diff/1/8#newcode870 Line 870: MapWord next_map = map->map_word(); // No public or default constructor. On 2008/10/23 11:33:58, Kevin Millikin wrote: > Here, next_map_word (or parent_map_word), since it's not a map. Done. http://codereview.chromium.org/8099/diff/1/2 File src/objects.cc (right): http://codereview.chromium.org/8099/diff/1/2#newcode4051 Line 4051: #ifdef DEBUG On 2008/10/23 11:33:58, Kevin Millikin wrote: > I think it's clearer to move the comment "Verify target." inside the #ifdef > DEBUG, and move the #endif to after all the asserts. Done. http://codereview.chromium.org/8099/diff/1/2#newcode4059 Line 4059: source_prototype->IsOddball()); On 2008/10/23 11:33:58, Kevin Millikin wrote: > Can it really be an oddball other than null? Done. http://codereview.chromium.org/8099/diff/1/2#newcode4064 Line 4064: // Point target back to source. On 2008/10/23 11:33:58, Kevin Millikin wrote: > Explain why set_prototype is undesirable here ('this' is not a JS object or > null, and we can avoid the write barrier). Done. http://codereview.chromium.org/8099/diff/1/2#newcode4072 Line 4072: // DescriptorArray objects may be marked, so we must use On 2008/10/23 11:33:58, Kevin Millikin wrote: > Is it more clear to say that live descriptor arrays (ie, the ones we consider > here) *will* be marked? Done. http://codereview.chromium.org/8099/diff/1/2#newcode4084 Line 4084: // check if the target is dead. If so, null the descriptor. On 2008/10/23 11:33:58, Kevin Millikin wrote: > I like non-live as the opposite of live (or simply unmarked). It doesn't sound > so harsh :) Done. http://codereview.chromium.org/8099/diff/1/7 File src/spaces.cc (right): http://codereview.chromium.org/8099/diff/1/7#newcode2025 Line 2025: HeapObjectIterator* iterator = new HeapObjectIterator(Heap::map_space()); On 2008/10/23 11:33:58, Kevin Millikin wrote: > new HeapObjectIterator(this) Done. Actually, HeapObjectIterator iterator(Heap::map_space()); A stack object, not a memory leak. http://codereview.chromium.org/8099/diff/1/7#newcode2026 Line 2026: while (iterator->has_next_object()) { On 2008/10/23 11:33:58, Kevin Millikin wrote: > has_next() Done. http://codereview.chromium.org/8099/diff/1/3 File src/spaces.h (right): http://codereview.chromium.org/8099/diff/1/3#newcode1456 Line 1456: void CreateBackPointers(); On 2008/10/23 11:33:58, Kevin Millikin wrote: > You might consider calling this "CreateParentPointers" to emphasize how it > affects the map-transition tree. You might also consider making it a (static) > member function of the full garbage collector. Done. http://codereview.chromium.org/8099 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
