Move all Symbol.species setup for builtin constructors to bootstrapper

Move all Symbol.species setup for builtin constructors to bootstrapper

This lets us share Builtins::kReturnReceiver, so should
every-so-slightly reduce the size of the context snapshot.

R=gsathya@chromium.org

Review-Url: https://codereview.chromium.org/2614623003
Cr-Commit-Position: refs/heads/master@{#42076}
Committed: https://chromium.googlesource.com/v8/v8/+/e968595b42f8d49bf88fe3181cbdf8568c0628f6

[gsathya, 2017-01-04 22:11:30]

lgtm

https://codereview.chromium.org/2614623003/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2614623003/diff/1/src/bootstrapper.cc#newcode517
src/bootstrapper.cc:517: Factory* factory =
constructor->GetIsolate()->factory();
Why not just pass the isolate to this function? The rest of the Install*
functions seem to do that.

[adamk, 2017-01-04 22:14:29]

https://codereview.chromium.org/2614623003/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2614623003/diff/1/src/bootstrapper.cc#newcode517
src/bootstrapper.cc:517: Factory* factory =
constructor->GetIsolate()->factory();
On 2017/01/04 22:11:30, gsathya wrote:
> Why not just pass the isolate to this function? The rest of the Install*
> functions seem to do that.

SimpleInstallGetter doesn't take an Isolate, so we're already "paying" for a
GetIsolate() call there. Given that this code doesn't ever run in production
(since it's part of the snapshot) I don't think that micro-opt should metter
here.

https://codereview.chromium.org/2614623003/diff/1/src/bootstrapper.cc#newcode518
src/bootstrapper.cc:518: SimpleInstallGetter(constructor,
factory->symbol_species_string(),
But speaking of micro-opts, it seems like I could make this even faster by
sharing a single SharedFunctionInfo for all these functions...

[adamk, 2017-01-04 22:25:24]

The CQ bit was checked by adamk@chromium.org

[adamk, 2017-01-04 22:25:24]

The patchset sent to the CQ was uploaded after l-g-t-m from gsathya@chromium.org
Link to the patchset: https://codereview.chromium.org/2614623003/#ps20001
(title: "Add TODO")

[commit-bot: I haz the power, 2017-01-04 22:25:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 22:50:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-04 22:50:47]

Failed to apply patch for src/bootstrapper.cc:
While running git apply --index -p1;
  error: patch failed: src/bootstrapper.cc:1884
  error: src/bootstrapper.cc: patch does not apply

Patch:       src/bootstrapper.cc
Index: src/bootstrapper.cc
diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index
2aa4e384a8fe96152a241cd05c3ba6559ea22ad2..b79954d1f583cdf5b601a16229897a979c7435a4
100644
--- a/src/bootstrapper.cc
+++ b/src/bootstrapper.cc
@@ -513,6 +513,15 @@ void InstallConstant(Isolate* isolate, Handle<JSObject>
holder,
       static_cast<PropertyAttributes>(DONT_DELETE | DONT_ENUM | READ_ONLY));
 }
 
+void InstallSpeciesGetter(Handle<JSFunction> constructor) {
+  Factory* factory = constructor->GetIsolate()->factory();
+  // TODO(adamk): We should be able to share a SharedFunctionInfo
+  // between all these JSFunctins.
+  SimpleInstallGetter(constructor, factory->symbol_species_string(),
+                      factory->species_symbol(), Builtins::kReturnReceiver,
+                      true);
+}
+
 }  // namespace
 
 Handle<JSFunction> Genesis::CreateEmptyFunction(Isolate* isolate) {
@@ -1310,6 +1319,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject>
global_object,
 
     InstallWithIntrinsicDefaultProto(isolate, array_function,
                                      Context::ARRAY_FUNCTION_INDEX);
+    InstallSpeciesGetter(array_function);
 
     // Cache the array maps, needed by ArrayConstructorStub
     CacheInitialJSArrayMaps(native_context(), initial_map);
@@ -1884,9 +1894,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject>
global_object,
     InstallWithIntrinsicDefaultProto(isolate, promise_catch,
                                      Context::PROMISE_CATCH_INDEX);
 
-    SimpleInstallGetter(promise_fun, factory->symbol_species_string(),
-                        factory->species_symbol(), Builtins::kReturnReceiver,
-                        true);
+    InstallSpeciesGetter(promise_fun);
 
     Handle<Map> prototype_map(prototype->map());
     Map::SetShouldBeFastPrototypeMap(prototype_map, true, isolate);
@@ -2069,9 +2077,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject>
global_object,
     {
       // RegExp getters and setters.
 
-      SimpleInstallGetter(regexp_fun, factory->symbol_species_string(),
-                          factory->species_symbol(), Builtins::kReturnReceiver,
-                          true);
+      InstallSpeciesGetter(regexp_fun);
 
       // Static properties set by a successful match.
 
@@ -2396,6 +2402,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject>
global_object,
         BuiltinFunctionId::kArrayBufferByteLength);
     InstallWithIntrinsicDefaultProto(isolate, array_buffer_fun,
                                      Context::ARRAY_BUFFER_FUN_INDEX);
+    InstallSpeciesGetter(array_buffer_fun);
   }
 
   {  // -- T y p e d A r r a y
@@ -2407,6 +2414,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject>
global_object,
         CreateFunction(isolate, factory->InternalizeUtf8String("TypedArray"),
                        JS_TYPED_ARRAY_TYPE, JSTypedArray::kSize, prototype,
                        Builtins::kIllegal);
+    InstallSpeciesGetter(typed_array_fun);
 
     // Install the "constructor" property on the {prototype}.
     JSObject::AddProperty(prototype, factory->constructor_string(),
@@ -2534,6 +2542,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject>
global_object,
         isolate->initial_object_prototype(), Builtins::kIllegal);
     InstallWithIntrinsicDefaultProto(isolate, js_map_fun,
                                      Context::JS_MAP_FUN_INDEX);
+    InstallSpeciesGetter(js_map_fun);
   }
 
   {  // -- S e t
@@ -2542,6 +2551,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject>
global_object,
         isolate->initial_object_prototype(), Builtins::kIllegal);
     InstallWithIntrinsicDefaultProto(isolate, js_set_fun,
                                      Context::JS_SET_FUN_INDEX);
+    InstallSpeciesGetter(js_set_fun);
   }
 
   {  // -- J S M o d u l e N a m e s p a c e

[adamk, 2017-01-04 22:52:35]

The CQ bit was checked by adamk@chromium.org

[adamk, 2017-01-04 22:52:36]

The patchset sent to the CQ was uploaded after l-g-t-m from gsathya@chromium.org
Link to the patchset: https://codereview.chromium.org/2614623003/#ps40001
(title: "Rebased")

[commit-bot: I haz the power, 2017-01-04 22:52:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 23:19:27]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483570355977080,
"parent_rev": "79ae8f17442bf61e9655fa8b27d4ddd529cccb0e", "commit_rev":
"e968595b42f8d49bf88fe3181cbdf8568c0628f6"}

[commit-bot: I haz the power, 2017-01-04 23:19:33]

Description was changed from

==========
Move all Symbol.species setup for builtin constructors to bootstrapper

This lets us share Builtins::kReturnReceiver, so should
every-so-slightly reduce the size of the context snapshot.

R=gsathya@chromium.org
==========

to

==========
Move all Symbol.species setup for builtin constructors to bootstrapper

This lets us share Builtins::kReturnReceiver, so should
every-so-slightly reduce the size of the context snapshot.

R=gsathya@chromium.org

Review-Url: https://codereview.chromium.org/2614623003
Cr-Commit-Position: refs/heads/master@{#42076}
Committed:
https://chromium.googlesource.com/v8/v8/+/e968595b42f8d49bf88fe3181cbdf8568c0...
==========

[commit-bot: I haz the power, 2017-01-04 23:19:34]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/e968595b42f8d49bf88fe3181cbdf8568c0...