Revert of Always deserialize scope infos for parsing (patchset #3 id:40001 of...

Revert of Always deserialize scope infos for parsing (patchset #3 id:40001 of https://codereview.chromium.org/2280933002/ )

Reason for revert:
Significantly tanks parsing. We probably should just keep on doing what we're doing: partially deserialize while resolving variables. If we do scope-info backed resolution after regular resolution based on remaining free variables, we can probably reduce the time-frame of that part. We soon after anyway need to sync with the main thread.

Original issue's description:
> 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
>
> Committed: https://crrev.com/81f824cad18e4dc873a8838943217eb9c9f0c1f0
> Cr-Commit-Position: refs/heads/master@{#38953}

TBR=adamk@chromium.org,marja@chromium.org,jochen@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5315

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

BUILD.gn

@@ -861,6 +861,8 @@ 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,6 +4,7 @@
 
 #include <stdlib.h>
 
+#include "src/ast/context-slot-cache.h"
 #include "src/ast/scopes.h"
 #include "src/bootstrapper.h"
 
@@ -476,6 +477,15 @@ 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) {
@@ -484,11 +494,17 @@ 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);
-        int result = Context::MIN_CONTEXT_SLOTS + 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;

src/ast/scopes.cc

@@ -286,10 +286,6 @@ 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()) {
@@ -325,9 +321,8 @@ 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->scope_info_ = Handle<ScopeInfo>::null();
+      current_scope->DeserializeScopeInfo(isolate, ast_value_factory);
     }
     if (innermost_scope == nullptr) innermost_scope = current_scope;
     context = context->previous();
@@ -403,6 +398,8 @@ void Scope::DeserializeScopeInfo(Isolate* isolate,
       result->AllocateTo(VariableLocation::CONTEXT, index);
     }
   }
+
+  scope_info_ = Handle<ScopeInfo>::null();
 }
 
 DeclarationScope* Scope::AsDeclarationScope() {
@@ -612,9 +609,75 @@ 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_;
-  return nullptr;
+  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;
+  }
 }
 
 

src/ast/scopes.h

@@ -124,9 +124,13 @@ class Scope: public ZoneObject {
 
   // Lookup a variable in this scope. Returns the variable or NULL if not found.
   Variable* LookupLocal(const AstRawString* name) {
-    return variables_.Lookup(name);
+    Variable* result = variables_.Lookup(name);
+    if (result != nullptr || scope_info_.is_null()) return result;
+    return LookupInScopeInfo(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,6 +6,7 @@
 
 #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"
@@ -1474,6 +1475,7 @@ 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());
@@ -2889,6 +2891,9 @@ 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,6 +9,7 @@
 #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"
@@ -1943,6 +1944,7 @@ 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),
@@ -2170,6 +2172,8 @@ 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;
 
@@ -2320,6 +2324,7 @@ 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,6 +874,10 @@ class Isolate {
     return keyed_lookup_cache_;
   }
 
+  ContextSlotCache* context_slot_cache() {
+    return context_slot_cache_;
+  }
+
   DescriptorLookupCache* descriptor_lookup_cache() {
     return descriptor_lookup_cache_;
   }
@@ -1315,6 +1319,7 @@ 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,6 +455,8 @@
         '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',