I have uploaded a much improved version of the changes. Do you want to look at it again, Kevin?
On Thu, Oct 23, 2008 at 2:43 PM, <[EMAIL PROTECTED]> wrote: > Otherwise LGTM, > Lars > > > http://codereview.chromium.org/8099/diff/22/209 > File src/mark-compact.cc (right): > > http://codereview.chromium.org/8099/diff/22/209#newcode81 > Line 81: if (FLAG_collect_maps) { > make this a one liner. > > http://codereview.chromium.org/8099/diff/22/209#newcode142 > Line 142: if (FLAG_collect_maps) { > One line > > http://codereview.chromium.org/8099/diff/22/209#newcode457 > Line 457: Object* object = Memory::Object_at( > Make abstraction instead of make three copies of the code. > > Mark(map->field_at(Map::kConstructorOffset)) > > or something. > > http://codereview.chromium.org/8099/diff/22/209#newcode486 > Line 486: if (d != Heap::empty_descriptor_array()) { > Top avoid the deep nested code in NewJSArrayWithElements, > please make a new function to handle the DescriptorArray. > > http://codereview.chromium.org/8099/diff/22/209#newcode533 > Line 533: if (map->instance_type() >= FIRST_JS_OBJECT_TYPE && > This test should match the test IsJSObject. > >> = FIRST_JS_OBJECT_TYPE is sufficient. >> > > http://codereview.chromium.org/8099/diff/22/209#newcode781 > Line 781: static int CountMarkedCallback(HeapObject* obj) { > Should this be wrapped in #ifdef DEBUG? > > http://codereview.chromium.org/8099/diff/22/205 > File src/objects.cc (right): > > http://codereview.chromium.org/8099/diff/22/205#newcode4053 > Line 4053: Object* target_prototype = target->prototype(); > I do not understand why the "!= NULL" asserts are providing value. > > > http://codereview.chromium.org/8099 > -- We can IMAGINE what is not --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
