LGTM You should add some tests though. I'm not sure that any of our test code hits this. Please create a number of tests for different kinds of top-level assignment sequences.
http://codereview.chromium.org/27128/diff/1/6 File src/codegen-ia32.cc (right): http://codereview.chromium.org/27128/diff/1/6#newcode2764 Line 2764: // Changing to slow case in the beginning of an initialization block Change to slow case ... to avoid ... or Changing to slow case ... avoids ... ? http://codereview.chromium.org/27128/diff/1/5 File src/parser.cc (right): http://codereview.chromium.org/27128/diff/1/5#newcode74 Line 74: static const int kMinInitializationBlock = 3; Is this limit based on benchmarking? We should make sure to only perform the two runtime calls if that overhead is less than the instance descriptor copying overhead. http://codereview.chromium.org/27128/diff/1/5#newcode1242 Line 1242: if (!InBlock() && (assignment != NULL)) StartBlock(assignment); In BlockContinues, you check if the operation is Token::ASSIGN. You should do that here (or in StartBlock) as well? Please add a test case that hits this. http://codereview.chromium.org/27128/diff/1/5#newcode1345 Line 1345: // degradation in any other scopes. You should write here that the reason for only doing this in top level code is that map transitions are not used in this case leading to different maps for all objects going through this series of assignments. http://codereview.chromium.org/27128 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
