Avoid sharing AccessorPairs during Genesis.

src/bootstrapper.cc

 // Copyright 2012 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:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 154 matching lines @@
   Handle<Context> global_context() { return global_context_; }
 
   // Creates some basic objects. Used for creating a context from scratch.
   void CreateRoots();
   // Creates the empty function.  Used for creating a context from scratch.
   Handle<JSFunction> CreateEmptyFunction(Isolate* isolate);
   // Creates the ThrowTypeError function. ECMA 5th Ed. 13.2.3
   Handle<JSFunction> GetThrowTypeErrorFunction();
 
   void CreateStrictModeFunctionMaps(Handle<JSFunction> empty);
+
+  // Make the "arguments" and "caller" properties throw a TypeError on access.
+  void InitArgumentsAndCaller(Handle<Map> map);

[Michael Starzinger, 2012/02/17 13:33:28]

In ECMA-Script speak this method should better be called
PoisonArgumentsAndCaller().

[Sven Panne, 2012/02/20 08:26:03]

Done.

+
   // Creates the global objects using the global and the template passed in
   // through the API.  We call this regardless of whether we are building a
   // context from scratch or using a deserialized one from the partial snapshot
   // but in the latter case we don't use the objects it produces directly, as
   // we have to used the deserialized ones that are linked together with the
   // rest of the context snapshot.
   Handle<JSGlobalProxy> CreateNewGlobals(
       v8::Handle<v8::ObjectTemplate> global_template,
       Handle<Object> global_object,
       Handle<GlobalObject>* global_proxy_out);
@@ skipping 64 matching lines @@
   };
 
   Handle<Map> CreateFunctionMap(PrototypePropertyMode prototype_mode);
 
   Handle<DescriptorArray> ComputeFunctionInstanceDescriptor(
       PrototypePropertyMode prototypeMode);
   void MakeFunctionInstancePrototypeWritable();
 
   Handle<Map> CreateStrictModeFunctionMap(
       PrototypePropertyMode prototype_mode,
-      Handle<JSFunction> empty_function,
-      Handle<AccessorPair> arguments_callbacks,
-      Handle<AccessorPair> caller_callbacks);
+      Handle<JSFunction> empty_function);
 
   Handle<DescriptorArray> ComputeStrictFunctionInstanceDescriptor(
-      PrototypePropertyMode propertyMode,
-      Handle<AccessorPair> arguments,
-      Handle<AccessorPair> caller);
+      PrototypePropertyMode propertyMode);
 
   static bool CompileBuiltin(Isolate* isolate, int index);
   static bool CompileExperimentalBuiltin(Isolate* isolate, int index);
   static bool CompileNative(Vector<const char> name, Handle<String> source);
   static bool CompileScriptCached(Vector<const char> name,
                                   Handle<String> source,
                                   SourceCodeCache* cache,
                                   v8::Extension* extension,
                                   Handle<Context> top_context,
                                   bool use_runtime_context);
