[parser] Remove ContextSlotCache

Now that we always cache lookups through scope-info-backed scopes on the
entry-point scope-info-backed scope, we don't need additional caching
per scope-info. The one missing piece was negative lookups, but they
automatically turn into DynamicGlobals which we also cache on the entry
scope.

The one possible difference is that we don't cache across compilation,
but seems unlikely to be very beneficial. We'll keep an eye out for
regressions though.


Change-Id: I23186d2b085d2042fafa32fb3cca88f88c61074c
Reviewed-on: https://chromium-review.googlesource.com/c/1337731
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57544}

BUILD.gn

@@ -1615,8 +1615,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"
-#include "src/objects-inl.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);
-  DCHECK(name->IsInternalizedString());
-  Key& key = keys_[index];
-  if (key.data == data && key.name == 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) {
-  DCHECK(name->IsInternalizedString());
-  DCHECK_LT(kNotFound, slot_index);
-  int index = Hash(*data, *name);
-  Key& key = keys_[index];
-  key.data = *data;
-  key.name = *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) {
-  DCHECK(name->IsInternalizedString());
-  int index = Hash(*data, *name);
-  Key& key = keys_[index];
-  DCHECK_EQ(key.data, *data);
-  DCHECK_EQ(key.name, *name);
-  Value result(values_[index]);
-  DCHECK_EQ(result.mode(), mode);
-  DCHECK_EQ(result.initialization_flag(), init_flag);
-  DCHECK_EQ(result.maybe_assigned_flag(), maybe_assigned_flag);
-  DCHECK_EQ(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"
-
-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 = nullptr;
-      keys_[i].name = nullptr;
-      values_[i] = static_cast<uint32_t>(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/heap/heap.cc

@@ -10,7 +10,6 @@
 #include "src/accessors.h"
 #include "src/api-inl.h"
 #include "src/assembler-inl.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"
@@ -1858,7 +1857,6 @@ void Heap::MarkCompactEpilogue() {
 
 void Heap::MarkCompactPrologue() {
   TRACE_GC(tracer(), GCTracer::Scope::MC_PROLOGUE);
-  isolate_->context_slot_cache()->Clear();
   isolate_->descriptor_lookup_cache()->Clear();
   RegExpResultsCache::Clear(string_split_cache());
   RegExpResultsCache::Clear(regexp_multiple_cache());

src/heap/setup-heap-internal.cc

@@ -5,7 +5,6 @@
 #include "src/setup-isolate.h"
 
 #include "src/accessors.h"
-#include "src/ast/context-slot-cache.h"
 #include "src/compilation-cache.h"
 #include "src/contexts.h"
 #include "src/heap-symbols.h"
@@ -910,9 +909,6 @@ void Heap::CreateInitialObjects() {
   // Initialize builtins constants table.
   set_builtins_constants_table(roots.empty_fixed_array());
 
-  // Initialize context slot cache.
-  isolate_->context_slot_cache()->Clear();
-
   // Initialize descriptor cache.
   isolate_->descriptor_lookup_cache()->Clear();
 

src/isolate.cc

@@ -15,7 +15,6 @@
 #include "src/api-inl.h"
 #include "src/assembler-inl.h"
 #include "src/ast/ast-value-factory.h"
-#include "src/ast/context-slot-cache.h"
 #include "src/ast/scopes.h"
 #include "src/base/adapters.h"
 #include "src/base/hashmap.h"
@@ -2958,8 +2957,6 @@ Isolate::~Isolate() {
 
   delete descriptor_lookup_cache_;
   descriptor_lookup_cache_ = nullptr;
-  delete context_slot_cache_;
-  context_slot_cache_ = nullptr;
 
   delete load_stub_cache_;
   load_stub_cache_ = nullptr;
@@ -3211,7 +3208,6 @@ bool Isolate::Init(StartupDeserializer* des) {
 #undef ASSIGN_ELEMENT
 
   compilation_cache_ = new CompilationCache(this);
-  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

@@ -1028,10 +1028,6 @@ class Isolate final : private HiddenFactory {
     return materialized_object_store_;
   }
 
-  ContextSlotCache* context_slot_cache() {
-    return context_slot_cache_;
-  }
-
   DescriptorLookupCache* descriptor_lookup_cache() {
     return descriptor_lookup_cache_;
   }
@@ -1712,7 +1708,6 @@ class Isolate final : private HiddenFactory {
   int stack_trace_for_uncaught_exceptions_frame_limit_ = 0;
   StackTrace::StackTraceOptions stack_trace_for_uncaught_exceptions_options_ =
       StackTrace::kOverview;
-  ContextSlotCache* context_slot_cache_ = nullptr;
   DescriptorLookupCache* descriptor_lookup_cache_ = nullptr;
   HandleScopeData handle_scope_data_;
   HandleScopeImplementer* handle_scope_implementer_ = nullptr;

src/objects/scope-info.cc

@@ -6,7 +6,6 @@
 
 #include "src/objects/scope-info.h"
 
-#include "src/ast/context-slot-cache.h"
 #include "src/ast/scopes.h"
 #include "src/ast/variables.h"
 #include "src/bootstrapper.h"
@@ -712,26 +711,6 @@ int ScopeInfo::ContextSlotIndex(Handle<ScopeInfo> scope_info,
 
   if (scope_info->length() == 0) return -1;
 
-  // Get the Isolate via the heap.
-  //
-  // Ideally we'd pass Isolate* through to this function, however this is mostly
-  // called from the parser, which is otherwise isolate independent. We can't
-  // assume that all scope infos are never RO space (like we can with JSReceiver
-  // or Context), but we can assume that *non-empty* scope infos are.
-  //
-  // So, we take the least-ugly approach of manually getting the isolate to be
-  // able to remove GetIsolate from ScopeInfo in the general case, while
-  // allowing it in this one particular case.
-  Isolate* isolate = Heap::FromWritableHeapObject(*scope_info)->isolate();
-
-  ContextSlotCache* context_slot_cache = isolate->context_slot_cache();
-  int result = context_slot_cache->Lookup(*scope_info, *name, mode, init_flag,
-                                          maybe_assigned_flag);
-  if (result != ContextSlotCache::kNotFound) {
-    DCHECK_LT(result, scope_info->ContextLength());
-    return result;
-  }
-
   int start = scope_info->ContextLocalNamesIndex();
   int end = start + scope_info->ContextLocalCount();
   for (int i = start; i < end; ++i) {
@@ -740,17 +719,12 @@ 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;
+      int result = Context::MIN_CONTEXT_SLOTS + var;
 
-      context_slot_cache->Update(scope_info, name, *mode, *init_flag,
-                                 *maybe_assigned_flag, result);
       DCHECK_LT(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, VariableMode::kTemporary,
-                             kNeedsInitialization, kNotAssigned, -1);
 
   return -1;
 }