LGTM with comments. Dynamic decision of the default elements kind is nifty, I like it :-)
Now that it's January, please change the year to 2012 in all files in this CL.
http://codereview.chromium.org/9050001/diff/6002/src/bootstrapper.cc File src/bootstrapper.cc (right): http://codereview.chromium.org/9050001/diff/6002/src/bootstrapper.cc#newcode2327 src/bootstrapper.cc:2327: context->set_fast_array_element_bias(0); I have a weak preference to use "Smi::FromInt(0)" here, just to make it clear what the "0" is. http://codereview.chromium.org/9050001/diff/6002/src/flag-definitions.h File src/flag-definitions.h (right): http://codereview.chromium.org/9050001/diff/6002/src/flag-definitions.h#newcode123 src/flag-definitions.h:123: DEFINE_bool(smi_only_arrays, true, "tracks arrays with only smi values") Is this change intentional? http://codereview.chromium.org/9050001/diff/6002/src/objects.cc File src/objects.cc (right): http://codereview.chromium.org/9050001/diff/6002/src/objects.cc#newcode1224 src/objects.cc:1224: if (global_context->untransitioned_js_array_map() != NULL && Don't you mean s/NULL/GetHeap()->undefined_value()/? (cf. changes to heap.cc) http://codereview.chromium.org/9050001/diff/6002/src/objects.cc#newcode1245 src/objects.cc:1245: &dummy); nit: indentation http://codereview.chromium.org/9050001/diff/6002/test/mjsunit/elements-kind-fast-element-bias.js File test/mjsunit/elements-kind-fast-element-bias.js (right): http://codereview.chromium.org/9050001/diff/6002/test/mjsunit/elements-kind-fast-element-bias.js#newcode38 test/mjsunit/elements-kind-fast-element-bias.js:38: // Ensure that there is a global bais for not aggressively transitioning Arrays s/bais/bias/ Even better, re-word the comment. Maybe "Test that repeated SMI->FAST transitions are avoided by initializing Arrays with FAST_ELEMENTS right away"? And then I'd add the inverse of the check at the end before messing with the bias: if (support_smi_only_arrays) { assertTrue(%HasFastSmiOnlyElements(new Array())); } http://codereview.chromium.org/9050001/diff/6002/test/mjsunit/elements-kind-smi-only-bias.js File test/mjsunit/elements-kind-smi-only-bias.js (right): http://codereview.chromium.org/9050001/diff/6002/test/mjsunit/elements-kind-smi-only-bias.js#newcode38 test/mjsunit/elements-kind-smi-only-bias.js:38: // Ensure that there is a global bais for not aggressively transitioning Arrays s/bais/bias/ Or re-word: "Test that Arrays are initialized with SMI_ONLY elements when the code benefits from transitioning to FAST_DOUBLE elements". http://codereview.chromium.org/9050001/diff/6002/test/mjsunit/elements-kind.js File test/mjsunit/elements-kind.js (left): http://codereview.chromium.org/9050001/diff/6002/test/mjsunit/elements-kind.js#oldcode42 test/mjsunit/elements-kind.js:42: print("Tests do NOT include smi-only arrays."); intentional removal? http://codereview.chromium.org/9050001/diff/6002/test/mjsunit/elements-kind.js File test/mjsunit/elements-kind.js (right): http://codereview.chromium.org/9050001/diff/6002/test/mjsunit/elements-kind.js#newcode45 test/mjsunit/elements-kind.js:45: // Ensure that there is a global bais for not aggressively transitioning Arrays suggestion: "Ensure that Arrays default to SMI_ONLY elements." http://codereview.chromium.org/9050001/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