@@ skipping 248 matching lines @@
       function_without_prototype_map);
   empty_fm->set_instance_descriptors(
       function_without_prototype_map->instance_descriptors());
   empty_fm->set_prototype(global_context()->object_function()->prototype());
   empty_function->set_map(*empty_fm);
   return empty_function;
 }
 
 
 Handle<DescriptorArray> Genesis::ComputeStrictFunctionInstanceDescriptor(
-    PrototypePropertyMode prototypeMode,
-    Handle<AccessorPair> arguments,
-    Handle<AccessorPair> caller) {
+    PrototypePropertyMode prototypeMode) {
   Handle<DescriptorArray> descriptors =
       factory()->NewDescriptorArray(prototypeMode == DONT_ADD_PROTOTYPE
                                     ? 4
                                     : 5);
   PropertyAttributes attributes = static_cast<PropertyAttributes>(
       DONT_ENUM | DONT_DELETE);
 
   DescriptorArray::WhitenessWitness witness(*descriptors);
 
   {  // length
     Handle<Foreign> foreign = factory()->NewForeign(&Accessors::FunctionLength);
     CallbacksDescriptor d(*factory()->length_symbol(), *foreign, attributes);
     descriptors->Set(0, &d, witness);
   }
   {  // name
     Handle<Foreign> foreign = factory()->NewForeign(&Accessors::FunctionName);
     CallbacksDescriptor d(*factory()->name_symbol(), *foreign, attributes);
     descriptors->Set(1, &d, witness);
   }
   {  // arguments
+    Handle<AccessorPair> arguments(factory()->NewAccessorPair());

[Michael Starzinger, 2012/02/17 13:33:28]

Could you rewrite this as using the assignment operator so that it is in sync
with the blocks above and below?

[Sven Panne, 2012/02/20 08:26:03]

As discussed offline, I'll do it the other way round. Furthermore, I'll sync
this with the non-strict version of this method.

[Kevin Millikin (Chromium), 2012/02/20 09:30:19]

The existing V8 style is to prefer '=' notation for constructors that are
(essentially) a copy (e.g., the pointer assignment Handle constructors) and
constructor call notation for ones that are doing some sort of allocation or
other work in the constructor itself (e.g., the HandleScope::CreateHandle Handle
constructors).

We've been pretty consistent about that, so you'll have to change a lot of sites
if you think it's worth changing.

     CallbacksDescriptor d(*factory()->arguments_symbol(),
                           *arguments,
                           attributes);
     descriptors->Set(2, &d, witness);
   }
   {  // caller
+    Handle<AccessorPair> caller(factory()->NewAccessorPair());

[Michael Starzinger, 2012/02/17 13:33:28]

Likewise.

[Sven Panne, 2012/02/20 08:26:03]

See above.

     CallbacksDescriptor d(*factory()->caller_symbol(), *caller, attributes);
     descriptors->Set(3, &d, witness);
   }
 
   // prototype
   if (prototypeMode != DONT_ADD_PROTOTYPE) {
     if (prototypeMode != ADD_WRITEABLE_PROTOTYPE) {
       attributes = static_cast<PropertyAttributes>(attributes | READ_ONLY);
     }
     Handle<Foreign> foreign =
@@ skipping 22 matching lines @@
     throw_type_error_function->shared()->DontAdaptArguments();
 
     JSObject::PreventExtensions(throw_type_error_function);
   }
   return throw_type_error_function;
 }
 
 
 Handle<Map> Genesis::CreateStrictModeFunctionMap(
     PrototypePropertyMode prototype_mode,
-    Handle<JSFunction> empty_function,
-    Handle<AccessorPair> arguments_callbacks,
-    Handle<AccessorPair> caller_callbacks) {
+    Handle<JSFunction> empty_function) {
   Handle<Map> map = factory()->NewMap(JS_FUNCTION_TYPE, JSFunction::kSize);
   Handle<DescriptorArray> descriptors =
-      ComputeStrictFunctionInstanceDescriptor(prototype_mode,
-                                              arguments_callbacks,
-                                              caller_callbacks);
+      ComputeStrictFunctionInstanceDescriptor(prototype_mode);
   map->set_instance_descriptors(*descriptors);
   map->set_function_with_prototype(prototype_mode != DONT_ADD_PROTOTYPE);
   map->set_prototype(*empty_function);
   return map;
 }
 
 
 void Genesis::CreateStrictModeFunctionMaps(Handle<JSFunction> empty) {
-  // Create the callbacks arrays for ThrowTypeError functions.
-  // The get/set callacks are filled in after the maps are created below.
-  Factory* factory = empty->GetIsolate()->factory();
-  Handle<AccessorPair> arguments(factory->NewAccessorPair());
-  Handle<AccessorPair> caller(factory->NewAccessorPair());
-
   // Allocate map for the strict mode function instances.
   Handle<Map> strict_mode_function_instance_map =
-      CreateStrictModeFunctionMap(
-          ADD_WRITEABLE_PROTOTYPE, empty, arguments, caller);
+      CreateStrictModeFunctionMap(ADD_WRITEABLE_PROTOTYPE, empty);
   global_context()->set_strict_mode_function_instance_map(
       *strict_mode_function_instance_map);
 
   // Allocate map for the prototype-less strict mode instances.
   Handle<Map> strict_mode_function_without_prototype_map =
-      CreateStrictModeFunctionMap(
-          DONT_ADD_PROTOTYPE, empty, arguments, caller);
+      CreateStrictModeFunctionMap(DONT_ADD_PROTOTYPE, empty);
   global_context()->set_strict_mode_function_without_prototype_map(
       *strict_mode_function_without_prototype_map);
 
   // Allocate map for the strict mode functions. This map is temporary, used
   // only for processing of builtins.
   // Later the map is replaced with writable prototype map, allocated below.
   Handle<Map> strict_mode_function_map =
-      CreateStrictModeFunctionMap(
-          ADD_READONLY_PROTOTYPE, empty, arguments, caller);
+      CreateStrictModeFunctionMap(ADD_READONLY_PROTOTYPE, empty);
   global_context()->set_strict_mode_function_map(
       *strict_mode_function_map);
 
   // The final map for the strict mode functions. Writeable prototype.
   // This map is installed in MakeFunctionInstancePrototypeWritable.
   strict_mode_function_instance_map_writable_prototype_ =
-      CreateStrictModeFunctionMap(
-          ADD_WRITEABLE_PROTOTYPE, empty, arguments, caller);
+      CreateStrictModeFunctionMap(ADD_WRITEABLE_PROTOTYPE, empty);
 
   // Create the ThrowTypeError function instance.
   Handle<JSFunction> throw_function =

[Michael Starzinger, 2012/02/17 13:33:28]

This is now unused, remove it.

[Sven Panne, 2012/02/20 08:26:03]

Done.

       GetThrowTypeErrorFunction();
 
   // Complete the callbacks.
-  arguments->set_getter(*throw_function);
-  arguments->set_setter(*throw_function);
-  caller->set_getter(*throw_function);
-  caller->set_setter(*throw_function);
+  InitArgumentsAndCaller(strict_mode_function_instance_map);
+  InitArgumentsAndCaller(strict_mode_function_without_prototype_map);
+  InitArgumentsAndCaller(strict_mode_function_map);
+  InitArgumentsAndCaller(strict_mode_function_instance_map_writable_prototype_);
 }
 
 
