Thanks for review! Submitted.
-- Vitaly http://codereview.chromium.org/573003/diff/5002/8006 File src/ia32/stub-cache-ia32.cc (right): http://codereview.chromium.org/573003/diff/5002/8006#newcode803 src/ia32/stub-cache-ia32.cc:803: __ bind(&miss); On 2010/02/08 12:07:38, antonm wrote:
feel free to ignore, but in case when !can_do_fast_api_call this adds unnecessary jump instruction---instead of branching to miss, we could immediately branch to miss_label. Apparently relatively easy to fix,
but up to
you.
Done. http://codereview.chromium.org/573003/diff/5002/8008 File src/stub-cache.h (right): http://codereview.chromium.org/573003/diff/5002/8008#newcode394 src/stub-cache.h:394: Register CheckPrototypes(JSObject* object, On 2010/02/08 08:56:58, Mads Ager wrote:
Add vertical space between the function declarations?
Done. http://codereview.chromium.org/573003/diff/5002/8013 File test/cctest/test-api.cc (right): http://codereview.chromium.org/573003/diff/5002/8013#newcode5796 test/cctest/test-api.cc:5796: static v8::Handle<Value> InterceptorCallICFastApi(Local<String> name, On 2010/02/08 12:07:38, antonm wrote:
I know that's boring, but apparently you didn't cover some cases like invalidation due to overriding in interceptor's holder-cached const
function's
holder chain.
Thanks, added a test. Yeah, it's boring... http://codereview.chromium.org/573003/diff/5002/8013#newcode5851 test/cctest/test-api.cc:5851: GenerateSomeGarbage(); On 2010/02/08 08:56:58, Mads Ager wrote:
Could you document why you generate some extra garbage in all of these
tests? Done. http://codereview.chromium.org/573003/diff/5002/8013#newcode5855 test/cctest/test-api.cc:5855: "for (var i = 0; i < 1000; i++) {" On 2010/02/08 08:56:58, Mads Ager wrote:
I think we use 1000 in other places as well, but do we need that many iterations? It is nice to have the tests run as fast as possible and
still
cover all the cases.
Sure we don't need that many. Reduced iterations by a factor of 10. http://codereview.chromium.org/573003/diff/5002/8013#newcode5858 test/cctest/test-api.cc:5858: CHECK_EQ(42, context->Global()->Get(v8_str("result"))->Int32Value()); On 2010/02/08 12:07:38, antonm wrote:
if you add result as a last line of script, you could use value here,
might be
more convenient
Yup, but I prefer a slightly more uniform way here. http://codereview.chromium.org/573003 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
