From b153dcfebf111e673fdc7050d0d59f88942029f6 Mon Sep 17 00:00:00 2001 From: "keuchel@chromium.org" Date: Mon, 14 Nov 2011 08:58:47 +0000 Subject: [PATCH] Make eval compilation cache calling scope sensitive. Review URL: http://codereview.chromium.org/8518001 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9984 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/full-codegen-arm.cc | 6 +++- src/compilation-cache.cc | 44 ++++++++++++++--------- src/compilation-cache.h | 34 +++++++++++++----- src/compiler.cc | 10 +++--- src/compiler.h | 3 +- src/ia32/full-codegen-ia32.cc | 5 ++- src/mips/full-codegen-mips.cc | 6 +++- src/objects.cc | 67 +++++++++++++++++++++-------------- src/objects.h | 8 +++-- src/runtime.cc | 27 ++++++++------ src/runtime.h | 2 +- src/x64/full-codegen-x64.cc | 5 ++- 12 files changed, 145 insertions(+), 72 deletions(-) diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index e8a2cb07f2..fcc0bd5813 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -2204,7 +2204,11 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) { __ mov(r1, Operand(Smi::FromInt(strict_mode))); __ push(r1); - __ CallRuntime(Runtime::kResolvePossiblyDirectEval, 4); + // Push the start position of the scope the calls resides in. + __ mov(r1, Operand(Smi::FromInt(scope()->start_position()))); + __ push(r1); + + __ CallRuntime(Runtime::kResolvePossiblyDirectEval, 5); } diff --git a/src/compilation-cache.cc b/src/compilation-cache.cc index 28e833a493..517e2c398a 100644 --- a/src/compilation-cache.cc +++ b/src/compilation-cache.cc @@ -1,4 +1,4 @@ -// Copyright 2008 the V8 project authors. All rights reserved. +// Copyright 2011 the V8 project authors. All rights reserved. // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions are // met: @@ -27,6 +27,7 @@ #include "v8.h" +#include "assembler.h" #include "compilation-cache.h" #include "serialize.h" @@ -250,7 +251,8 @@ void CompilationCacheScript::Put(Handle source, Handle CompilationCacheEval::Lookup( Handle source, Handle context, - StrictModeFlag strict_mode) { + StrictModeFlag strict_mode, + int scope_position) { // 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. @@ -259,7 +261,8 @@ Handle CompilationCacheEval::Lookup( { HandleScope scope(isolate()); for (generation = 0; generation < generations(); generation++) { Handle table = GetTable(generation); - result = table->LookupEval(*source, *context, strict_mode); + result = table->LookupEval( + *source, *context, strict_mode, scope_position); if (result->IsSharedFunctionInfo()) { break; } @@ -269,7 +272,7 @@ Handle CompilationCacheEval::Lookup( Handle function_info(SharedFunctionInfo::cast(result), isolate()); if (generation != 0) { - Put(source, context, function_info); + Put(source, context, function_info, scope_position); } isolate()->counters()->compilation_cache_hits()->Increment(); return function_info; @@ -283,27 +286,31 @@ Handle CompilationCacheEval::Lookup( MaybeObject* CompilationCacheEval::TryTablePut( Handle source, Handle context, - Handle function_info) { + Handle function_info, + int scope_position) { Handle table = GetFirstTable(); - return table->PutEval(*source, *context, *function_info); + return table->PutEval(*source, *context, *function_info, scope_position); } Handle CompilationCacheEval::TablePut( Handle source, Handle context, - Handle function_info) { + Handle function_info, + int scope_position) { CALL_HEAP_FUNCTION(isolate(), - TryTablePut(source, context, function_info), + TryTablePut( + source, context, function_info, scope_position), CompilationCacheTable); } void CompilationCacheEval::Put(Handle source, Handle context, - Handle function_info) { + Handle function_info, + int scope_position) { HandleScope scope(isolate()); - SetFirstTable(TablePut(source, context, function_info)); + SetFirstTable(TablePut(source, context, function_info, scope_position)); } @@ -389,16 +396,19 @@ Handle CompilationCache::LookupEval( Handle source, Handle context, bool is_global, - StrictModeFlag strict_mode) { + StrictModeFlag strict_mode, + int scope_position) { if (!IsEnabled()) { return Handle::null(); } Handle result; if (is_global) { - result = eval_global_.Lookup(source, context, strict_mode); + result = eval_global_.Lookup(source, context, strict_mode, scope_position); } else { - result = eval_contextual_.Lookup(source, context, strict_mode); + ASSERT(scope_position != RelocInfo::kNoPosition); + result = eval_contextual_.Lookup( + source, context, strict_mode, scope_position); } return result; } @@ -427,16 +437,18 @@ void CompilationCache::PutScript(Handle source, void CompilationCache::PutEval(Handle source, Handle context, bool is_global, - Handle function_info) { + Handle function_info, + int scope_position) { if (!IsEnabled()) { return; } HandleScope scope(isolate()); if (is_global) { - eval_global_.Put(source, context, function_info); + eval_global_.Put(source, context, function_info, scope_position); } else { - eval_contextual_.Put(source, context, function_info); + ASSERT(scope_position != RelocInfo::kNoPosition); + eval_contextual_.Put(source, context, function_info, scope_position); } } diff --git a/src/compilation-cache.h b/src/compilation-cache.h index 4339d22641..009afe6e00 100644 --- a/src/compilation-cache.h +++ b/src/compilation-cache.h @@ -1,4 +1,4 @@ -// Copyright 2008 the V8 project authors. All rights reserved. +// Copyright 2011 the V8 project authors. All rights reserved. // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions are // met: @@ -123,7 +123,19 @@ class CompilationCacheScript : public CompilationSubCache { }; -// Sub-cache for eval scripts. +// Sub-cache for eval scripts. Two caches for eval are used. One for eval calls +// in global contexts and one for eval calls in other contexts. The cache +// considers the following pieces of information when checking for matching +// entries: +// 1. The source string. +// 2. The shared function info of the calling function. +// 3. Whether the source should be compiled as strict code or as non-strict +// code. +// Note: Currently there are clients of CompileEval that always compile +// non-strict code even if the calling function is a strict mode function. +// More specifically these are the CompileString, DebugEvaluate and +// DebugEvaluateGlobal runtime functions. +// 4. The start position of the calling scope. class CompilationCacheEval: public CompilationSubCache { public: CompilationCacheEval(Isolate* isolate, int generations) @@ -131,23 +143,27 @@ class CompilationCacheEval: public CompilationSubCache { Handle Lookup(Handle source, Handle context, - StrictModeFlag strict_mode); + StrictModeFlag strict_mode, + int scope_position); void Put(Handle source, Handle context, - Handle function_info); + Handle function_info, + int scope_position); private: MUST_USE_RESULT MaybeObject* TryTablePut( Handle source, Handle context, - Handle function_info); + Handle function_info, + int scope_position); // Note: Returns a new hash table if operation results in expansion. Handle TablePut( Handle source, Handle context, - Handle function_info); + Handle function_info, + int scope_position); DISALLOW_IMPLICIT_CONSTRUCTORS(CompilationCacheEval); }; @@ -198,7 +214,8 @@ class CompilationCache { Handle LookupEval(Handle source, Handle context, bool is_global, - StrictModeFlag strict_mode); + StrictModeFlag strict_mode, + int scope_position); // Returns the regexp data associated with the given regexp if it // is in cache, otherwise an empty handle. @@ -215,7 +232,8 @@ class CompilationCache { void PutEval(Handle source, Handle context, bool is_global, - Handle function_info); + Handle function_info, + int scope_position); // Associate the (source, flags) pair to the given regexp data. // This may overwrite an existing mapping. diff --git a/src/compiler.cc b/src/compiler.cc index 63d783145b..1e90469a92 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -533,7 +533,8 @@ Handle Compiler::Compile(Handle source, Handle Compiler::CompileEval(Handle source, Handle context, bool is_global, - StrictModeFlag strict_mode) { + StrictModeFlag strict_mode, + int scope_position) { Isolate* isolate = source->GetIsolate(); int source_length = source->length(); isolate->counters()->total_eval_size()->Increment(source_length); @@ -549,7 +550,8 @@ Handle Compiler::CompileEval(Handle source, result = compilation_cache->LookupEval(source, context, is_global, - strict_mode); + strict_mode, + scope_position); if (result.is_null()) { // Create a script object describing the script to be compiled. @@ -561,13 +563,13 @@ Handle Compiler::CompileEval(Handle source, info.SetCallingContext(context); result = MakeFunctionInfo(&info); if (!result.is_null()) { - CompilationCache* compilation_cache = isolate->compilation_cache(); // If caller is strict mode, the result must be strict as well, // but not the other way around. Consider: // eval("'use strict'; ..."); // TODO(keuchel): adapt this for extended mode. ASSERT(strict_mode == kNonStrictMode || result->strict_mode()); - compilation_cache->PutEval(source, context, is_global, result); + compilation_cache->PutEval( + source, context, is_global, result, scope_position); } } diff --git a/src/compiler.h b/src/compiler.h index bedf5eebb9..ecf112072c 100644 --- a/src/compiler.h +++ b/src/compiler.h @@ -289,7 +289,8 @@ class Compiler : public AllStatic { static Handle CompileEval(Handle source, Handle context, bool is_global, - StrictModeFlag strict_mode); + StrictModeFlag strict_mode, + int scope_position); // Compile from function info (used for lazy compilation). Returns true on // success and false if the compilation resulted in a stack overflow. diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 7ad1173043..28e5443772 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -2152,7 +2152,10 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) { FLAG_harmony_scoping ? kStrictMode : strict_mode_flag(); __ push(Immediate(Smi::FromInt(strict_mode))); - __ CallRuntime(Runtime::kResolvePossiblyDirectEval, 4); + // Push the start position of the scope the calls resides in. + __ push(Immediate(Smi::FromInt(scope()->start_position()))); + + __ CallRuntime(Runtime::kResolvePossiblyDirectEval, 5); } diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc index 555cad9899..a4b87664bb 100644 --- a/src/mips/full-codegen-mips.cc +++ b/src/mips/full-codegen-mips.cc @@ -2271,7 +2271,11 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) { __ li(a1, Operand(Smi::FromInt(strict_mode))); __ push(a1); - __ CallRuntime(Runtime::kResolvePossiblyDirectEval, 4); + // Push the start position of the scope the calls resides in. + __ li(a1, Operand(Smi::FromInt(scope()->start_position()))); + __ push(a1); + + __ CallRuntime(Runtime::kResolvePossiblyDirectEval, 5); } diff --git a/src/objects.cc b/src/objects.cc index 9a2319a81b..ca8714a63d 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -10298,74 +10298,83 @@ class StringSharedKey : public HashTableKey { public: StringSharedKey(String* source, SharedFunctionInfo* shared, - StrictModeFlag strict_mode) + StrictModeFlag strict_mode, + int scope_position) : source_(source), shared_(shared), - strict_mode_(strict_mode) { } + strict_mode_(strict_mode), + scope_position_(scope_position) { } bool IsMatch(Object* other) { if (!other->IsFixedArray()) return false; - FixedArray* pair = FixedArray::cast(other); - SharedFunctionInfo* shared = SharedFunctionInfo::cast(pair->get(0)); + FixedArray* other_array = FixedArray::cast(other); + SharedFunctionInfo* shared = SharedFunctionInfo::cast(other_array->get(0)); if (shared != shared_) return false; - int strict_unchecked = Smi::cast(pair->get(2))->value(); + int strict_unchecked = Smi::cast(other_array->get(2))->value(); ASSERT(strict_unchecked == kStrictMode || strict_unchecked == kNonStrictMode); StrictModeFlag strict_mode = static_cast(strict_unchecked); if (strict_mode != strict_mode_) return false; - String* source = String::cast(pair->get(1)); + int scope_position = Smi::cast(other_array->get(3))->value(); + if (scope_position != scope_position_) return false; + String* source = String::cast(other_array->get(1)); return source->Equals(source_); } static uint32_t StringSharedHashHelper(String* source, SharedFunctionInfo* shared, - StrictModeFlag strict_mode) { + StrictModeFlag strict_mode, + int scope_position) { uint32_t hash = source->Hash(); if (shared->HasSourceCode()) { // Instead of using the SharedFunctionInfo pointer in the hash // code computation, we use a combination of the hash of the - // script source code and the start and end positions. We do - // this to ensure that the cache entries can survive garbage + // script source code and the start position of the calling scope. + // We do this to ensure that the cache entries can survive garbage // collection. Script* script = Script::cast(shared->script()); hash ^= String::cast(script->source())->Hash(); if (strict_mode == kStrictMode) hash ^= 0x8000; - hash += shared->start_position(); + hash += scope_position; } return hash; } uint32_t Hash() { - return StringSharedHashHelper(source_, shared_, strict_mode_); + return StringSharedHashHelper( + source_, shared_, strict_mode_, scope_position_); } uint32_t HashForObject(Object* obj) { - FixedArray* pair = FixedArray::cast(obj); - SharedFunctionInfo* shared = SharedFunctionInfo::cast(pair->get(0)); - String* source = String::cast(pair->get(1)); - int strict_unchecked = Smi::cast(pair->get(2))->value(); + FixedArray* other_array = FixedArray::cast(obj); + SharedFunctionInfo* shared = SharedFunctionInfo::cast(other_array->get(0)); + String* source = String::cast(other_array->get(1)); + int strict_unchecked = Smi::cast(other_array->get(2))->value(); ASSERT(strict_unchecked == kStrictMode || strict_unchecked == kNonStrictMode); StrictModeFlag strict_mode = static_cast(strict_unchecked); - return StringSharedHashHelper(source, shared, strict_mode); + int scope_position = Smi::cast(other_array->get(3))->value(); + return StringSharedHashHelper(source, shared, strict_mode, scope_position); } MUST_USE_RESULT MaybeObject* AsObject() { Object* obj; - { MaybeObject* maybe_obj = source_->GetHeap()->AllocateFixedArray(3); + { MaybeObject* maybe_obj = source_->GetHeap()->AllocateFixedArray(4); if (!maybe_obj->ToObject(&obj)) return maybe_obj; } - FixedArray* pair = FixedArray::cast(obj); - pair->set(0, shared_); - pair->set(1, source_); - pair->set(2, Smi::FromInt(strict_mode_)); - return pair; + FixedArray* other_array = FixedArray::cast(obj); + other_array->set(0, shared_); + other_array->set(1, source_); + other_array->set(2, Smi::FromInt(strict_mode_)); + other_array->set(3, Smi::FromInt(scope_position_)); + return other_array; } private: String* source_; SharedFunctionInfo* shared_; StrictModeFlag strict_mode_; + int scope_position_; }; @@ -11520,8 +11529,12 @@ Object* CompilationCacheTable::Lookup(String* src) { Object* CompilationCacheTable::LookupEval(String* src, Context* context, - StrictModeFlag strict_mode) { - StringSharedKey key(src, context->closure()->shared(), strict_mode); + StrictModeFlag strict_mode, + int scope_position) { + StringSharedKey key(src, + context->closure()->shared(), + strict_mode, + scope_position); int entry = FindEntry(&key); if (entry == kNotFound) return GetHeap()->undefined_value(); return get(EntryToIndex(entry) + 1); @@ -11556,10 +11569,12 @@ MaybeObject* CompilationCacheTable::Put(String* src, Object* value) { MaybeObject* CompilationCacheTable::PutEval(String* src, Context* context, - SharedFunctionInfo* value) { + SharedFunctionInfo* value, + int scope_position) { StringSharedKey key(src, context->closure()->shared(), - value->strict_mode_flag()); + value->strict_mode_flag(), + scope_position); Object* obj; { MaybeObject* maybe_obj = EnsureCapacity(1, &key); if (!maybe_obj->ToObject(&obj)) return maybe_obj; diff --git a/src/objects.h b/src/objects.h index fbaee6b953..a7554d28be 100644 --- a/src/objects.h +++ b/src/objects.h @@ -5849,12 +5849,16 @@ class CompilationCacheTable: public HashTable shared = Compiler::CompileEval(source, - context, - true, - kNonStrictMode); + Handle shared = Compiler::CompileEval( + source, context, true, kNonStrictMode, RelocInfo::kNoPosition); if (shared.is_null()) return Failure::Exception(); Handle fun = isolate->factory()->NewFunctionFromSharedFunctionInfo(shared, @@ -9493,7 +9491,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CompileString) { static ObjectPair CompileGlobalEval(Isolate* isolate, Handle source, Handle receiver, - StrictModeFlag strict_mode) { + StrictModeFlag strict_mode, + int scope_position) { Handle context = Handle(isolate->context()); Handle global_context = Handle(context->global_context()); @@ -9511,7 +9510,8 @@ static ObjectPair CompileGlobalEval(Isolate* isolate, source, Handle(isolate->context()), context->IsGlobalContext(), - strict_mode); + strict_mode, + scope_position); if (shared.is_null()) return MakePair(Failure::Exception(), NULL); Handle compiled = isolate->factory()->NewFunctionFromSharedFunctionInfo( @@ -9521,7 +9521,7 @@ static ObjectPair CompileGlobalEval(Isolate* isolate, RUNTIME_FUNCTION(ObjectPair, Runtime_ResolvePossiblyDirectEval) { - ASSERT(args.length() == 4); + ASSERT(args.length() == 5); HandleScope scope(isolate); Handle callee = args.at(0); @@ -9537,10 +9537,12 @@ RUNTIME_FUNCTION(ObjectPair, Runtime_ResolvePossiblyDirectEval) { } CONVERT_STRICT_MODE_ARG(strict_mode, 3); + ASSERT(args[4]->IsSmi()); return CompileGlobalEval(isolate, args.at(1), args.at(2), - strict_mode); + strict_mode, + args.smi_at(4)); } @@ -12154,7 +12156,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) { Compiler::CompileEval(function_source, context, context->IsGlobalContext(), - kNonStrictMode); + kNonStrictMode, + RelocInfo::kNoPosition); if (shared.is_null()) return Failure::Exception(); Handle compiled_function = isolate->factory()->NewFunctionFromSharedFunctionInfo(shared, context); @@ -12247,7 +12250,11 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluateGlobal) { // Currently, the eval code will be executed in non-strict mode, // even in the strict code context. Handle shared = - Compiler::CompileEval(source, context, is_global, kNonStrictMode); + Compiler::CompileEval(source, + context, + is_global, + kNonStrictMode, + RelocInfo::kNoPosition); if (shared.is_null()) return Failure::Exception(); Handle compiled_function = Handle( diff --git a/src/runtime.h b/src/runtime.h index 581a6ab46a..ef172d3266 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -259,7 +259,7 @@ namespace internal { \ /* Eval */ \ F(GlobalReceiver, 1, 1) \ - F(ResolvePossiblyDirectEval, 4, 2) \ + F(ResolvePossiblyDirectEval, 5, 2) \ \ F(SetProperty, -1 /* 4 or 5 */, 1) \ F(DefineOrRedefineDataProperty, 4, 1) \ diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index bb6407af2b..587f73b882 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -2096,7 +2096,10 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) { FLAG_harmony_scoping ? kStrictMode : strict_mode_flag(); __ Push(Smi::FromInt(strict_mode)); - __ CallRuntime(Runtime::kResolvePossiblyDirectEval, 4); + // Push the start position of the scope the calls resides in. + __ Push(Smi::FromInt(scope()->start_position())); + + __ CallRuntime(Runtime::kResolvePossiblyDirectEval, 5); }