+static void SetAccessors(Handle<Map> map,
+                         Handle<String> name,
+                         Handle<JSFunction> func) {
+  DescriptorArray* descs = map->instance_descriptors();
+  int number = descs->Search(*name);
+  AccessorPair* accessors = AccessorPair::cast(descs->GetValue(number));
+  accessors->set_getter(*func);
+  accessors->set_setter(*func);
+}
+
+
+void Genesis::InitArgumentsAndCaller(Handle<Map> map) {
+  SetAccessors(map, factory()->arguments_symbol(), GetThrowTypeErrorFunction());
+  SetAccessors(map, factory()->caller_symbol(), GetThrowTypeErrorFunction());
+}
+
+
 static void AddToWeakGlobalContextList(Context* context) {
   ASSERT(context->IsGlobalContext());
   Heap* heap = context->GetIsolate()->heap();
 #ifdef DEBUG
   { // NOLINT
     ASSERT(context->get(Context::NEXT_CONTEXT_LINK)->IsUndefined());
     // Check that context is not in the list yet.
     for (Object* current = heap->global_contexts_list();
          !current->IsUndefined();
          current = Context::cast(current)->get(Context::NEXT_CONTEXT_LINK)) {
@@ skipping 1685 matching lines @@
   return from + sizeof(NestingCounterType);
 }
 
 
 // Called when the top-level V8 mutex is destroyed.
 void Bootstrapper::FreeThreadResources() {
   ASSERT(!IsActive());
 }
 
 } }  // namespace v8::internal