Revert of Group lexical context variables for faster look up. (patchset #2...

Revert of Group lexical context variables for faster look up. (patchset #2 id:20001 of https://codereview.chromium.org/1281883002/ )

Reason for revert:
This performance hack is no longer necessary.

Original issue's description:
> Group lexical context variables for faster look up.
>
> Currently, looking up a lexical context variable requires looking up
> the variable name and then checking its mode. This can be a bottleneck
> in Runtime_DeclareGlobals, even when no lexical context variables are
> declared.
>
> R=rossberg@chromium.org
> BUG=crbug:517778
> LOG=N
>
> Committed: https://crrev.com/a45ed17bb6aca02e940f13bbf456d660cccc86ae
> Cr-Commit-Position: refs/heads/master@{#30075}

TBR=rossberg@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=crbug:517778

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

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

src/contexts.cc

@@ -56,19 +56,6 @@ bool ScriptContextTable::Lookup(Handle<ScriptContextTable> table,
 }
 
 
-bool ScriptContextTable::LookupLexical(Handle<ScriptContextTable> table,
-                                       Handle<String> name) {
-  for (int i = 0; i < table->used(); i++) {
-    Handle<Context> context = GetContext(table, i);
-    DCHECK(context->IsScriptContext());
-    Handle<ScopeInfo> scope_info(ScopeInfo::cast(context->extension()));
-    int slot_index = ScopeInfo::LexicalContextSlotIndex(scope_info, name);
-    if (slot_index >= 0) return true;
-  }
-  return false;
-}
-
-
 Context* Context::declaration_context() {
   Context* current = this;
   while (!current->IsFunctionContext() && !current->IsNativeContext() &&

src/contexts.h

@@ -241,10 +241,6 @@ class ScriptContextTable : public FixedArray {
   static bool Lookup(Handle<ScriptContextTable> table, Handle<String> name,
                      LookupResult* result);
 
-  MUST_USE_RESULT
-  static bool LookupLexical(Handle<ScriptContextTable> table,
-                            Handle<String> name);
-
   MUST_USE_RESULT
   static Handle<ScriptContextTable> Extend(Handle<ScriptContextTable> table,
                                            Handle<Context> script_context);

src/objects.h

@@ -3935,9 +3935,6 @@ class ScopeInfo : public FixedArray {
                               InitializationFlag* init_flag,
                               MaybeAssignedFlag* maybe_assigned_flag);
 
-  static int LexicalContextSlotIndex(Handle<ScopeInfo> scope_info,
-                                     Handle<String> name);
-
   // Lookup the name of a certain context slot by its index.
   String* ContextSlotName(int slot_index);
 
@@ -3982,7 +3979,6 @@ class ScopeInfo : public FixedArray {
   V(ParameterCount)               \
   V(StackLocalCount)              \
   V(ContextLocalCount)            \
-  V(LexicalContextLocalCount)     \
   V(ContextGlobalCount)           \
   V(StrongModeFreeVariableCount)
 

src/runtime/runtime-scopes.cc

@@ -36,10 +36,9 @@ static Object* DeclareGlobals(Isolate* isolate, Handle<GlobalObject> global,
                               bool is_const, bool is_function) {
   Handle<ScriptContextTable> script_contexts(
       global->native_context()->script_context_table());
-  // We use LookupLexical to limit lookup to lexical variables. As long as
-  // lexical variables are not used extensively, this is a performance win.
-  // TODO(yangguo): reconsider this shortcut.
-  if (ScriptContextTable::LookupLexical(script_contexts, name)) {
+  ScriptContextTable::LookupResult lookup;
+  if (ScriptContextTable::Lookup(script_contexts, name, &lookup) &&
+      IsLexicalVariableMode(lookup.mode)) {
     return ThrowRedeclarationError(isolate, name);
   }
 
@@ -625,13 +624,9 @@ static Object* FindNameClash(Handle<ScopeInfo> scope_info,
   for (int var = 0; var < scope_info->ContextLocalCount(); var++) {
     Handle<String> name(scope_info->ContextLocalName(var));
     VariableMode mode = scope_info->ContextLocalMode(var);
-    if (IsLexicalVariableMode(mode)) {
     ScriptContextTable::LookupResult lookup;
     if (ScriptContextTable::Lookup(script_context, name, &lookup)) {
-        return ThrowRedeclarationError(isolate, name);
-      }
-    } else {
-      if (ScriptContextTable::LookupLexical(script_context, name)) {
+      if (IsLexicalVariableMode(mode) || IsLexicalVariableMode(lookup.mode)) {
         return ThrowRedeclarationError(isolate, name);
       }
     }

src/scopeinfo.cc

@@ -149,8 +149,6 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone,
 
   // Add context locals' info.
   DCHECK(index == scope_info->ContextLocalInfoEntriesIndex());
-  bool encountered_lexical = false;
-  int lexical_context_local_count = 0;
   for (int i = 0; i < context_local_count; ++i) {
     Variable* var = context_locals[i];
     uint32_t value =
@@ -158,15 +156,7 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone,
         ContextLocalInitFlag::encode(var->initialization_flag()) |
         ContextLocalMaybeAssignedFlag::encode(var->maybe_assigned());
     scope_info->set(index++, Smi::FromInt(value));
-    if (encountered_lexical) {
-      // Check that context locals are sorted so that lexicals are at the end.
-      DCHECK(IsLexicalVariableMode(var->mode()));
-    } else if (IsLexicalVariableMode(var->mode())) {
-      lexical_context_local_count = context_local_count - i;
   }
-  }
-
-  scope_info->SetLexicalContextLocalCount(lexical_context_local_count);
 
   // Add context globals' info.
   DCHECK(index == scope_info->ContextGlobalInfoEntriesIndex());
@@ -232,7 +222,6 @@ Handle<ScopeInfo> ScopeInfo::CreateGlobalThisBinding(Isolate* isolate) {
 
   const int stack_local_count = 0;
   const int context_local_count = 1;
-  const int lexical_context_local_count = 1;
   const int context_global_count = 0;
   const int strong_mode_free_variable_count = 0;
   const bool has_simple_parameters = true;
@@ -265,7 +254,6 @@ Handle<ScopeInfo> ScopeInfo::CreateGlobalThisBinding(Isolate* isolate) {
   scope_info->SetParameterCount(parameter_count);
   scope_info->SetStackLocalCount(stack_local_count);
   scope_info->SetContextLocalCount(context_local_count);
-  scope_info->SetLexicalContextLocalCount(lexical_context_local_count);
   scope_info->SetContextGlobalCount(context_global_count);
   scope_info->SetStrongModeFreeVariableCount(strong_mode_free_variable_count);
 
@@ -584,31 +572,6 @@ int ScopeInfo::ContextSlotIndex(Handle<ScopeInfo> scope_info,
 }
 
 
-int ScopeInfo::LexicalContextSlotIndex(Handle<ScopeInfo> scope_info,
-                                       Handle<String> name) {
-  DCHECK(name->IsInternalizedString());
-  if (scope_info->length() > 0) {
-    // TODO(yangguo): consider using the context slot cache here.
-    int total_count = scope_info->ContextLocalCount();
-    int lexical_count = scope_info->LexicalContextLocalCount();
-    int non_lexical_count = total_count - lexical_count;
-
-    int start = scope_info->ContextLocalNameEntriesIndex();
-    int end = start + total_count;
-    int lexical_start = start + non_lexical_count;
-
-    for (int i = lexical_start; i < end; ++i) {
-      if (*name == scope_info->get(i)) {
-        int var = i - start;
-        DCHECK(IsLexicalVariableMode(scope_info->ContextLocalMode(var)));
-        return Context::MIN_CONTEXT_SLOTS + var;
-      }
-    }
-  }
-  return -1;
-}
-
-
 String* ScopeInfo::ContextSlotName(int slot_index) {
   int const var = slot_index - Context::MIN_CONTEXT_SLOTS;
   DCHECK_LE(0, var);

src/scopes.cc

@@ -596,11 +596,7 @@ class VarAndOrder {
   Variable* var() const { return var_; }
   int order() const { return order_; }
   static int Compare(const VarAndOrder* a, const VarAndOrder* b) {
-    // Sort lexical variables to the end of the list.
-    bool a_is_lexical = IsLexicalVariableMode(a->var()->mode());
-    bool b_is_lexical = IsLexicalVariableMode(b->var()->mode());
-    if (a_is_lexical == b_is_lexical) return a->order_ - b->order_;
-    return a_is_lexical ? 1 : -1;
+    return a->order_ - b->order_;
   }
 
  private: