Always deserialize scope infos for parsing

When looking up variables in the ScopeInfo, we did a linear scan of the
ScopeInfo. Since that's unacceptably slow, a context slot cache was added
that would speed up repeated lookups of the same variable.

Instead, just always fully convert the ScopeInfo into scopes, so they can
lookup variables without scanning the ScopeInfo.

This also allows for removing the now unused ContextSlotCache.

R=adamk@chromium.org,verwaest@chromium.org,marja@chromium.org
BUG=v8:5315

Review-Url: https://codereview.chromium.org/2280933002
Cr-Commit-Position: refs/heads/master@{#38953}

BUILD.gn

@@ -861,8 +861,6 @@ v8_source_set("v8_base") {
     "src/ast/ast-value-factory.h",
     "src/ast/ast.cc",
     "src/ast/ast.h",
-    "src/ast/context-slot-cache.cc",
-    "src/ast/context-slot-cache.h",
     "src/ast/modules.cc",
     "src/ast/modules.h",
     "src/ast/prettyprinter.cc",

src/ast/context-slot-cache.cc

-// Copyright 2016 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/ast/context-slot-cache.h"
-
-#include <stdlib.h>
-
-#include "src/ast/scopes.h"
-#include "src/bootstrapper.h"
-
-namespace v8 {
-namespace internal {
-
-int ContextSlotCache::Hash(Object* data, String* name) {
-  // Uses only lower 32 bits if pointers are larger.
-  uintptr_t addr_hash =
-      static_cast<uint32_t>(reinterpret_cast<uintptr_t>(data)) >> 2;
-  return static_cast<int>((addr_hash ^ name->Hash()) % kLength);
-}
-
-int ContextSlotCache::Lookup(Object* data, String* name, VariableMode* mode,
-                             InitializationFlag* init_flag,
-                             MaybeAssignedFlag* maybe_assigned_flag) {
-  int index = Hash(data, name);
-  Key& key = keys_[index];
-  if ((key.data == data) && key.name->Equals(name)) {
-    Value result(values_[index]);
-    if (mode != nullptr) *mode = result.mode();
-    if (init_flag != nullptr) *init_flag = result.initialization_flag();
-    if (maybe_assigned_flag != nullptr)
-      *maybe_assigned_flag = result.maybe_assigned_flag();
-    return result.index() + kNotFound;
-  }
-  return kNotFound;
-}
-
-void ContextSlotCache::Update(Handle<Object> data, Handle<String> name,
-                              VariableMode mode, InitializationFlag init_flag,
-                              MaybeAssignedFlag maybe_assigned_flag,
-                              int slot_index) {
-  DisallowHeapAllocation no_gc;
-  Handle<String> internalized_name;
-  DCHECK(slot_index > kNotFound);
-  if (StringTable::InternalizeStringIfExists(name->GetIsolate(), name)
-          .ToHandle(&internalized_name)) {
-    int index = Hash(*data, *internalized_name);
-    Key& key = keys_[index];
-    key.data = *data;
-    key.name = *internalized_name;
-    // Please note value only takes a uint as index.
-    values_[index] =
-        Value(mode, init_flag, maybe_assigned_flag, slot_index - kNotFound)
-            .raw();
-#ifdef DEBUG
-    ValidateEntry(data, name, mode, init_flag, maybe_assigned_flag, slot_index);
-#endif
-  }
-}
-
-void ContextSlotCache::Clear() {
-  for (int index = 0; index < kLength; index++) keys_[index].data = nullptr;
-}
-
-#ifdef DEBUG
-
-void ContextSlotCache::ValidateEntry(Handle<Object> data, Handle<String> name,
-                                     VariableMode mode,
-                                     InitializationFlag init_flag,
-                                     MaybeAssignedFlag maybe_assigned_flag,
-                                     int slot_index) {
-  DisallowHeapAllocation no_gc;
-  Handle<String> internalized_name;
-  if (StringTable::InternalizeStringIfExists(name->GetIsolate(), name)
-          .ToHandle(&internalized_name)) {
-    int index = Hash(*data, *name);
-    Key& key = keys_[index];
-    DCHECK(key.data == *data);
-    DCHECK(key.name->Equals(*name));
-    Value result(values_[index]);
-    DCHECK(result.mode() == mode);
-    DCHECK(result.initialization_flag() == init_flag);
-    DCHECK(result.maybe_assigned_flag() == maybe_assigned_flag);
-    DCHECK(result.index() + kNotFound == slot_index);
-  }
-}
-
-#endif  // DEBUG
-
-}  // namespace internal
-}  // namespace v8

src/ast/context-slot-cache.h

-// Copyright 2016 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.
-
-#ifndef V8_AST_CONTEXT_SLOT_CACHE_H_
-#define V8_AST_CONTEXT_SLOT_CACHE_H_
-
-#include "src/allocation.h"
-#include "src/ast/modules.h"
-#include "src/ast/variables.h"
-
-namespace v8 {
-namespace internal {
-
-// Cache for mapping (data, property name) into context slot index.
-// The cache contains both positive and negative results.
-// Slot index equals -1 means the property is absent.
-// Cleared at startup and prior to mark sweep collection.
-class ContextSlotCache {
- public:
-  // Lookup context slot index for (data, name).
-  // If absent, kNotFound is returned.
-  int Lookup(Object* data, String* name, VariableMode* mode,
-             InitializationFlag* init_flag,
-             MaybeAssignedFlag* maybe_assigned_flag);
-
-  // Update an element in the cache.
-  void Update(Handle<Object> data, Handle<String> name, VariableMode mode,
-              InitializationFlag init_flag,
-              MaybeAssignedFlag maybe_assigned_flag, int slot_index);
-
-  // Clear the cache.
-  void Clear();
-
-  static const int kNotFound = -2;
-
- private:
-  ContextSlotCache() {
-    for (int i = 0; i < kLength; ++i) {
-      keys_[i].data = NULL;
-      keys_[i].name = NULL;
-      values_[i] = kNotFound;
-    }
-  }
-
-  inline static int Hash(Object* data, String* name);
-
-#ifdef DEBUG
-  void ValidateEntry(Handle<Object> data, Handle<String> name,
-                     VariableMode mode, InitializationFlag init_flag,
-                     MaybeAssignedFlag maybe_assigned_flag, int slot_index);
-#endif
-
-  static const int kLength = 256;
-  struct Key {
-    Object* data;
-    String* name;
-  };
-
-  struct Value {
-    Value(VariableMode mode, InitializationFlag init_flag,
-          MaybeAssignedFlag maybe_assigned_flag, int index) {
-      DCHECK(ModeField::is_valid(mode));
-      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) |
-               InitField::encode(init_flag) |
-               MaybeAssignedField::encode(maybe_assigned_flag);
-      DCHECK(mode == this->mode());
-      DCHECK(init_flag == this->initialization_flag());
-      DCHECK(maybe_assigned_flag == this->maybe_assigned_flag());
-      DCHECK(index == this->index());
-    }
-
-    explicit inline Value(uint32_t value) : value_(value) {}
-
-    uint32_t raw() { return value_; }
-
-    VariableMode mode() { return ModeField::decode(value_); }
-
-    InitializationFlag initialization_flag() {
-      return InitField::decode(value_);
-    }
-
-    MaybeAssignedFlag maybe_assigned_flag() {
-      return MaybeAssignedField::decode(value_);
-    }
-
-    int index() { return IndexField::decode(value_); }
-
-    // Bit fields in value_ (type, shift, size). Must be public so the
-    // constants can be embedded in generated code.
-    class ModeField : public BitField<VariableMode, 0, 4> {};
-    class InitField : public BitField<InitializationFlag, 4, 1> {};
-    class MaybeAssignedField : public BitField<MaybeAssignedFlag, 5, 1> {};
-    class IndexField : public BitField<int, 6, 32 - 6> {};
-
-   private:
-    uint32_t value_;
-  };
-
-  Key keys_[kLength];
-  uint32_t values_[kLength];
-
-  friend class Isolate;
-  DISALLOW_COPY_AND_ASSIGN(ContextSlotCache);
-};
-
-}  // namespace internal
-}  // namespace v8
-
-#endif  // V8_AST_CONTEXT_SLOT_CACHE_H_

src/ast/scopeinfo.cc

@@ -4,7 +4,6 @@
 
 #include <stdlib.h>
 
-#include "src/ast/context-slot-cache.h"
 #include "src/ast/scopes.h"
 #include "src/bootstrapper.h"
 
@@ -477,15 +476,6 @@ int ScopeInfo::ContextSlotIndex(Handle<ScopeInfo> scope_info,
   DCHECK_NOT_NULL(maybe_assigned_flag);
 
   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, init_flag,
-                                            maybe_assigned_flag);
-    if (result != ContextSlotCache::kNotFound) {
-      DCHECK(result < scope_info->ContextLength());
-      return result;
-    }
-
     int start = scope_info->ContextLocalNameEntriesIndex();
     int end = start + scope_info->ContextLocalCount();
     for (int i = start; i < end; ++i) {
@@ -494,17 +484,11 @@ int ScopeInfo::ContextSlotIndex(Handle<ScopeInfo> scope_info,
         *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);
+        int result = Context::MIN_CONTEXT_SLOTS + var;
         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;

src/ast/scopes.cc

@@ -286,6 +286,10 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
       // nesting script scopes.
       Handle<ScopeInfo> scope_info(context->scope_info(), isolate);
       script_scope->SetScriptScopeInfo(scope_info);
+      script_scope->DeserializeScopeInfo(isolate, ast_value_factory);
+      if (deserialization_mode == DeserializationMode::kDeserializeOffHeap) {
+        script_scope->scope_info_ = Handle<ScopeInfo>::null();
+      }
       DCHECK(context->previous()->IsNativeContext());
       break;
     } else if (context->IsFunctionContext()) {
@@ -321,8 +325,9 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
       outer_scope->AddInnerScope(current_scope);
     }
     current_scope = outer_scope;
+    current_scope->DeserializeScopeInfo(isolate, ast_value_factory);
     if (deserialization_mode == DeserializationMode::kDeserializeOffHeap) {
-      current_scope->DeserializeScopeInfo(isolate, ast_value_factory);
+      current_scope->scope_info_ = Handle<ScopeInfo>::null();
     }
     if (innermost_scope == nullptr) innermost_scope = current_scope;
     context = context->previous();
@@ -398,8 +403,6 @@ void Scope::DeserializeScopeInfo(Isolate* isolate,
       result->AllocateTo(VariableLocation::CONTEXT, index);
     }
   }
-
-  scope_info_ = Handle<ScopeInfo>::null();
 }
 
 DeclarationScope* Scope::AsDeclarationScope() {
@@ -609,75 +612,9 @@ void Scope::PropagateUsageFlagsToScope(Scope* other) {
   if (calls_eval()) other->RecordEvalCall();
 }
 
-Variable* Scope::LookupInScopeInfo(const AstRawString* name) {
-  Handle<String> name_handle = name->string();
-  // The Scope is backed up by ScopeInfo. This means it cannot operate in a
-  // heap-independent mode, and all strings must be internalized immediately. So
-  // it's ok to get the Handle<String> here.
-  // If we have a serialized scope info, we might find the variable there.
-  // There should be no local slot with the given name.
-  DCHECK(scope_info_->StackSlotIndex(*name_handle) < 0);
-
-  VariableMode mode;
-  InitializationFlag init_flag;
-  MaybeAssignedFlag maybe_assigned_flag;
-
-  VariableLocation location = VariableLocation::CONTEXT;
-  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);
-    DCHECK(index < 0 || (is_script_scope() && mode == VAR));
-  }
-  if (index < 0) {
-    location = VariableLocation::LOOKUP;
-    index = scope_info_->ParameterIndex(*name_handle);
-    if (index >= 0) {
-      mode = DYNAMIC;
-      init_flag = kCreatedInitialized;
-      // Be conservative and flag parameters as maybe assigned. Better
-      // information would require ScopeInfo to serialize the maybe_assigned bit
-      // also for parameters.
-      maybe_assigned_flag = kMaybeAssigned;
-    }
-  }
-  if (index < 0 && scope_type() == MODULE_SCOPE) {
-    location = VariableLocation::MODULE;
-    index = -1;  // TODO(neis): Find module variables in scope info.
-  }
-  if (index < 0) return nullptr;  // Nowhere found.
-
-  Variable::Kind kind = Variable::NORMAL;
-  if (location == VariableLocation::CONTEXT &&
-      index == scope_info_->ReceiverContextSlotIndex()) {
-    kind = Variable::THIS;
-  }
-  // TODO(marja, rossberg): Correctly declare FUNCTION, CLASS, NEW_TARGET, and
-  // ARGUMENTS bindings as their corresponding Variable::Kind.
-
-  Variable* var = variables_.Declare(zone(), this, name, mode, kind, init_flag,
-                                     maybe_assigned_flag);
-  var->AllocateTo(location, index);
-  return var;
-}
-
 Variable* DeclarationScope::LookupFunctionVar(const AstRawString* name) {
-  if (function_ != nullptr && function_->raw_name() == name) {
-    return function_;
-  } else if (!scope_info_.is_null()) {
-    // If we are backed by a scope info, try to lookup the variable there.
-    VariableMode mode;
-    int index = scope_info_->FunctionContextSlotIndex(*(name->string()), &mode);
-    if (index < 0) return nullptr;
-    Variable* var = DeclareFunctionVar(name);
-    DCHECK_EQ(mode, var->mode());
-    var->AllocateTo(VariableLocation::CONTEXT, index);
-    return var;
-  } else {
-    return nullptr;
-  }
+  if (function_ != nullptr && function_->raw_name() == name) return function_;
+  return nullptr;
 }
 
 

