Optimize @@species based on a global 'protector' cell

src/lookup.cc

Index: src/lookup.cc
diff --git a/src/lookup.cc b/src/lookup.cc
index 83bfc79c8eefe6435b9134c33943e91f1d6bad83..f89a88bc11636bf40048a3b0a3909601aa0f5795 100644
--- a/src/lookup.cc
+++ b/src/lookup.cc
@@ -142,6 +142,49 @@ void LookupIterator::ReloadPropertyInformation() {
   DCHECK(IsFound() || !holder_->HasFastProperties());
 }
 
+void LookupIterator::UpdateProtector() {
+  if (IsElement()) return;
+  if (isolate_->bootstrapper()->IsActive()) return;
+  if (!isolate_->IsArraySpeciesLookupChainIntact()) return;
+
+  if (*name_ == *isolate_->factory()->constructor_string()) {
+    // Setting the constructor property could change an instance's @@species
+    if (holder_->IsJSArray()) {
+      isolate_->CountUsage(
+          v8::Isolate::UseCounterFeature::kArrayInstanceConstructorModified);
+      isolate_->InvalidateArraySpeciesProtector();
+    } else if (holder_->map()->is_prototype_map()) {
+      // Setting the constructor of Array.prototype of any realm also needs
+      // to invalidate the species protector
+      Object* context = heap()->native_contexts_list();

[adamk, 2016/02/19 00:01:17]

The use of raw pointers instead of handles looks a little funny here; is there a
specific reason this isn't Handlified?

[Dan Ehrenberg, 2016/02/19 00:15:37]

I was following the way contexts are used by the array protector code. I guess I
wasn't accounting for the case where contexts would go away while this stuff is
happening. I don't see how that could happen, since we don't allocate or call
into JS, but seems like good practice anyway. Should I change both of these?

[adamk, 2016/02/19 01:06:01]

I agree that I don't see any problematic code here, but if you want to stay
unhandlified the best practice would be to add a DisallowHeapAllocation object
on the stack which will at least CHECK-fail in debug mode if something changes
and starts allocating.

[Camillo Bruni, 2016/02/22 15:29:34]

right, I think the other code forgot to add DisallowHeapAllocation. Should be
fine in the latest patch.

+      while (!context->IsUndefined()) {
+        Context* current_context = Context::cast(context);
+        if (current_context->initial_array_prototype() == *holder_) {
+          isolate_->CountUsage(v8::Isolate::UseCounterFeature::
+                                   kArrayPrototypeConstructorModified);
+          isolate_->InvalidateArraySpeciesProtector();
+          break;
+        }
+        context = current_context->get(Context::NEXT_CONTEXT_LINK);
+      }
+    }
+  } else if (FLAG_harmony_species &&
+             *name_ == *isolate_->factory()->species_symbol()) {
+    // Setting the Symbol.species property of any Array constructor invalidates
+    // the species protector
+    Object* context = heap()->native_contexts_list();
+    while (!context->IsUndefined()) {
+      Context* current_context = Context::cast(context);
+      if (current_context->array_function() == *holder_) {
+        isolate_->CountUsage(
+            v8::Isolate::UseCounterFeature::kArraySpeciesModified);
+        isolate_->InvalidateArraySpeciesProtector();
+        break;
+      }
+      context = current_context->get(Context::NEXT_CONTEXT_LINK);
+    }
+  }
+}
 
 void LookupIterator::PrepareForDataProperty(Handle<Object> value) {
   DCHECK(state_ == DATA || state_ == ACCESSOR);