LGTM with a couple of nits.
http://codereview.chromium.org/436001/diff/2001/2012 File src/ia32/ic-ia32.cc (right): http://codereview.chromium.org/436001/diff/2001/2012#newcode316 src/ia32/ic-ia32.cc:316: // Array index string: If short enough use cache in length/hash field (ebx). We need to look for formulations like these and get rid of them. http://codereview.chromium.org/436001/diff/2001/2012#newcode323 src/ia32/ic-ia32.cc:323: const int kLengthFieldLimit = I'm getting confused by the naming here. We are talking about the hash field here and not the length field, right? http://codereview.chromium.org/436001/diff/2001/2014 File src/objects.h (right): http://codereview.chromium.org/436001/diff/2001/2014#newcode225 src/objects.h:225: V(SYMBOL_TYPE) \ Please align the '\' on the right as before. http://codereview.chromium.org/436001/diff/2001/2014#newcode298 src/objects.h:298: V(SYMBOL_TYPE, \ Alignment again. http://codereview.chromium.org/436001/diff/2001/2014#newcode3881 src/objects.h:3881: // For strings which are array indexes the hash value has the Complete comment please. I got confused by the ArrayIndexHashLengthShift, but I see from the implementation why it is there. The comment should explain that. http://codereview.chromium.org/436001 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
