http://codereview.chromium.org/409007/diff/1/2 File src/heap.cc (right):
http://codereview.chromium.org/409007/diff/1/2#newcode1785 Line 1785: if ((c1 - '0' > '9' - '0' || On 2009/11/19 21:42:57, Søren Gjesse wrote: > Why c1 - '0' > '9' - '0' instead of c1 > '9'? I moved this to a different function so I could point out in a comment that the variables are unsigned and thus what you suggest would be completely different. http://codereview.chromium.org/409007/diff/1/2#newcode1878 Line 1878: } else if (length == 2) { On 2009/11/19 21:42:57, Søren Gjesse wrote: > Refactor duplicated code. Done. http://codereview.chromium.org/409007/diff/1/6 File src/objects-inl.h (right): http://codereview.chromium.org/409007/diff/1/6#newcode3113 Line 3113: const int kArraySizeThatFitsComfortablyInNewSpace = 128; On 2009/11/19 21:42:57, Søren Gjesse wrote: > This does not take the calculation required_size + (required_size >> 3) in > Expand into account... Since it's a number I pulled out of my backside I don't think that matters. http://codereview.chromium.org/409007/diff/1/6#newcode3114 Line 3114: if (required_size < kArraySizeThatFitsComfortablyInNewSpace) { On 2009/11/19 21:42:57, Søren Gjesse wrote: > Maybe combine these two if's into one if condition. Also make a note that > "Expand" will always allocate a new backing store even if it is not expanding. Done. http://codereview.chromium.org/409007/diff/1/3 File src/objects.cc (right): http://codereview.chromium.org/409007/diff/1/3#newcode420 Line 420: // constantly reallocating them. On 2009/11/19 21:42:57, Søren Gjesse wrote: > This code is duplicated below, please refactor. Instead I removed this occurrence since it is very rarely hit. http://codereview.chromium.org/409007/diff/1/3#newcode7350 Line 7350: : c1_(c1), c2_(c2) { On 2009/11/19 21:42:57, Søren Gjesse wrote: > How about ASSERT(c1 > '9' || c2 > '9') here? I see that the ASSERT_EQ below will > catch it, but it might be a bit obscure why. Added a comment to the ASSERT_EQ below instead. http://codereview.chromium.org/409007 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
