Avoid one repeated property lookup when computing load ICs.

src/ic.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/v8.h"
 
 #include "src/accessors.h"
 #include "src/api.h"
 #include "src/arguments.h"
 #include "src/codegen.h"
@@ skipping 172 matching lines @@
 Code* IC::GetOriginalCode() const {
   HandleScope scope(isolate());
   Handle<SharedFunctionInfo> shared(GetSharedFunctionInfo(), isolate());
   ASSERT(Debug::HasDebugInfo(shared));
   Code* original_code = Debug::GetDebugInfo(shared)->original_code();
   ASSERT(original_code->IsCode());
   return original_code;
 }
 
 
-static bool HasInterceptorGetter(JSObject* object) {
-  return !object->GetNamedInterceptor()->getter()->IsUndefined();
-}
-
-
 static bool HasInterceptorSetter(JSObject* object) {
   return !object->GetNamedInterceptor()->setter()->IsUndefined();
 }
 
 
-static void LookupForRead(Handle<Object> object,
-                          Handle<String> name,
-                          LookupResult* lookup) {
-  // Skip all the objects with named interceptors, but
-  // without actual getter.
-  while (true) {
-    object->Lookup(name, lookup);
-    // Besides normal conditions (property not found or it's not
-    // an interceptor), bail out if lookup is not cacheable: we won't
-    // be able to IC it anyway and regular lookup should work fine.
-    if (!lookup->IsInterceptor() || !lookup->IsCacheable()) {
-      return;
+static void LookupForRead(LookupIterator* it) {
+  for (; it->IsFound(); it->Next()) {
+    switch (it->state()) {
+      case LookupIterator::NOT_FOUND:
+        UNREACHABLE();
+      case LookupIterator::JSPROXY:
+        return;
+      case LookupIterator::INTERCEPTOR: {
+        // If there is a getter, return; otherwise loop to perform the lookup.
+        Handle<JSObject> holder = it->GetHolder<JSObject>();
+        if (!holder->GetNamedInterceptor()->getter()->IsUndefined()) {

[Toon Verwaest, 2014/07/30 16:54:52]

In the future we should probably get the property from the interceptor to see if
it's there. If not, we should continue the lookup behind the interceptor and
compile this into the IC as well.

Since this matches what we did before, it's fine for now.

[Jakob Kummerow, 2014/07/31 13:55:51]

But that assumes that the interceptor never changes its behavior, right? I'm not
sure we should make that assumption.
Acknowledged.

+          return;
+        }
+        break;
+      }
+      case LookupIterator::ACCESS_CHECK:
+        return;
+      case LookupIterator::PROPERTY:
+        if (it->HasProperty()) return;  // Yay!
+        break;
     }
-
-    Handle<JSObject> holder(lookup->holder(), lookup->isolate());
-    if (HasInterceptorGetter(*holder)) {
-      return;
-    }
-
-    holder->LookupOwnRealNamedProperty(name, lookup);
-    if (lookup->IsFound()) {
-      ASSERT(!lookup->IsInterceptor());
-      return;
-    }
-
-    PrototypeIterator iter(lookup->isolate(), holder);
-    if (iter.IsAtEnd()) {
-      ASSERT(!lookup->IsFound());
-      return;
-    }
-
-    object = PrototypeIterator::GetCurrent(iter);
   }
 }
 
 
 bool IC::TryRemoveInvalidPrototypeDependentStub(Handle<Object> receiver,
                                                 Handle<String> name) {
   if (!IsNameCompatibleWithPrototypeFailure(name)) return false;
   Handle<Map> receiver_map = TypeToMap(*receiver_type(), isolate());
   maybe_handler_ = target()->FindHandlerForMap(*receiver_map);
 
@@ skipping 322 matching lines @@
         isolate(),
         result,
         Runtime::GetElementOrCharAt(isolate(), object, index),
         Object);
     return result;
   }
 
   bool use_ic = MigrateDeprecated(object) ? false : FLAG_use_ic;
 
   // Named lookup in the object.
-  LookupResult lookup(isolate());
-  LookupForRead(object, name, &lookup);
+  LookupIterator it(object, name);
+  LookupForRead(&it);
 
   // If we did not find a property, check if we need to throw an exception.
-  if (!lookup.IsFound()) {
+  if (!it.IsFound()) {
     if (IsUndeclaredGlobal(object)) {
       return ReferenceError("not_defined", name);
     }
     LOG(isolate(), SuspectReadEvent(*name, *object));
   }
 
   // Update inline cache and stub cache.
-  if (use_ic) UpdateCaches(&lookup, object, name);
+  if (use_ic) UpdateCaches(&it, object, name);
 
   // Get the property.
-  LookupIterator it(object, name);
   Handle<Object> result;
   ASSIGN_RETURN_ON_EXCEPTION(
       isolate(), result, Object::GetProperty(&it), Object);
   // If the property is not present, check if we need to throw an exception.
-  if ((lookup.IsInterceptor() || lookup.IsHandler()) &&
-      !it.IsFound() && IsUndeclaredGlobal(object)) {
+  if (!it.IsFound() && IsUndeclaredGlobal(object)) {
     return ReferenceError("not_defined", name);
   }
 
   return result;
 }
 
 
 static bool AddOneReceiverMapIfMissing(MapHandleList* receiver_maps,
                                        Handle<Map> new_receiver_map) {
   ASSERT(!new_receiver_map.is_null());
@@ skipping 212 matching lines @@
   }
 }
 
 
 Handle<Code> LoadIC::SimpleFieldLoad(FieldIndex index) {
   LoadFieldStub stub(isolate(), index);
   return stub.GetCode();
 }
 
 
-void LoadIC::UpdateCaches(LookupResult* lookup,
-                          Handle<Object> object,
+void LoadIC::UpdateCaches(LookupIterator* lookup, Handle<Object> object,
                           Handle<String> name) {
   if (state() == UNINITIALIZED) {
     // This is the first time we execute this inline cache.
     // Set the target to the pre monomorphic stub to delay
     // setting the monomorphic state.
     set_target(*pre_monomorphic_stub());
     TRACE_IC("LoadIC", name);
     return;
   }
 
   Handle<Code> code;
-  if (!lookup->IsCacheable()) {
-    // Bail out if the result is not cacheable.
+  if (lookup->state() == LookupIterator::JSPROXY ||
+      lookup->state() == LookupIterator::ACCESS_CHECK) {
     code = slow_stub();
-  } else if (!lookup->IsProperty()) {
+  } else if (!lookup->IsFound()) {
     if (kind() == Code::LOAD_IC) {
       code = NamedLoadHandlerCompiler::ComputeLoadNonexistent(name,
                                                               receiver_type());
       // TODO(jkummerow/verwaest): Introduce a builtin that handles this case.
       if (code.is_null()) code = slow_stub();
     } else {
       code = slow_stub();
     }
   } else {
     code = ComputeHandler(lookup, object, name);
   }
 
   PatchCache(name, code);
   TRACE_IC("LoadIC", name);
 }
 
 
 void IC::UpdateMegamorphicCache(HeapType* type, Name* name, Code* code) {
   if (kind() == Code::KEYED_LOAD_IC || kind() == Code::KEYED_STORE_IC) return;
   Map* map = *TypeToMap(type, isolate());
   isolate()->stub_cache()->Set(name, map, code);
 }
 
 
-Handle<Code> IC::ComputeHandler(LookupResult* lookup,
-                                Handle<Object> object,
-                                Handle<String> name,
-                                Handle<Object> value) {
+Handle<Code> IC::ComputeHandler(LookupIterator* lookup, Handle<Object> object,
+                                Handle<String> name, Handle<Object> value) {
+  bool receiver_is_holder = lookup->HolderIsReceiver();

[Toon Verwaest, 2014/07/30 16:54:53]

These 2 things are different.
Rename HolderIsReceiver to HolderIsReceiverOrHiddenPrototype() along the way to
make it explicit?

[Jakob Kummerow, 2014/07/31 13:55:51]

Done.

+  CacheHolderFlag flag;
+  Handle<Map> stub_holder_map = IC::GetHandlerCacheHolder(
+      *receiver_type(), receiver_is_holder, isolate(), &flag);
+
+  Handle<Code> code = PropertyHandlerCompiler::Find(
+      name, stub_holder_map, kind(), flag,
+      lookup->has_fast_properties() ? Code::FAST : Code::NORMAL);

[Toon Verwaest, 2014/07/30 16:54:53]

What about just doing lookup->holder_map()->is_dictionary_map() and removing
this method again from the LookupIterator?

The difference between Code::FAST and Code::NORMAL should go away any time soon
now anyway ;)

[Jakob Kummerow, 2014/07/31 13:55:51]

Done.

+  // Use the cached value if it exists, and if it is different from the
+  // handler that just missed.
+  if (!code.is_null()) {
+    if (!maybe_handler_.is_null() &&
+        !maybe_handler_.ToHandleChecked().is_identical_to(code)) {
+      return code;
+    }
+    if (maybe_handler_.is_null()) {
+      // maybe_handler_ is only populated for MONOMORPHIC and POLYMORPHIC ICs.
+      // In MEGAMORPHIC case, check if the handler in the megamorphic stub
+      // cache (which just missed) is different from the cached handler.
+      if (state() == MEGAMORPHIC && object->IsHeapObject()) {
+        Map* map = Handle<HeapObject>::cast(object)->map();
+        Code* megamorphic_cached_code =
+            isolate()->stub_cache()->Get(*name, map, code->flags());
+        if (megamorphic_cached_code != *code) return code;
+      } else {
+        return code;
+      }
+    }
+  }
+
+  code = CompileHandler(lookup, object, name, value, flag);
+  ASSERT(code->is_handler());
+
+  if (code->type() != Code::NORMAL) {
+    Map::UpdateCodeCache(stub_holder_map, name, code);
+  }
+
+  return code;
+}
+
+
+Handle<Code> IC::ComputeStoreHandler(LookupResult* lookup,
+                                     Handle<Object> object, Handle<String> name,
+                                     Handle<Object> value) {
   bool receiver_is_holder = lookup->ReceiverIsHolder(object);
   CacheHolderFlag flag;
   Handle<Map> stub_holder_map = IC::GetHandlerCacheHolder(
       *receiver_type(), receiver_is_holder, isolate(), &flag);
 
   Handle<Code> code = PropertyHandlerCompiler::Find(
       name, stub_holder_map, handler_kind(), flag,
       lookup->holder()->HasFastProperties() ? Code::FAST : Code::NORMAL);
   // Use the cached value if it exists, and if it is different from the
   // handler that just missed.
@@ skipping 10 matching lines @@
         Map* map = Handle<HeapObject>::cast(object)->map();
         Code* megamorphic_cached_code =
             isolate()->stub_cache()->Get(*name, map, code->flags());
         if (megamorphic_cached_code != *code) return code;
       } else {
         return code;
       }
     }
   }
 
-  code = CompileHandler(lookup, object, name, value, flag);
+  code = CompileStoreHandler(lookup, object, name, value, flag);
   ASSERT(code->is_handler());
 
   if (code->type() != Code::NORMAL) {
     Map::UpdateCodeCache(stub_holder_map, name, code);
   }
 
   return code;
 }
 
 
-Handle<Code> LoadIC::CompileHandler(LookupResult* lookup, Handle<Object> object,
-                                    Handle<String> name, Handle<Object> unused,
+Handle<Code> LoadIC::CompileHandler(LookupIterator* lookup,
+                                    Handle<Object> object, Handle<String> name,
+                                    Handle<Object> unused,
                                     CacheHolderFlag cache_holder) {
   if (object->IsString() &&
       String::Equals(isolate()->factory()->length_string(), name)) {
     FieldIndex index = FieldIndex::ForInObjectOffset(String::kLengthOffset);
     return SimpleFieldLoad(index);
   }
 
   if (object->IsStringWrapper() &&
       String::Equals(isolate()->factory()->length_string(), name)) {
     StringLengthStub string_length_stub(isolate());
     return string_length_stub.GetCode();
   }
 
   // Use specialized code for getting prototype of functions.
   if (object->IsJSFunction() &&
       String::Equals(isolate()->factory()->prototype_string(), name) &&
       Handle<JSFunction>::cast(object)->should_have_prototype() &&
       !Handle<JSFunction>::cast(object)->map()->has_non_instance_prototype()) {
     Handle<Code> stub;
     FunctionPrototypeStub function_prototype_stub(isolate());
     return function_prototype_stub.GetCode();
   }
 
   Handle<HeapType> type = receiver_type();
-  Handle<JSObject> holder(lookup->holder());
+  Handle<JSObject> holder(lookup->GetHolder<JSObject>());

[Toon Verwaest, 2014/07/30 16:54:52]

Handle<JSObject> holder = lookup->GetHolder<JSObject>();

[Jakob Kummerow, 2014/07/31 13:55:50]

Done.

   bool receiver_is_holder = object.is_identical_to(holder);
   NamedLoadHandlerCompiler compiler(isolate(), cache_holder);
 
-  switch (lookup->type()) {
+  if (lookup->state() == LookupIterator::INTERCEPTOR) {
+    ASSERT(!holder->GetNamedInterceptor()->getter()->IsUndefined());
+    return compiler.CompileLoadInterceptor(type, holder, name);
+  }
+  ASSERT(lookup->state() == LookupIterator::PROPERTY);
+
+  switch (lookup->property_details().type()) {

[Toon Verwaest, 2014/07/30 16:54:52]

What about doing the same as GetProperty; using property_kind() to switch
between callbacks and others, then using property_encoding for dict vs
field/constant; and only at the end using property_details?

I'd like to get rid of the current form of PropertyType at some point.

[Jakob Kummerow, 2014/07/31 13:55:51]

Done.

     case FIELD: {
       FieldIndex field = lookup->GetFieldIndex();
       if (receiver_is_holder) {
         return SimpleFieldLoad(field);
       }
       return compiler.CompileLoadField(
           type, holder, name, field, lookup->representation());
     }
     case CONSTANT: {
-      Handle<Object> constant(lookup->GetConstant(), isolate());
+      Handle<Object> constant = lookup->GetDataValue();
       return compiler.CompileLoadConstant(type, holder, name, constant);
     }
     case NORMAL:
       if (kind() != Code::LOAD_IC) break;
       if (holder->IsGlobalObject()) {
         Handle<GlobalObject> global = Handle<GlobalObject>::cast(holder);
         Handle<PropertyCell> cell(
             global->GetPropertyCell(lookup), isolate());
         Handle<Code> code = compiler.CompileLoadGlobal(
-            type, global, cell, name, lookup->IsDontDelete());
+            type, global, cell, name, !lookup->IsConfigurable());

[Toon Verwaest, 2014/07/30 16:54:52]

What about changing the interface of CompileLoadGlobal? Otherwise you negate the
flag 3 times :)
!is_dont_delete -> !configurable -> !is_dont_delete

[Jakob Kummerow, 2014/07/31 13:55:51]

Done.

         // TODO(verwaest): Move caching of these NORMAL stubs outside as well.
         CacheHolderFlag flag;
         Handle<Map> stub_holder_map =
             GetHandlerCacheHolder(*type, receiver_is_holder, isolate(), &flag);
         Map::UpdateCodeCache(stub_holder_map, name, code);
         return code;
       }
       // There is only one shared stub for loading normalized
       // properties. It does not traverse the prototype chain, so the
       // property must be found in the object for the stub to be
       // applicable.
       if (!receiver_is_holder) break;
       return isolate()->builtins()->LoadIC_Normal();
     case CALLBACKS: {
       // Use simple field loads for some well-known callback properties.
       if (receiver_is_holder) {
         ASSERT(object->IsJSObject());
         Handle<JSObject> receiver = Handle<JSObject>::cast(object);
         int object_offset;
         if (Accessors::IsJSObjectFieldAccessor<HeapType>(
                 type, name, &object_offset)) {
           FieldIndex index = FieldIndex::ForInObjectOffset(
               object_offset, receiver->map());
           return SimpleFieldLoad(index);
         }
       }
 
-      Handle<Object> callback(lookup->GetCallbackObject(), isolate());
-      if (callback->IsExecutableAccessorInfo()) {
+      Handle<Object> accessors = lookup->GetAccessors();
+      if (accessors->IsExecutableAccessorInfo()) {
         Handle<ExecutableAccessorInfo> info =
-            Handle<ExecutableAccessorInfo>::cast(callback);
+            Handle<ExecutableAccessorInfo>::cast(accessors);
         if (v8::ToCData<Address>(info->getter()) == 0) break;
         if (!info->IsCompatibleReceiver(*object)) break;
         return compiler.CompileLoadCallback(type, holder, name, info);
-      } else if (callback->IsAccessorPair()) {
-        Handle<Object> getter(Handle<AccessorPair>::cast(callback)->getter(),
+      } else if (accessors->IsAccessorPair()) {
+        Handle<Object> getter(Handle<AccessorPair>::cast(accessors)->getter(),
                               isolate());
         if (!getter->IsJSFunction()) break;
         if (holder->IsGlobalObject()) break;
         if (!holder->HasFastProperties()) break;
         Handle<JSFunction> function = Handle<JSFunction>::cast(getter);
         if (!object->IsJSObject() &&
             !function->IsBuiltin() &&
             function->shared()->strict_mode() == SLOPPY) {
           // Calling sloppy non-builtins with a value as the receiver
           // requires boxing.
           break;
         }
         CallOptimization call_optimization(function);
         if (call_optimization.is_simple_api_call() &&
             call_optimization.IsCompatibleReceiver(object, holder)) {
           return compiler.CompileLoadCallback(
               type, holder, name, call_optimization);
         }
         return compiler.CompileLoadViaGetter(type, holder, name, function);
       }
       // TODO(dcarney): Handle correctly.
-      ASSERT(callback->IsDeclaredAccessorInfo());
+      ASSERT(accessors->IsDeclaredAccessorInfo());
       break;
     }
-    case INTERCEPTOR:
-      ASSERT(HasInterceptorGetter(*holder));
-      return compiler.CompileLoadInterceptor(type, holder, name);
     default:
       break;
   }
 
   return slow_stub();
 }
 
 
 static Handle<Object> TryConvertKey(Handle<Object> key, Isolate* isolate) {
   // This helper implements a few common fast cases for converting
@@ skipping 326 matching lines @@
 
 void StoreIC::UpdateCaches(LookupResult* lookup,
                            Handle<JSObject> receiver,
                            Handle<String> name,
                            Handle<Object> value) {
   ASSERT(lookup->IsFound());
 
   // These are not cacheable, so we never see such LookupResults here.
   ASSERT(!lookup->IsHandler());
 
-  Handle<Code> code = ComputeHandler(lookup, receiver, name, value);
+  Handle<Code> code = ComputeStoreHandler(lookup, receiver, name, value);
 
   PatchCache(name, code);
   TRACE_IC("StoreIC", name);
 }
 
 
-Handle<Code> StoreIC::CompileHandler(LookupResult* lookup,
-                                     Handle<Object> object, Handle<String> name,
-                                     Handle<Object> value,
-                                     CacheHolderFlag cache_holder) {
+Handle<Code> StoreIC::CompileStoreHandler(LookupResult* lookup,
+                                          Handle<Object> object,
+                                          Handle<String> name,
+                                          Handle<Object> value,
+                                          CacheHolderFlag cache_holder) {
   if (object->IsAccessCheckNeeded()) return slow_stub();
   ASSERT(cache_holder == kCacheOnReceiver || lookup->type() == CALLBACKS ||
          (object->IsJSGlobalProxy() && lookup->holder()->IsJSGlobalObject()));
   // This is currently guaranteed by checks in StoreIC::Store.
   Handle<JSObject> receiver = Handle<JSObject>::cast(object);
 
   Handle<JSObject> holder(lookup->holder());
   NamedStoreHandlerCompiler compiler(isolate());
 
   if (lookup->IsTransition()) {
@@ skipping 1650 matching lines @@
 #undef ADDR
 };
 
 
 Address IC::AddressFromUtilityId(IC::UtilityId id) {
   return IC_utilities[id];
 }
 
 
 } }  // namespace v8::internal