Inline loading of immutable properties

src/hydrogen.cc

 // Copyright 2013 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 6698 matching lines @@
 
   } else if (expr->IsFunctionPrototype()) {
     HValue* function = Pop();
     BuildCheckHeapObject(function);
     instr = New<HLoadFunctionPrototype>(function);
 
   } else if (expr->key()->IsPropertyName()) {
     Handle<String> name = expr->key()->AsLiteral()->AsPropertyName();
     HValue* object = Pop();
 
+    // See if the property to be loaded is non-configurable and non-writable
+    if (object->IsConstant() &&
+        HConstant::cast(object)->handle(isolate())->IsJSObject()) {
+      Handle<JSObject> js_object =
+          Handle<JSObject>::cast(HConstant::cast(object)->handle(isolate()));
+
+      if (!js_object->map()->is_observed()) {
+        LookupResult lookup(isolate());
+        js_object->LocalLookup(*name, &lookup, false);
+
+        if (lookup.IsFound() && lookup.IsCacheable() &&
+            (js_object->map()->is_frozen() ||

[Toon Verwaest, 2014/04/02 14:51:47]

You don't need to special-case is_frozen(), such properties will be read-only
non-deleteable.

+            (lookup.IsReadOnly() && lookup.IsDontDelete()))) {
+          Handle<Object> value = handle(lookup.GetLazyValue(), isolate());
+
+          if (!value.is_null() &&
+              !value->IsTheHole() &&
+              !value->IsCallable()) {

[Toon Verwaest, 2014/04/02 14:51:47]

Why these cases? Can you introduce abstractions that make it clear what these
cases mean?

[p.antonov, 2014/04/02 15:53:58]

The null check should actually be unnecessary (since the lookup is found?).

About the hole check: I took a look at the different lookup types and it looked
like Normal, Field and Constant were the ones that can be applied in this
situation. I didn't find a method that checks for all of those but I noticed
GetLazyValue returns TheHole if the lookup type is not a normal, a field or a
constant. Should I make a method that checks of the lookup type is one of these
three types?

The callable check is there because if the value is a function, then inlining
its load and calling it causes crash. I think this is because of some assumption
is being violated somewhere but I don't know at this time what it is. Should I
add a fixme comment?

+            instr = New<HConstant>(value);
+            return ast_context()->ReturnInstruction(instr, ast_id);
+          }
+        }
+      }
+    }
+
     instr = BuildNamedAccess(LOAD, ast_id, expr->LoadId(), expr,
                              object, name, NULL, expr->IsUninitialized());
     if (instr == NULL) return;
     if (instr->IsLinked()) return ast_context()->ReturnValue(instr);
 
   } else {
     HValue* key = Pop();
     HValue* obj = Pop();
 
     bool has_side_effects = false;
@@ skipping 4772 matching lines @@
   if (ShouldProduceTraceOutput()) {
     isolate()->GetHTracer()->TraceHydrogen(name(), graph_);
   }
 
 #ifdef DEBUG
   graph_->Verify(false);  // No full verify.
 #endif
 }
 
 } }  // namespace v8::internal