Do not look for the slot in the ScopeInfo's global range when it's not necessary.

This fixes the Runtime_DeclareGlobals performance regression caused by a huge number of global var declarations mentioned in chromium:517778.

BUG=chromium:517778
LOG=N

Review URL: https://codereview.chromium.org/1335633002

Cr-Commit-Position: refs/heads/master@{#30679}

diff --git a/src/contexts.cc b/src/contexts.cc
index ade019a703aedff4fb40a7b70c3b12636d54bd65..a008d49ac37ebfdf9e47cdcb995eac2fba591918 100644 (file)
--- a/src/contexts.cc
+++ b/src/contexts.cc
@@ -44,10 +44,10 @@ bool ScriptContextTable::Lookup(Handle<ScriptContextTable> table,
     DCHECK(context->IsScriptContext());
     Handle<ScopeInfo> scope_info(context->scope_info());
     int slot_index = ScopeInfo::ContextSlotIndex(
-        scope_info, name, &result->mode, &result->location, &result->init_flag,
+        scope_info, name, &result->mode, &result->init_flag,
         &result->maybe_assigned_flag);
 
-    if (slot_index >= 0 && result->location == VariableLocation::CONTEXT) {
+    if (slot_index >= 0) {
       result->context_index = i;
       result->slot_index = slot_index;
       return true;
@@ -333,15 +333,14 @@ Handle<Object> Context::Lookup(Handle<String> name,
           ? context->closure()->shared()->scope_info()
           : context->scope_info());
       VariableMode mode;
-      VariableLocation location;
       InitializationFlag init_flag;
       // TODO(sigurds) Figure out whether maybe_assigned_flag should
       // be used to compute binding_flags.
       MaybeAssignedFlag maybe_assigned_flag;
       int slot_index = ScopeInfo::ContextSlotIndex(
-          scope_info, name, &mode, &location, &init_flag, &maybe_assigned_flag);
+          scope_info, name, &mode, &init_flag, &maybe_assigned_flag);
       DCHECK(slot_index < 0 || slot_index >= MIN_CONTEXT_SLOTS);
-      if (slot_index >= 0 && location == VariableLocation::CONTEXT) {
+      if (slot_index >= 0) {
         if (FLAG_trace_contexts) {
           PrintF("=> found local in context slot %d (mode = %d)\n",
                  slot_index, mode);

diff --git a/src/contexts.h b/src/contexts.h
index 4685e1636c3a29ee8f00f44809edfd39a7985c5b..75cb1513b43876e8d0daee50ff201b57e34590ca 100644 (file)
--- a/src/contexts.h
+++ b/src/contexts.h
@@ -302,7 +302,6 @@ class ScriptContextTable : public FixedArray {
     int context_index;
     int slot_index;
     VariableMode mode;
-    VariableLocation location;
     InitializationFlag init_flag;
     MaybeAssignedFlag maybe_assigned_flag;
   };

diff --git a/src/debug/debug-evaluate.cc b/src/debug/debug-evaluate.cc
index 8d7635c01df8088e8afa34fd1dffb419d3e6ff67..b24956132424705258f3517d28b7aa7dd5de346d 100644 (file)
--- a/src/debug/debug-evaluate.cc
+++ b/src/debug/debug-evaluate.cc
@@ -280,15 +280,14 @@ Handle<Context> DebugEvaluate::ContextBuilder::MaterializeReceiver(
   switch (scope_info->scope_type()) {
     case FUNCTION_SCOPE: {
       VariableMode mode;
-      VariableLocation location;
       InitializationFlag init_flag;
       MaybeAssignedFlag maybe_assigned_flag;
 
       // Don't bother creating a fake context node if "this" is in the context
       // already.
-      if (ScopeInfo::ContextSlotIndex(
-              scope_info, isolate_->factory()->this_string(), &mode, &location,
-              &init_flag, &maybe_assigned_flag) >= 0) {
+      if (ScopeInfo::ContextSlotIndex(scope_info,
+                                      isolate_->factory()->this_string(), &mode,
+                                      &init_flag, &maybe_assigned_flag) >= 0) {
         return target;
       }
       receiver = handle(frame_->receiver(), isolate_);

diff --git a/src/debug/debug-frames.cc b/src/debug/debug-frames.cc
index c0970a359f25e5c707aac1e939bbef328d062185..c4c288148c5c637d76011b30f5f4fc2f846c9248 100644 (file)
--- a/src/debug/debug-frames.cc
+++ b/src/debug/debug-frames.cc
@@ -180,11 +180,10 @@ void FrameInspector::UpdateStackLocalsFromMaterializedObject(
 bool FrameInspector::ParameterIsShadowedByContextLocal(
     Handle<ScopeInfo> info, Handle<String> parameter_name) {
   VariableMode mode;
-  VariableLocation location;
   InitializationFlag init_flag;
   MaybeAssignedFlag maybe_assigned_flag;
-  return ScopeInfo::ContextSlotIndex(info, parameter_name, &mode, &location,
-                                     &init_flag, &maybe_assigned_flag) != -1;
+  return ScopeInfo::ContextSlotIndex(info, parameter_name, &mode, &init_flag,
+                                     &maybe_assigned_flag) != -1;
 }
 
 

diff --git a/src/debug/debug-scopes.cc b/src/debug/debug-scopes.cc
index 8252154cb41cf645dddf1ce6c07b955310cadd7d..e8ef2403934dc06f795a989897c49fa10144d412 100644 (file)
--- a/src/debug/debug-scopes.cc
+++ b/src/debug/debug-scopes.cc
@@ -561,12 +561,10 @@ bool ScopeIterator::SetContextLocalValue(Handle<ScopeInfo> scope_info,
     Handle<String> next_name(scope_info->ContextLocalName(i));
     if (String::Equals(variable_name, next_name)) {
       VariableMode mode;
-      VariableLocation location;
       InitializationFlag init_flag;
       MaybeAssignedFlag maybe_assigned_flag;
-      int context_index =
-          ScopeInfo::ContextSlotIndex(scope_info, next_name, &mode, &location,
-                                      &init_flag, &maybe_assigned_flag);
+      int context_index = ScopeInfo::ContextSlotIndex(
+          scope_info, next_name, &mode, &init_flag, &maybe_assigned_flag);
       context->set(context_index, *new_value);
       return true;
     }

diff --git a/src/objects.h b/src/objects.h
index dd43f0fe5b08df6f813d7fabd23ed9880c8d1a33..113ba3a67102e687a1b9597c1bafdfa99e96592c 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -3898,16 +3898,25 @@ class ScopeInfo : public FixedArray {
   // string.
   int StackSlotIndex(String* name);
 
-  // Lookup support for serialized scope info. Returns the
-  // context slot index for a given slot name if the slot is present; otherwise
+  // Lookup support for serialized scope info. Returns the local context slot
+  // index for a given slot name if the slot is present; otherwise
   // returns a value < 0. The name must be an internalized string.
   // If the slot is present and mode != NULL, sets *mode to the corresponding
   // mode for that variable.
   static int ContextSlotIndex(Handle<ScopeInfo> scope_info, Handle<String> name,
-                              VariableMode* mode, VariableLocation* location,
-                              InitializationFlag* init_flag,
+                              VariableMode* mode, InitializationFlag* init_flag,
                               MaybeAssignedFlag* maybe_assigned_flag);
 
+  // Similar to ContextSlotIndex() but this method searches only among
+  // global slots of the serialized scope info. Returns the context slot index
+  // for a given slot name if the slot is present; otherwise returns a
+  // value < 0. The name must be an internalized string. If the slot is present
+  // and mode != NULL, sets *mode to the corresponding mode for that variable.
+  static int ContextGlobalSlotIndex(Handle<ScopeInfo> scope_info,
+                                    Handle<String> name, VariableMode* mode,
+                                    InitializationFlag* init_flag,
+                                    MaybeAssignedFlag* maybe_assigned_flag);
+
   // Lookup the name of a certain context slot by its index.
   String* ContextSlotName(int slot_index);
 

diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc
index 02d6cf9599c5a2ee0b5158ed2de1bb20cc6428d6..9f49e4d5d2a1add0faadb9994ebb295481014f7c 100644 (file)
--- a/src/runtime/runtime-debug.cc
+++ b/src/runtime/runtime-debug.cc
@@ -568,13 +568,11 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
       if (scope_info->LocalIsSynthetic(i)) continue;
       Handle<String> name(scope_info->LocalName(i));
       VariableMode mode;
-      VariableLocation location;
       InitializationFlag init_flag;
       MaybeAssignedFlag maybe_assigned_flag;
       locals->set(local * 2, *name);
       int context_slot_index = ScopeInfo::ContextSlotIndex(
-          scope_info, name, &mode, &location, &init_flag, &maybe_assigned_flag);
-      DCHECK(VariableLocation::CONTEXT == location);
+          scope_info, name, &mode, &init_flag, &maybe_assigned_flag);
       Object* value = context->get(context_slot_index);
       locals->set(local * 2 + 1, value);
       local++;

diff --git a/src/scopeinfo.cc b/src/scopeinfo.cc
index 5492922c455843acc2b3b2097bbe76526a2b6c01..75938c355987087b69179a3875014fc62fcc7797 100644 (file)
--- a/src/scopeinfo.cc
+++ b/src/scopeinfo.cc
@@ -524,57 +524,73 @@ int ScopeInfo::StackSlotIndex(String* name) {
 
 int ScopeInfo::ContextSlotIndex(Handle<ScopeInfo> scope_info,
                                 Handle<String> name, VariableMode* mode,
-                                VariableLocation* location,
                                 InitializationFlag* init_flag,
                                 MaybeAssignedFlag* maybe_assigned_flag) {
   DCHECK(name->IsInternalizedString());
   DCHECK(mode != NULL);
-  DCHECK(location != NULL);
   DCHECK(init_flag != NULL);
   if (scope_info->length() > 0) {
     ContextSlotCache* context_slot_cache =
         scope_info->GetIsolate()->context_slot_cache();
-    int result = context_slot_cache->Lookup(*scope_info, *name, mode, location,
-                                            init_flag, maybe_assigned_flag);
+    int result = context_slot_cache->Lookup(*scope_info, *name, mode, init_flag,
+                                            maybe_assigned_flag);
     if (result != ContextSlotCache::kNotFound) {
       DCHECK(result < scope_info->ContextLength());
       return result;
     }
 
+    int start = scope_info->ContextLocalNameEntriesIndex();
+    int end = scope_info->ContextLocalNameEntriesIndex() +
+              scope_info->ContextLocalCount();
+    for (int i = start; i < end; ++i) {
+      if (*name == scope_info->get(i)) {
+        int var = i - start;
+        *mode = scope_info->ContextLocalMode(var);
+        *init_flag = scope_info->ContextLocalInitFlag(var);
+        *maybe_assigned_flag = scope_info->ContextLocalMaybeAssignedFlag(var);
+        result = Context::MIN_CONTEXT_SLOTS + var;
+
+        context_slot_cache->Update(scope_info, name, *mode, *init_flag,
+                                   *maybe_assigned_flag, result);
+        DCHECK(result < scope_info->ContextLength());
+        return result;
+      }
+    }
+    // Cache as not found. Mode, init flag and maybe assigned flag don't matter.
+    context_slot_cache->Update(scope_info, name, TEMPORARY,
+                               kNeedsInitialization, kNotAssigned, -1);
+  }
+  return -1;
+}
+
+
+int ScopeInfo::ContextGlobalSlotIndex(Handle<ScopeInfo> scope_info,
+                                      Handle<String> name, VariableMode* mode,
+                                      InitializationFlag* init_flag,
+                                      MaybeAssignedFlag* maybe_assigned_flag) {
+  DCHECK(name->IsInternalizedString());
+  DCHECK(mode != NULL);
+  DCHECK(init_flag != NULL);
+  if (scope_info->length() > 0) {
+    // This is to ensure that ContextLocalMode() and co. queries would work.
     DCHECK_EQ(scope_info->ContextGlobalNameEntriesIndex(),
               scope_info->ContextLocalNameEntriesIndex() +
                   scope_info->ContextLocalCount());
-    int start = scope_info->ContextLocalNameEntriesIndex();
+    int base = scope_info->ContextLocalNameEntriesIndex();
+    int start = scope_info->ContextGlobalNameEntriesIndex();
     int end = scope_info->ContextGlobalNameEntriesIndex() +
               scope_info->ContextGlobalCount();
     for (int i = start; i < end; ++i) {
       if (*name == scope_info->get(i)) {
-        int var = i - start;
+        int var = i - base;
         *mode = scope_info->ContextLocalMode(var);
         *init_flag = scope_info->ContextLocalInitFlag(var);
         *maybe_assigned_flag = scope_info->ContextLocalMaybeAssignedFlag(var);
-
-        if (var < scope_info->ContextLocalCount()) {
-          *location = VariableLocation::CONTEXT;
-          result = Context::MIN_CONTEXT_SLOTS + var;
-        } else {
-          var -= scope_info->ContextLocalCount();
-          *location = VariableLocation::GLOBAL;
-          result = Context::MIN_CONTEXT_SLOTS +
-                   scope_info->ContextLocalCount() + var;
-        }
-
-        context_slot_cache->Update(scope_info, name, *mode, *location,
-                                   *init_flag, *maybe_assigned_flag, result);
+        int result = Context::MIN_CONTEXT_SLOTS + var;
         DCHECK(result < scope_info->ContextLength());
         return result;
       }
     }
-    // Cache as not found. Mode, location, init flag and maybe assigned flag
-    // don't matter.
-    context_slot_cache->Update(scope_info, name, TEMPORARY,
-                               VariableLocation::CONTEXT, kNeedsInitialization,
-                               kNotAssigned, -1);
   }
   return -1;
 }
@@ -701,7 +717,6 @@ int ContextSlotCache::Hash(Object* data, String* name) {
 
 
 int ContextSlotCache::Lookup(Object* data, String* name, VariableMode* mode,
-                             VariableLocation* location,
                              InitializationFlag* init_flag,
                              MaybeAssignedFlag* maybe_assigned_flag) {
   int index = Hash(data, name);
@@ -709,7 +724,6 @@ int ContextSlotCache::Lookup(Object* data, String* name, VariableMode* mode,
   if ((key.data == data) && key.name->Equals(name)) {
     Value result(values_[index]);
     if (mode != NULL) *mode = result.mode();
-    if (location != NULL) *location = result.location();
     if (init_flag != NULL) *init_flag = result.initialization_flag();
     if (maybe_assigned_flag != NULL)
       *maybe_assigned_flag = result.maybe_assigned_flag();
@@ -720,8 +734,7 @@ int ContextSlotCache::Lookup(Object* data, String* name, VariableMode* mode,
 
 
 void ContextSlotCache::Update(Handle<Object> data, Handle<String> name,
-                              VariableMode mode, VariableLocation location,
-                              InitializationFlag init_flag,
+                              VariableMode mode, InitializationFlag init_flag,
                               MaybeAssignedFlag maybe_assigned_flag,
                               int slot_index) {
   DisallowHeapAllocation no_gc;
@@ -734,11 +747,10 @@ void ContextSlotCache::Update(Handle<Object> data, Handle<String> name,
     key.data = *data;
     key.name = *internalized_name;
     // Please note value only takes a uint as index.
-    values_[index] = Value(mode, location, init_flag, maybe_assigned_flag,
+    values_[index] = Value(mode, init_flag, maybe_assigned_flag,
                            slot_index - kNotFound).raw();
 #ifdef DEBUG
-    ValidateEntry(data, name, mode, location, init_flag, maybe_assigned_flag,
-                  slot_index);
+    ValidateEntry(data, name, mode, init_flag, maybe_assigned_flag, slot_index);
 #endif
   }
 }
@@ -753,7 +765,6 @@ void ContextSlotCache::Clear() {
 
 void ContextSlotCache::ValidateEntry(Handle<Object> data, Handle<String> name,
                                      VariableMode mode,
-                                     VariableLocation location,
                                      InitializationFlag init_flag,
                                      MaybeAssignedFlag maybe_assigned_flag,
                                      int slot_index) {
@@ -767,7 +778,6 @@ void ContextSlotCache::ValidateEntry(Handle<Object> data, Handle<String> name,
     DCHECK(key.name->Equals(*name));
     Value result(values_[index]);
     DCHECK(result.mode() == mode);
-    DCHECK(result.location() == location);
     DCHECK(result.initialization_flag() == init_flag);
     DCHECK(result.maybe_assigned_flag() == maybe_assigned_flag);
     DCHECK(result.index() + kNotFound == slot_index);

diff --git a/src/scopeinfo.h b/src/scopeinfo.h
index adefaef9741d56d82f5cef62d05bc687baaa8bf7..70a17cd7d4a945975ce7ed59f7dc73c9d42d569c 100644 (file)
--- a/src/scopeinfo.h
+++ b/src/scopeinfo.h
@@ -21,12 +21,12 @@ class ContextSlotCache {
   // Lookup context slot index for (data, name).
   // If absent, kNotFound is returned.
   int Lookup(Object* data, String* name, VariableMode* mode,
-             VariableLocation* location, InitializationFlag* init_flag,
+             InitializationFlag* init_flag,
              MaybeAssignedFlag* maybe_assigned_flag);
 
   // Update an element in the cache.
   void Update(Handle<Object> data, Handle<String> name, VariableMode mode,
-              VariableLocation location, InitializationFlag init_flag,
+              InitializationFlag init_flag,
               MaybeAssignedFlag maybe_assigned_flag, int slot_index);
 
   // Clear the cache.
@@ -47,8 +47,7 @@ class ContextSlotCache {
 
 #ifdef DEBUG
   void ValidateEntry(Handle<Object> data, Handle<String> name,
-                     VariableMode mode, VariableLocation location,
-                     InitializationFlag init_flag,
+                     VariableMode mode, InitializationFlag init_flag,
                      MaybeAssignedFlag maybe_assigned_flag, int slot_index);
 #endif
 
@@ -59,26 +58,16 @@ class ContextSlotCache {
   };
 
   struct Value {
-    enum VariableLocationFlag { kContext, kGlobal };
-
-    Value(VariableMode mode, VariableLocation location,
-          InitializationFlag init_flag, MaybeAssignedFlag maybe_assigned_flag,
-          int index) {
-      DCHECK(location == VariableLocation::CONTEXT ||
-             location == VariableLocation::GLOBAL);
-      VariableLocationFlag location_flag =
-          location == VariableLocation::CONTEXT ? kContext : kGlobal;
+    Value(VariableMode mode, InitializationFlag init_flag,
+          MaybeAssignedFlag maybe_assigned_flag, int index) {
       DCHECK(ModeField::is_valid(mode));
-      DCHECK(VariableLocationField::is_valid(location_flag));
       DCHECK(InitField::is_valid(init_flag));
       DCHECK(MaybeAssignedField::is_valid(maybe_assigned_flag));
       DCHECK(IndexField::is_valid(index));
       value_ = ModeField::encode(mode) | IndexField::encode(index) |
-               VariableLocationField::encode(location_flag) |
                InitField::encode(init_flag) |
                MaybeAssignedField::encode(maybe_assigned_flag);
       DCHECK(mode == this->mode());
-      DCHECK(location == this->location());
       DCHECK(init_flag == this->initialization_flag());
       DCHECK(maybe_assigned_flag == this->maybe_assigned_flag());
       DCHECK(index == this->index());
@@ -90,17 +79,6 @@ class ContextSlotCache {
 
     VariableMode mode() { return ModeField::decode(value_); }
 
-    VariableLocation location() {
-      switch (VariableLocationField::decode(value_)) {
-        case kContext:
-          return VariableLocation::CONTEXT;
-        case kGlobal:
-          return VariableLocation::GLOBAL;
-      }
-      UNREACHABLE();
-      return VariableLocation::CONTEXT;
-    }
-
     InitializationFlag initialization_flag() {
       return InitField::decode(value_);
     }
@@ -116,9 +94,7 @@ class ContextSlotCache {
     class ModeField : public BitField<VariableMode, 0, 4> {};
     class InitField : public BitField<InitializationFlag, 4, 1> {};
     class MaybeAssignedField : public BitField<MaybeAssignedFlag, 5, 1> {};
-    class VariableLocationField : public BitField<VariableLocationFlag, 6, 1> {
-    };
-    class IndexField : public BitField<int, 7, 32 - 7> {};
+    class IndexField : public BitField<int, 6, 32 - 6> {};
 
    private:
     uint32_t value_;

diff --git a/src/scopes.cc b/src/scopes.cc
index ba2efb2bcc9e1d7c742780f0f9d586b81a70c6f8..963d539afb96e664e8b2f063a14f7e85ee03048f 100644 (file)
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -394,12 +394,16 @@ Variable* Scope::LookupLocal(const AstRawString* name) {
 
   // Check context slot lookup.
   VariableMode mode;
-  VariableLocation location;
+  VariableLocation location = VariableLocation::CONTEXT;
   InitializationFlag init_flag;
   MaybeAssignedFlag maybe_assigned_flag;
-  int index =
-      ScopeInfo::ContextSlotIndex(scope_info_, name_handle, &mode, &location,
-                                  &init_flag, &maybe_assigned_flag);
+  int index = ScopeInfo::ContextSlotIndex(scope_info_, name_handle, &mode,
+                                          &init_flag, &maybe_assigned_flag);
+  if (index < 0) {
+    location = VariableLocation::GLOBAL;
+    index = ScopeInfo::ContextGlobalSlotIndex(scope_info_, name_handle, &mode,
+                                              &init_flag, &maybe_assigned_flag);
+  }
   if (index < 0) {
     // Check parameters.
     index = scope_info_->ParameterIndex(*name_handle);