LGTM, but I think you should extend the test case to make sure the normalization code has been tested and that the filler objects can be GC'ed correctly.
http://codereview.chromium.org/17308/diff/213/17 File src/objects.cc (right): http://codereview.chromium.org/17308/diff/213/17#newcode1919 Line 1919: Object* filler_map = Heap::AllocateMap(FILLER_TYPE, instance_size_delta); It seems a bit nasty to have to allocate a new map for the filler object. How about just using a fixed array if it fits (at least two words) or a simple one-word filler otherwise? http://codereview.chromium.org/17308/diff/213/18 File src/runtime.cc (right): http://codereview.chromium.org/17308/diff/213/18#newcode105 Line 105: int number_of_properties = constant_properties->length()/2; Spaces around / operator? http://codereview.chromium.org/17308/diff/213/14 File test/mjsunit/large-object-literal.js (right): http://codereview.chromium.org/17308/diff/213/14#newcode29 Line 29: var nofProperties = 256; How about refactoring this test code so that nofProperties is a parameter to a function and try it with different values like 1,2,5,35,150,255,256,280 and maybe extending it to force the object into normal case and also force a GC to make sure everything works with the normalization before checking the property values? http://codereview.chromium.org/17308 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
