From 7d527f857ffb7e9a187a7f7db45a6b9bfee602d9 Mon Sep 17 00:00:00 2001 From: "kmillikin@chromium.org" Date: Tue, 14 Jun 2011 12:16:23 +0000 Subject: [PATCH] Change the representation of catch contexts. Before, they had no extra slots and an extension object with one named property. Now, they use the extension slot for the property name and have an extra slot for the thrown object. This increases the size of the context itself, but removes overall allocation and eliminates a level of indirection. R=ager@chromium.org Review URL: http://codereview.chromium.org/7152002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8277 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/contexts.cc | 48 ++++++++++++------- src/contexts.h | 13 ++++-- src/factory.cc | 5 +- src/factory.h | 3 +- src/full-codegen.cc | 4 +- src/heap.cc | 10 ++-- src/heap.h | 3 +- src/objects.cc | 2 +- src/runtime.cc | 111 ++++++++++++++++++-------------------------- src/runtime.h | 5 +- 10 files changed, 103 insertions(+), 101 deletions(-) diff --git a/src/contexts.cc b/src/contexts.cc index 0c356c1c71..c590d02267 100644 --- a/src/contexts.cc +++ b/src/contexts.cc @@ -98,24 +98,38 @@ Handle Context::Lookup(Handle name, ContextLookupFlags flags, // Check extension/with/global object. if (context->has_extension()) { - Handle extension = Handle(context->extension(), - isolate); - // Context extension objects needs to behave as if they have no - // prototype. So even if we want to follow prototype chains, we - // need to only do a local lookup for context extension objects. - if ((flags & FOLLOW_PROTOTYPE_CHAIN) == 0 || - extension->IsJSContextExtensionObject()) { - *attributes = extension->GetLocalPropertyAttribute(*name); - } else { - *attributes = extension->GetPropertyAttribute(*name); - } - if (*attributes != ABSENT) { - // property found - if (FLAG_trace_contexts) { - PrintF("=> found property in context object %p\n", - reinterpret_cast(*extension)); + if (context->IsCatchContext()) { + // Catch contexts have the variable name in the extension slot. + if (name->Equals(String::cast(context->extension()))) { + if (FLAG_trace_contexts) { + PrintF("=> found in catch context\n"); + } + *index_ = Context::THROWN_OBJECT_INDEX; + *attributes = NONE; + return context; + } + } else { + // Global, function, and with contexts may have an object in the + // extension slot. + Handle extension(JSObject::cast(context->extension()), + isolate); + // Context extension objects needs to behave as if they have no + // prototype. So even if we want to follow prototype chains, we + // need to only do a local lookup for context extension objects. + if ((flags & FOLLOW_PROTOTYPE_CHAIN) == 0 || + extension->IsJSContextExtensionObject()) { + *attributes = extension->GetLocalPropertyAttribute(*name); + } else { + *attributes = extension->GetPropertyAttribute(*name); + } + if (*attributes != ABSENT) { + // property found + if (FLAG_trace_contexts) { + PrintF("=> found property in context object %p\n", + reinterpret_cast(*extension)); + } + return extension; } - return extension; } } diff --git a/src/contexts.h b/src/contexts.h index 94cd630732..f1bebf44bd 100644 --- a/src/contexts.h +++ b/src/contexts.h @@ -181,10 +181,16 @@ class Context: public FixedArray { CLOSURE_INDEX, FCONTEXT_INDEX, PREVIOUS_INDEX, + // The extension slot is used for either the global object (in global + // contexts), eval extension object (function contexts), subject of with + // (with contexts), or the variable name (catch contexts). EXTENSION_INDEX, GLOBAL_INDEX, MIN_CONTEXT_SLOTS, + // This slot holds the thrown value in catch contexts. + THROWN_OBJECT_INDEX = MIN_CONTEXT_SLOTS, + // These slots are only in global contexts. GLOBAL_PROXY_INDEX = MIN_CONTEXT_SLOTS, SECURITY_TOKEN_INDEX, @@ -265,9 +271,9 @@ class Context: public FixedArray { } void set_previous(Context* context) { set(PREVIOUS_INDEX, context); } - bool has_extension() { return unchecked_extension() != NULL; } - JSObject* extension() { return JSObject::cast(unchecked_extension()); } - void set_extension(JSObject* object) { set(EXTENSION_INDEX, object); } + bool has_extension() { return extension() != NULL; } + Object* extension() { return get(EXTENSION_INDEX); } + void set_extension(Object* object) { set(EXTENSION_INDEX, object); } GlobalObject* global() { Object* result = get(GLOBAL_INDEX); @@ -385,7 +391,6 @@ class Context: public FixedArray { private: // Unchecked access to the slots. Object* unchecked_previous() { return get(PREVIOUS_INDEX); } - Object* unchecked_extension() { return get(EXTENSION_INDEX); } #ifdef DEBUG // Bootstrapping-aware type checks. diff --git a/src/factory.cc b/src/factory.cc index ebe57c08ae..2c100a4940 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -250,10 +250,11 @@ Handle Factory::NewFunctionContext(int length, Handle Factory::NewCatchContext(Handle previous, - Handle extension) { + Handle name, + Handle thrown_object) { CALL_HEAP_FUNCTION( isolate(), - isolate()->heap()->AllocateCatchContext(*previous, *extension), + isolate()->heap()->AllocateCatchContext(*previous, *name, *thrown_object), Context); } diff --git a/src/factory.h b/src/factory.h index 19dc7cf55b..2c4aa21657 100644 --- a/src/factory.h +++ b/src/factory.h @@ -151,7 +151,8 @@ class Factory { // Create a catch context. Handle NewCatchContext(Handle previous, - Handle extension); + Handle name, + Handle thrown_object); // Create a 'with' context. Handle NewWithContext(Handle previous, diff --git a/src/full-codegen.cc b/src/full-codegen.cc index f6d673b5c6..9353ab5156 100644 --- a/src/full-codegen.cc +++ b/src/full-codegen.cc @@ -1107,9 +1107,7 @@ void FullCodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) { { Comment cmnt(masm_, "[ Extend catch context"); __ Push(stmt->name()); __ push(result_register()); - __ CallRuntime(Runtime::kCreateCatchExtensionObject, 2); - __ push(result_register()); - __ CallRuntime(Runtime::kPushCatchContext, 1); + __ CallRuntime(Runtime::kPushCatchContext, 2); StoreToFrameField(StandardFrameConstants::kContextOffset, context_register()); } diff --git a/src/heap.cc b/src/heap.cc index 94136734f1..a5f512d8bd 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -3936,9 +3936,12 @@ MaybeObject* Heap::AllocateFunctionContext(int length, JSFunction* function) { MaybeObject* Heap::AllocateCatchContext(Context* previous, - JSObject* extension) { + String* name, + Object* thrown_object) { + STATIC_ASSERT(Context::MIN_CONTEXT_SLOTS == Context::THROWN_OBJECT_INDEX); Object* result; - { MaybeObject* maybe_result = AllocateFixedArray(Context::MIN_CONTEXT_SLOTS); + { MaybeObject* maybe_result = + AllocateFixedArray(Context::MIN_CONTEXT_SLOTS + 1); if (!maybe_result->ToObject(&result)) return maybe_result; } Context* context = reinterpret_cast(result); @@ -3946,8 +3949,9 @@ MaybeObject* Heap::AllocateCatchContext(Context* previous, context->set_closure(previous->closure()); context->set_fcontext(previous->fcontext()); context->set_previous(previous); - context->set_extension(extension); + context->set_extension(name); context->set_global(previous->global()); + context->set(Context::THROWN_OBJECT_INDEX, thrown_object); return context; } diff --git a/src/heap.h b/src/heap.h index 8b0c83367e..521b476517 100644 --- a/src/heap.h +++ b/src/heap.h @@ -648,7 +648,8 @@ class Heap { // Allocate a catch context. MUST_USE_RESULT MaybeObject* AllocateCatchContext(Context* previous, - JSObject* extension); + String* name, + Object* thrown_object); // Allocate a 'with' context. MUST_USE_RESULT MaybeObject* AllocateWithContext(Context* previous, JSObject* extension); diff --git a/src/objects.cc b/src/objects.cc index 871913f64d..4b8a8aa988 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -3230,7 +3230,7 @@ bool JSObject::ReferencesObject(Object* obj) { // Check the context extension if any. if (context->has_extension()) { - return context->extension()->ReferencesObject(obj); + return JSObject::cast(context->extension())->ReferencesObject(obj); } } diff --git a/src/runtime.cc b/src/runtime.cc index ee7fc8ea83..31e31a31ca 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -614,31 +614,6 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetHandler) { } -RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateCatchExtensionObject) { - ASSERT(args.length() == 2); - CONVERT_CHECKED(String, key, args[0]); - Object* value = args[1]; - ASSERT(!value->IsFailure()); - // Create a catch context extension object. - JSFunction* constructor = - isolate->context()->global_context()-> - context_extension_function(); - Object* object; - { MaybeObject* maybe_object = isolate->heap()->AllocateJSObject(constructor); - if (!maybe_object->ToObject(&object)) return maybe_object; - } - // Assign the exception value to the catch variable and make sure - // that the catch variable is DontDelete. - { MaybeObject* maybe_value = - // Passing non-strict per ECMA-262 5th Ed. 12.14. Catch, bullet #4. - JSObject::cast(object)->SetProperty( - key, value, DONT_DELETE, kNonStrictMode); - if (!maybe_value->ToObject(&value)) return maybe_value; - } - return object; -} - - RUNTIME_FUNCTION(MaybeObject*, Runtime_ClassOf) { NoHandleAllocation ha; ASSERT(args.length() == 1); @@ -1310,7 +1285,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DeclareContextSlot) { Handle context_ext; if (context->has_extension()) { // The function context's extension context exists - use it. - context_ext = Handle(context->extension()); + context_ext = Handle(JSObject::cast(context->extension())); } else { // The function context's extension context does not exists - allocate // it. @@ -7942,12 +7917,14 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_PushWithContext) { RUNTIME_FUNCTION(MaybeObject*, Runtime_PushCatchContext) { NoHandleAllocation ha; - ASSERT(args.length() == 1); - JSObject* extension_object = JSObject::cast(args[0]); + ASSERT(args.length() == 2); + String* name = String::cast(args[0]); + Object* thrown_object = args[1]; Context* context; MaybeObject* maybe_context = isolate->heap()->AllocateCatchContext(isolate->context(), - extension_object); + name, + thrown_object); if (!maybe_context->To(&context)) return maybe_context; isolate->set_context(context); return context; @@ -10200,6 +10177,23 @@ static Handle MaterializeClosure(Isolate* isolate, } +// Create a plain JSObject which materializes the scope for the specified +// catch context. +static Handle MaterializeCatchScope(Isolate* isolate, + Handle context) { + ASSERT(context->IsCatchContext()); + Handle name(String::cast(context->extension())); + Handle thrown_object(context->get(Context::THROWN_OBJECT_INDEX)); + Handle catch_scope = + isolate->factory()->NewJSObject(isolate->object_function()); + RETURN_IF_EMPTY_HANDLE_VALUE( + isolate, + SetProperty(catch_scope, name, thrown_object, NONE, kNonStrictMode), + Handle()); + return catch_scope; +} + + // Iterate over the actual scopes visible from a stack frame. All scopes are // backed by an actual context except the local scope, which is inserted // "artifically" in the context chain. @@ -10210,10 +10204,6 @@ class ScopeIterator { ScopeTypeLocal, ScopeTypeWith, ScopeTypeClosure, - // Every catch block contains an implicit with block (its parameter is - // a JSContextExtensionObject) that extends current scope with a variable - // holding exception object. Such with blocks are treated as scopes of their - // own type. ScopeTypeCatch }; @@ -10291,12 +10281,7 @@ class ScopeIterator { return ScopeTypeClosure; } ASSERT(context_->has_extension()); - // Current scope is either an explicit with statement or a with statement - // implicitely generated for a catch block. - // If the extension object here is a JSContextExtensionObject then - // current with statement is one frome a catch block otherwise it's a - // regular with statement. - if (context_->extension()->IsJSContextExtensionObject()) { + if (context_->IsCatchContext()) { return ScopeTypeCatch; } return ScopeTypeWith; @@ -10307,20 +10292,17 @@ class ScopeIterator { switch (Type()) { case ScopeIterator::ScopeTypeGlobal: return Handle(CurrentContext()->global()); - break; case ScopeIterator::ScopeTypeLocal: // Materialize the content of the local scope into a JSObject. return MaterializeLocalScope(isolate_, frame_); - break; case ScopeIterator::ScopeTypeWith: - case ScopeIterator::ScopeTypeCatch: // Return the with object. - return Handle(CurrentContext()->extension()); - break; + return Handle(JSObject::cast(CurrentContext()->extension())); + case ScopeIterator::ScopeTypeCatch: + return MaterializeCatchScope(isolate_, CurrentContext()); case ScopeIterator::ScopeTypeClosure: // Materialize the content of the closure scope into a JSObject. return MaterializeClosure(isolate_, CurrentContext()); - break; } UNREACHABLE(); return Handle(); @@ -10351,8 +10333,7 @@ class ScopeIterator { if (!CurrentContext().is_null()) { CurrentContext()->Print(); if (CurrentContext()->has_extension()) { - Handle extension = - Handle(CurrentContext()->extension()); + Handle extension(CurrentContext()->extension()); if (extension->IsJSContextExtensionObject()) { extension->Print(); } @@ -10361,34 +10342,27 @@ class ScopeIterator { break; } - case ScopeIterator::ScopeTypeWith: { + case ScopeIterator::ScopeTypeWith: PrintF("With:\n"); - Handle extension = - Handle(CurrentContext()->extension()); - extension->Print(); + CurrentContext()->extension()->Print(); break; - } - case ScopeIterator::ScopeTypeCatch: { + case ScopeIterator::ScopeTypeCatch: PrintF("Catch:\n"); - Handle extension = - Handle(CurrentContext()->extension()); - extension->Print(); + CurrentContext()->extension()->Print(); + CurrentContext()->get(Context::THROWN_OBJECT_INDEX)->Print(); break; - } - case ScopeIterator::ScopeTypeClosure: { + case ScopeIterator::ScopeTypeClosure: PrintF("Closure:\n"); CurrentContext()->Print(); if (CurrentContext()->has_extension()) { - Handle extension = - Handle(CurrentContext()->extension()); + Handle extension(CurrentContext()->extension()); if (extension->IsJSContextExtensionObject()) { extension->Print(); } } break; - } default: UNREACHABLE(); @@ -10867,10 +10841,17 @@ static Handle CopyWithContextChain(Isolate* isolate, HandleScope scope(isolate); Handle previous(current->previous()); Handle new_previous = CopyWithContextChain(isolate, previous, base); - Handle extension(JSObject::cast(current->extension())); - Handle new_current = current->IsCatchContext() - ? isolate->factory()->NewCatchContext(new_previous, extension) - : isolate->factory()->NewWithContext(new_previous, extension); + Handle new_current; + if (current->IsCatchContext()) { + Handle name(String::cast(current->extension())); + Handle thrown_object(current->get(Context::THROWN_OBJECT_INDEX)); + new_current = + isolate->factory()->NewCatchContext(new_previous, name, thrown_object); + } else { + Handle extension(JSObject::cast(current->extension())); + new_current = + isolate->factory()->NewWithContext(new_previous, extension); + } return scope.CloseAndEscape(new_current); } diff --git a/src/runtime.h b/src/runtime.h index 4e90532c2c..cb5bb74f7a 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -283,9 +283,6 @@ namespace internal { F(IsJSProxy, 1, 1) \ F(GetHandler, 1, 1) \ \ - /* Catch context extension objects */ \ - F(CreateCatchExtensionObject, 2, 1) \ - \ /* Statements */ \ F(NewClosure, 3, 1) \ F(NewObject, 1, 1) \ @@ -300,7 +297,7 @@ namespace internal { /* Contexts */ \ F(NewFunctionContext, 1, 1) \ F(PushWithContext, 1, 1) \ - F(PushCatchContext, 1, 1) \ + F(PushCatchContext, 2, 1) \ F(DeleteContextSlot, 2, 1) \ F(LoadContextSlot, 2, 2) \ F(LoadContextSlotNoReferenceError, 2, 2) \