From 0c7af39737d551d6b01c8a095eff9b81fd0e3f2b Mon Sep 17 00:00:00 2001 From: "ager@chromium.org" Date: Tue, 24 Feb 2009 13:11:53 +0000 Subject: [PATCH] Speed up access to global variables from eval scopes. Traverse the surrounding context to figure out if the variable could be global. If the variable could be global we check context extension objects at runtime and use a global LoadIC if no variables have been introduced by eval. Fix crash bug when loading function arguments from inside eval. The shadowed variable in the DYNAMIC_LOCAL case does not rewrite to a slot in that case. Review URL: http://codereview.chromium.org/28027 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1348 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/codegen-arm.cc | 41 ++++-- src/codegen-ia32.cc | 41 ++++-- src/compilation-cache.cc | 33 ++++- src/compilation-cache.h | 15 ++- src/compiler.cc | 23 ++-- src/compiler.h | 1 + src/contexts.cc | 36 ++++++ src/contexts.h | 9 ++ src/objects.cc | 124 ++++++++++++++++--- src/objects.h | 2 + src/runtime.cc | 19 +-- src/scopeinfo.cc | 30 ++++- src/scopeinfo.h | 6 +- src/scopes.cc | 29 +++-- src/scopes.h | 25 +++- test/mjsunit/global-load-from-eval.js | 85 +++++++++++++ test/mjsunit/global-load-from-nested-eval.js | 66 ++++++++++ 17 files changed, 514 insertions(+), 71 deletions(-) create mode 100644 test/mjsunit/global-load-from-eval.js create mode 100644 test/mjsunit/global-load-from-nested-eval.js diff --git a/src/codegen-arm.cc b/src/codegen-arm.cc index 31ff62057..8a47f8c23 100644 --- a/src/codegen-arm.cc +++ b/src/codegen-arm.cc @@ -2022,12 +2022,16 @@ void CodeGenerator::LoadFromSlot(Slot* slot, TypeofState typeof_state) { } else if (slot->var()->mode() == Variable::DYNAMIC_LOCAL) { Slot* potential_slot = slot->var()->local_if_not_shadowed()->slot(); - __ ldr(r0, - ContextSlotOperandCheckExtensions(potential_slot, - r1, - r2, - &slow)); - __ b(&done); + // Only generate the fast case for locals that rewrite to slots. + // This rules out argument loads. + if (potential_slot != NULL) { + __ ldr(r0, + ContextSlotOperandCheckExtensions(potential_slot, + r1, + r2, + &slow)); + __ b(&done); + } } __ bind(&slow); @@ -2074,7 +2078,8 @@ void CodeGenerator::LoadFromGlobalSlotCheckExtensions(Slot* slot, // Check that no extension objects have been created by calls to // eval from the current scope to the global scope. Register context = cp; - for (Scope* s = scope(); s != NULL; s = s->outer_scope()) { + Scope* s = scope(); + while (s != NULL) { if (s->num_heap_slots() > 0) { if (s->calls_eval()) { // Check that extension is NULL. @@ -2089,7 +2094,27 @@ void CodeGenerator::LoadFromGlobalSlotCheckExtensions(Slot* slot, } // If no outer scope calls eval, we do not need to check more // context extensions. - if (!s->outer_scope_calls_eval()) break; + if (!s->outer_scope_calls_eval() || s->is_eval_scope()) break; + s = s->outer_scope(); + } + + if (s->is_eval_scope()) { + Label next, fast; + if (!context.is(tmp)) __ mov(tmp, Operand(context)); + __ bind(&next); + // Terminate at global context. + __ ldr(tmp2, FieldMemOperand(tmp, HeapObject::kMapOffset)); + __ cmp(tmp2, Operand(Factory::global_context_map())); + __ b(eq, &fast); + // Check that extension is NULL. + __ ldr(tmp2, ContextOperand(tmp, Context::EXTENSION_INDEX)); + __ tst(tmp2, tmp2); + __ b(ne, slow); + // Load next context in chain. + __ ldr(tmp, ContextOperand(tmp, Context::CLOSURE_INDEX)); + __ ldr(tmp, FieldMemOperand(tmp, JSFunction::kContextOffset)); + __ b(&next); + __ bind(&fast); } // All extension objects were empty and it is safe to use a global diff --git a/src/codegen-ia32.cc b/src/codegen-ia32.cc index e65215044..e09554108 100644 --- a/src/codegen-ia32.cc +++ b/src/codegen-ia32.cc @@ -2365,11 +2365,15 @@ void CodeGenerator::LoadFromSlot(Slot* slot, TypeofState typeof_state) { } else if (slot->var()->mode() == Variable::DYNAMIC_LOCAL) { Slot* potential_slot = slot->var()->local_if_not_shadowed()->slot(); - __ mov(eax, - ContextSlotOperandCheckExtensions(potential_slot, - ebx, - &slow)); - __ jmp(&done); + // Only generate the fast case for locals that rewrite to slots. + // This rules out argument loads. + if (potential_slot != NULL) { + __ mov(eax, + ContextSlotOperandCheckExtensions(potential_slot, + ebx, + &slow)); + __ jmp(&done); + } } __ bind(&slow); @@ -2414,7 +2418,8 @@ void CodeGenerator::LoadFromGlobalSlotCheckExtensions(Slot* slot, // Check that no extension objects have been created by calls to // eval from the current scope to the global scope. Register context = esi; - for (Scope* s = scope(); s != NULL; s = s->outer_scope()) { + Scope* s = scope(); + while (s != NULL) { if (s->num_heap_slots() > 0) { if (s->calls_eval()) { // Check that extension is NULL. @@ -2427,8 +2432,28 @@ void CodeGenerator::LoadFromGlobalSlotCheckExtensions(Slot* slot, context = tmp; } // If no outer scope calls eval, we do not need to check more - // context extensions. - if (!s->outer_scope_calls_eval()) break; + // context extensions. If we have reached an eval scope, we check + // all extensions from this point. + if (!s->outer_scope_calls_eval() || s->is_eval_scope()) break; + s = s->outer_scope(); + } + + if (s->is_eval_scope()) { + Label next, fast; + if (!context.is(tmp)) __ mov(tmp, Operand(context)); + __ bind(&next); + // Terminate at global context. + __ cmp(FieldOperand(tmp, HeapObject::kMapOffset), + Immediate(Factory::global_context_map())); + __ j(equal, &fast); + // Check that extension is NULL. + __ cmp(ContextOperand(tmp, Context::EXTENSION_INDEX), Immediate(0)); + __ j(not_equal, slow, not_taken); + // Load next context in chain. + __ mov(tmp, ContextOperand(tmp, Context::CLOSURE_INDEX)); + __ mov(tmp, FieldOperand(tmp, JSFunction::kContextOffset)); + __ jmp(&next); + __ bind(&fast); } // All extension objects were empty and it is safe to use a global diff --git a/src/compilation-cache.cc b/src/compilation-cache.cc index bf3262124..d5a00489b 100644 --- a/src/compilation-cache.cc +++ b/src/compilation-cache.cc @@ -102,6 +102,25 @@ static Handle Lookup(Handle source, } +static Handle Lookup(Handle source, + Handle context, + CompilationCache::Entry entry) { + // Make sure not to leak the table into the surrounding handle + // scope. Otherwise, we risk keeping old tables around even after + // having cleared the cache. + Object* result; + { HandleScope scope; + Handle table = GetTable(entry); + result = table->LookupEval(*source, *context); + } + if (result->IsJSFunction()) { + return Handle(JSFunction::cast(result)); + } else { + return Handle::null(); + } +} + + Handle CompilationCache::LookupScript(Handle source, Handle name, int line_offset, @@ -120,9 +139,10 @@ Handle CompilationCache::LookupScript(Handle source, Handle CompilationCache::LookupEval(Handle source, + Handle context, Entry entry) { ASSERT(entry == EVAL_GLOBAL || entry == EVAL_CONTEXTUAL); - Handle result = Lookup(source, entry); + Handle result = Lookup(source, context, entry); if (result.is_null()) { Counters::compilation_cache_misses.Increment(); } else { @@ -142,6 +162,17 @@ void CompilationCache::PutFunction(Handle source, } +void CompilationCache::PutEvalFunction(Handle source, + Handle context, + Entry entry, + Handle boilerplate) { + HandleScope scope; + ASSERT(boilerplate->IsBoilerplate()); + Handle table = GetTable(entry); + CALL_HEAP_FUNCTION_VOID(table->PutEval(*source, *context, *boilerplate)); +} + + Handle CompilationCache::LookupRegExp(Handle source, JSRegExp::Flags flags) { Handle table = GetTable(REGEXP); diff --git a/src/compilation-cache.h b/src/compilation-cache.h index abe61102c..3fc65c272 100644 --- a/src/compilation-cache.h +++ b/src/compilation-cache.h @@ -55,10 +55,11 @@ class CompilationCache { int line_offset, int column_offset); - // Finds the function boilerplate for a source string for - // eval. Returns an empty handle if the cache doesn't contain a - // script for the given source string. + // Finds the function boilerplate for a source string for eval in a + // given context. Returns an empty handle if the cache doesn't + // contain a script for the given source string. static Handle LookupEval(Handle source, + Handle context, Entry entry); // Returns the regexp data associated with the given regexp if it @@ -78,6 +79,14 @@ class CompilationCache { Entry entry, Handle boilerplate); + // Associate the (source, context->closure()->shared(), kind) + // triple with the boilerplate. This may overwrite an existing + // mapping. + static void PutEvalFunction(Handle source, + Handle context, + Entry entry, + Handle boilerplate); + // Clear the cache - also used to initialize the cache at startup. static void Clear(); diff --git a/src/compiler.cc b/src/compiler.cc index 737ec4e09..896e9d4fa 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -40,6 +40,7 @@ namespace v8 { namespace internal { static Handle MakeCode(FunctionLiteral* literal, Handle