src/ast/scopes.h

@@ -124,13 +124,9 @@ class Scope: public ZoneObject {
 
   // Lookup a variable in this scope. Returns the variable or NULL if not found.
   Variable* LookupLocal(const AstRawString* name) {
-    Variable* result = variables_.Lookup(name);
-    if (result != nullptr || scope_info_.is_null()) return result;
-    return LookupInScopeInfo(name);
+    return variables_.Lookup(name);
   }
 
-  Variable* LookupInScopeInfo(const AstRawString* name);
-
   // Lookup a variable in this scope or outer scopes.
   // Returns the variable or NULL if not found.
   Variable* Lookup(const AstRawString* name);

src/heap/heap.cc

@@ -6,7 +6,6 @@
 
 #include "src/accessors.h"
 #include "src/api.h"
-#include "src/ast/context-slot-cache.h"
 #include "src/base/bits.h"
 #include "src/base/once.h"
 #include "src/base/utils/random-number-generator.h"
@@ -1475,7 +1474,6 @@ void Heap::MarkCompactPrologue() {
   // At any old GC clear the keyed lookup cache to enable collection of unused
   // maps.
   isolate_->keyed_lookup_cache()->Clear();
-  isolate_->context_slot_cache()->Clear();
   isolate_->descriptor_lookup_cache()->Clear();
   RegExpResultsCache::Clear(string_split_cache());
   RegExpResultsCache::Clear(regexp_multiple_cache());
@@ -2891,9 +2889,6 @@ void Heap::CreateInitialObjects() {
   // Initialize keyed lookup cache.
   isolate_->keyed_lookup_cache()->Clear();
 
-  // Initialize context slot cache.
-  isolate_->context_slot_cache()->Clear();
-
   // Initialize descriptor cache.
   isolate_->descriptor_lookup_cache()->Clear();
 

src/isolate.cc

@@ -9,7 +9,6 @@
 #include <fstream>  // NOLINT(readability/streams)
 #include <sstream>
 
-#include "src/ast/context-slot-cache.h"
 #include "src/base/accounting-allocator.h"
 #include "src/base/hashmap.h"
 #include "src/base/platform/platform.h"
@@ -1944,7 +1943,6 @@ Isolate::Isolate(bool enable_serializer)
       stack_trace_for_uncaught_exceptions_frame_limit_(0),
       stack_trace_for_uncaught_exceptions_options_(StackTrace::kOverview),
       keyed_lookup_cache_(NULL),
-      context_slot_cache_(NULL),
       descriptor_lookup_cache_(NULL),
       handle_scope_implementer_(NULL),
       unicode_cache_(NULL),
@@ -2172,8 +2170,6 @@ Isolate::~Isolate() {
 
   delete descriptor_lookup_cache_;
   descriptor_lookup_cache_ = NULL;
-  delete context_slot_cache_;
-  context_slot_cache_ = NULL;
   delete keyed_lookup_cache_;
   keyed_lookup_cache_ = NULL;
 
@@ -2324,7 +2320,6 @@ bool Isolate::Init(Deserializer* des) {
 
   compilation_cache_ = new CompilationCache(this);
   keyed_lookup_cache_ = new KeyedLookupCache();
-  context_slot_cache_ = new ContextSlotCache();
   descriptor_lookup_cache_ = new DescriptorLookupCache();
   unicode_cache_ = new UnicodeCache();
   inner_pointer_to_code_cache_ = new InnerPointerToCodeCache(this);

src/isolate.h

@@ -874,10 +874,6 @@ class Isolate {
     return keyed_lookup_cache_;
   }
 
-  ContextSlotCache* context_slot_cache() {
-    return context_slot_cache_;
-  }
-
   DescriptorLookupCache* descriptor_lookup_cache() {
     return descriptor_lookup_cache_;
   }
@@ -1319,7 +1315,6 @@ class Isolate {
   int stack_trace_for_uncaught_exceptions_frame_limit_;
   StackTrace::StackTraceOptions stack_trace_for_uncaught_exceptions_options_;
   KeyedLookupCache* keyed_lookup_cache_;
-  ContextSlotCache* context_slot_cache_;
   DescriptorLookupCache* descriptor_lookup_cache_;
   HandleScopeData handle_scope_data_;
   HandleScopeImplementer* handle_scope_implementer_;

src/v8.gyp

@@ -455,8 +455,6 @@
         'ast/ast-value-factory.h',
         'ast/ast.cc',
         'ast/ast.h',
-        'ast/context-slot-cache.cc',
-        'ast/context-slot-cache.h',
         'ast/modules.cc',
         'ast/modules.h',
         'ast/prettyprinter.cc',