Canonicalize handles for optimized compilation.

R=bmeurer@chromium.org

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

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

include/v8.h

@@ -966,8 +966,8 @@ class V8_EXPORT SealHandleScope {
   void operator delete(void*, size_t);
 
   internal::Isolate* isolate_;
-  int prev_level_;
   internal::Object** prev_limit_;
+  int prev_sealed_level_;
 };
 
 

src/allocation-site-scopes.h

@@ -36,7 +36,7 @@ class AllocationSiteContext {
 
   void InitializeTraversal(Handle<AllocationSite> site) {
     top_ = site;
-    current_ = Handle<AllocationSite>(*top_, isolate());
+    current_ = Handle<AllocationSite>::New(*top_, isolate());
   }
 
  private:

src/api.cc

@@ -734,17 +734,17 @@ SealHandleScope::SealHandleScope(Isolate* isolate) {
   i::HandleScopeData* current = internal_isolate->handle_scope_data();
   prev_limit_ = current->limit;
   current->limit = current->next;
-  prev_level_ = current->level;
-  current->level = 0;
+  prev_sealed_level_ = current->sealed_level;
+  current->sealed_level = current->level;
 }
 
 
 SealHandleScope::~SealHandleScope() {
   i::HandleScopeData* current = isolate_->handle_scope_data();
-  DCHECK_EQ(0, current->level);
-  current->level = prev_level_;
   DCHECK_EQ(current->next, current->limit);
   current->limit = prev_limit_;
+  DCHECK_EQ(current->level, current->sealed_level);
+  current->sealed_level = prev_sealed_level_;
 }
 
 

src/arm/macro-assembler-arm.cc

@@ -24,8 +24,8 @@ MacroAssembler::MacroAssembler(Isolate* arg_isolate, void* buffer, int size)
       generating_stub_(false),
       has_frame_(false) {
   if (isolate() != NULL) {
-    code_object_ = Handle<Object>(isolate()->heap()->undefined_value(),
-                                  isolate());
+    code_object_ =
+        Handle<Object>::New(isolate()->heap()->undefined_value(), isolate());
   }
 }
 

src/arm64/macro-assembler-arm64.cc

@@ -36,8 +36,8 @@ MacroAssembler::MacroAssembler(Isolate* arg_isolate,
       tmp_list_(DefaultTmpList()),
       fptmp_list_(DefaultFPTmpList()) {
   if (isolate() != NULL) {
-    code_object_ = Handle<Object>(isolate()->heap()->undefined_value(),
-                                  isolate());
+    code_object_ =
+        Handle<Object>::New(isolate()->heap()->undefined_value(), isolate());
   }
 }
 

src/compiler.cc

@@ -863,9 +863,12 @@ bool Compiler::ParseAndAnalyze(ParseInfo* info) {
 
 
 static bool GetOptimizedCodeNow(CompilationInfo* info) {
+  Isolate* isolate = info->isolate();
+  CanonicalHandleScope canonical(isolate);
+
   if (!Compiler::ParseAndAnalyze(info->parse_info())) return false;
 
-  TimerEventScope<TimerEventRecompileSynchronous> timer(info->isolate());
+  TimerEventScope<TimerEventRecompileSynchronous> timer(isolate);
 
   OptimizedCompileJob job(info);
   if (job.CreateGraph() != OptimizedCompileJob::SUCCEEDED ||
@@ -880,7 +883,7 @@ static bool GetOptimizedCodeNow(CompilationInfo* info) {
   }
 
   // Success!
-  DCHECK(!info->isolate()->has_pending_exception());
+  DCHECK(!isolate->has_pending_exception());
   InsertCodeIntoOptimizedCodeMap(info);
   RecordFunctionCompilation(Logger::LAZY_COMPILE_TAG, info,
                             info->shared_info());
@@ -890,6 +893,8 @@ static bool GetOptimizedCodeNow(CompilationInfo* info) {
 
 static bool GetOptimizedCodeLater(CompilationInfo* info) {
   Isolate* isolate = info->isolate();
+  CanonicalHandleScope canonical(isolate);
+
   if (!isolate->optimizing_compile_dispatcher()->IsQueueAvailable()) {
     if (FLAG_trace_concurrent_recompilation) {
       PrintF("  ** Compilation queue full, will retry optimizing ");

src/handles-inl.h

@@ -14,15 +14,23 @@ namespace v8 {
 namespace internal {
 
 HandleBase::HandleBase(Object* object, Isolate* isolate)
-    : location_(HandleScope::CreateHandle(isolate, object)) {}
+    : location_(HandleScope::GetHandle(isolate, object)) {}
+
+
+template <typename T>
+// Allocate a new handle for the object, do not canonicalize.
+Handle<T> Handle<T>::New(T* object, Isolate* isolate) {
+  return Handle(
+      reinterpret_cast<T**>(HandleScope::CreateHandle(isolate, object)));
+}
 
 
 HandleScope::HandleScope(Isolate* isolate) {
-  HandleScopeData* current = isolate->handle_scope_data();
+  HandleScopeData* data = isolate->handle_scope_data();
   isolate_ = isolate;
-  prev_next_ = current->next;
-  prev_limit_ = current->limit;
-  current->level++;
+  prev_next_ = data->next;
+  prev_limit_ = data->limit;
+  data->level++;
 }
 
 
@@ -76,7 +84,7 @@ Handle<T> HandleScope::CloseAndEscape(Handle<T> handle_value) {
   // Throw away all handles in the current scope.
   CloseScope(isolate_, prev_next_, prev_limit_);
   // Allocate one handle in the parent scope.
-  DCHECK(current->level > 0);
+  DCHECK(current->level > current->sealed_level);
   Handle<T> result(value, isolate_);
   // Reinitialize the current scope (so that it's ready
   // to be used or closed again).
@@ -87,24 +95,30 @@ Handle<T> HandleScope::CloseAndEscape(Handle<T> handle_value) {
 }
 
 
-template <typename T>
-T** HandleScope::CreateHandle(Isolate* isolate, T* value) {
+Object** HandleScope::CreateHandle(Isolate* isolate, Object* value) {
   DCHECK(AllowHandleAllocation::IsAllowed());
-  HandleScopeData* current = isolate->handle_scope_data();
+  HandleScopeData* data = isolate->handle_scope_data();
 
-  Object** cur = current->next;
-  if (cur == current->limit) cur = Extend(isolate);
+  Object** result = data->next;
+  if (result == data->limit) result = Extend(isolate);
   // Update the current next field, set the value in the created
   // handle, and return the result.
-  DCHECK(cur < current->limit);
-  current->next = cur + 1;
+  DCHECK(result < data->limit);
+  data->next = result + 1;
 
-  T** result = reinterpret_cast<T**>(cur);
   *result = value;
   return result;
 }
 
 
+Object** HandleScope::GetHandle(Isolate* isolate, Object* value) {
+  DCHECK(AllowHandleAllocation::IsAllowed());
+  HandleScopeData* data = isolate->handle_scope_data();
+  CanonicalHandleScope* canonical = data->canonical_scope;
+  return canonical ? canonical->Lookup(value) : CreateHandle(isolate, value);
+}
+
+
 #ifdef DEBUG
 inline SealHandleScope::SealHandleScope(Isolate* isolate) : isolate_(isolate) {
   // Make sure the current thread is allowed to create handles to begin with.
@@ -112,10 +126,10 @@ inline SealHandleScope::SealHandleScope(Isolate* isolate) : isolate_(isolate) {
   HandleScopeData* current = isolate_->handle_scope_data();
   // Shrink the current handle scope to make it impossible to do
   // handle allocations without an explicit handle scope.
-  limit_ = current->limit;
+  prev_limit_ = current->limit;
   current->limit = current->next;
-  level_ = current->level;
-  current->level = 0;
+  prev_sealed_level_ = current->sealed_level;
+  current->sealed_level = current->level;
 }
 
 
@@ -123,10 +137,10 @@ inline SealHandleScope::~SealHandleScope() {
   // Restore state in current handle scope to re-enable handle
   // allocations.
   HandleScopeData* current = isolate_->handle_scope_data();
-  DCHECK_EQ(0, current->level);
-  current->level = level_;
   DCHECK_EQ(current->next, current->limit);
-  current->limit = limit_;
+  current->limit = prev_limit_;
+  DCHECK_EQ(current->level, current->sealed_level);
+  current->sealed_level = prev_sealed_level_;
 }
 
 #endif

src/handles.cc

@@ -4,7 +4,9 @@
 
 #include "src/handles.h"
 
+#include "src/address-map.h"
 #include "src/base/logging.h"
+#include "src/identity-map.h"
 #include "src/objects-inl.h"
 
 namespace v8 {
@@ -55,7 +57,7 @@ Object** HandleScope::Extend(Isolate* isolate) {
   DCHECK(result == current->limit);
   // Make sure there's at least one scope on the stack and that the
   // top of the scope stack isn't a barrier.
-  if (!Utils::ApiCheck(current->level != 0,
+  if (!Utils::ApiCheck(current->level != current->sealed_level,
                        "v8::HandleScope::CreateHandle()",
                        "Cannot create a handle without a HandleScope")) {
     return NULL;
@@ -117,6 +119,48 @@ Address HandleScope::current_limit_address(Isolate* isolate) {
 }
 
 
+CanonicalHandleScope::CanonicalHandleScope(Isolate* isolate)
+    : isolate_(isolate) {
+  HandleScopeData* handle_scope_data = isolate_->handle_scope_data();
+  prev_canonical_scope_ = handle_scope_data->canonical_scope;
+  handle_scope_data->canonical_scope = this;
+  root_index_map_ = new RootIndexMap(isolate);
+  identity_map_ = new IdentityMap<Object**>(isolate->heap(), &zone_);
+  canonical_level_ = handle_scope_data->level;
+}
+
+
+CanonicalHandleScope::~CanonicalHandleScope() {
+  delete root_index_map_;
+  delete identity_map_;
+  isolate_->handle_scope_data()->canonical_scope = prev_canonical_scope_;
+}
+
+
+Object** CanonicalHandleScope::Lookup(Object* object) {
+  DCHECK_LE(canonical_level_, isolate_->handle_scope_data()->level);
+  if (isolate_->handle_scope_data()->level != canonical_level_) {
+    // We are in an inner handle scope. Do not canonicalize since we will leave
+    // this handle scope while still being in the canonical scope.
+    return HandleScope::CreateHandle(isolate_, object);
+  }
+  if (object->IsHeapObject()) {
+    int index = root_index_map_->Lookup(HeapObject::cast(object));
+    if (index != RootIndexMap::kInvalidRootIndex) {
+      return isolate_->heap()
+          ->root_handle(static_cast<Heap::RootListIndex>(index))
+          .location();
+    }
+  }
+  Object*** entry = identity_map_->Get(object);
+  if (*entry == nullptr) {
+    // Allocate new handle location.
+    *entry = HandleScope::CreateHandle(isolate_, object);
+  }
+  return reinterpret_cast<Object**>(*entry);
+}
+
+
 DeferredHandleScope::DeferredHandleScope(Isolate* isolate)
     : impl_(isolate->handle_scope_implementer()) {
   impl_->BeginDeferredScope();

src/handles.h

@@ -10,6 +10,7 @@
 #include "src/base/macros.h"
 #include "src/checks.h"
 #include "src/globals.h"
+#include "src/zone.h"
 
 namespace v8 {
 namespace internal {
@@ -92,6 +93,9 @@ class Handle final : public HandleBase {
   V8_INLINE explicit Handle(T* object) : Handle(object, object->GetIsolate()) {}
   V8_INLINE Handle(T* object, Isolate* isolate) : HandleBase(object, isolate) {}
 
+  // Allocate a new handle for the object, do not canonicalize.
+  V8_INLINE static Handle<T> New(T* object, Isolate* isolate);
+
   // Constructor for handling automatic up casting.
   // Ex. Handle<JSFunction> can be passed when Handle<Object> is expected.
   template <typename S>
@@ -254,9 +258,11 @@ class HandleScope {
   // Counts the number of allocated handles.
   static int NumberOfHandles(Isolate* isolate);
 
+  // Create a new handle or lookup a canonical handle.
+  static inline Object** GetHandle(Isolate* isolate, Object* value);
+
   // Creates a new handle with the given value.
-  template <typename T>
-  static inline T** CreateHandle(Isolate* isolate, T* value);
+  static inline Object** CreateHandle(Isolate* isolate, Object* value);
 
   // Deallocates any extensions used by the current scope.
   static void DeleteExtensions(Isolate* isolate);
@@ -305,11 +311,44 @@ class HandleScope {
 
   friend class v8::HandleScope;
   friend class DeferredHandles;
+  friend class DeferredHandleScope;
   friend class HandleScopeImplementer;
   friend class Isolate;
 };
 
 
+// Forward declarations for CanonicalHandleScope.
+template <typename V>
+class IdentityMap;
+class RootIndexMap;
+
+
+// A CanonicalHandleScope does not open a new HandleScope. It changes the
+// existing HandleScope so that Handles created within are canonicalized.
+// This does not apply to nested inner HandleScopes unless a nested
+// CanonicalHandleScope is introduced. Handles are only canonicalized within
+// the same CanonicalHandleScope, but not across nested ones.
+class CanonicalHandleScope final {
+ public:
+  explicit CanonicalHandleScope(Isolate* isolate);
+  ~CanonicalHandleScope();
+
+ private:
+  Object** Lookup(Object* object);
+
+  Isolate* isolate_;
+  Zone zone_;
+  RootIndexMap* root_index_map_;
+  IdentityMap<Object**>* identity_map_;
+  // Ordinary nested handle scopes within the current one are not canonical.
+  int canonical_level_;
+  // We may have nested canonical scopes. Handles are canonical within each one.
+  CanonicalHandleScope* prev_canonical_scope_;
+
+  friend class HandleScope;
+};
+
+
 class DeferredHandleScope final {
  public:
   explicit DeferredHandleScope(Isolate* isolate);
@@ -345,8 +384,8 @@ class SealHandleScope final {
   inline ~SealHandleScope();
  private:
   Isolate* isolate_;
-  Object** limit_;
-  int level_;
+  Object** prev_limit_;
+  int prev_sealed_level_;
 #endif
 };
 
@@ -355,10 +394,13 @@ struct HandleScopeData final {
   Object** next;
   Object** limit;
   int level;
+  int sealed_level;
+  CanonicalHandleScope* canonical_scope;
 
   void Initialize() {
     next = limit = NULL;
-    level = 0;
+    sealed_level = level = 0;
+    canonical_scope = NULL;
   }
 };
 

src/ia32/macro-assembler-ia32.cc

@@ -24,9 +24,8 @@ MacroAssembler::MacroAssembler(Isolate* arg_isolate, void* buffer, int size)
       generating_stub_(false),
       has_frame_(false) {
   if (isolate() != NULL) {
-    // TODO(titzer): should we just use a null handle here instead?
-    code_object_ = Handle<Object>(isolate()->heap()->undefined_value(),
-                                  isolate());
+    code_object_ =
+        Handle<Object>::New(isolate()->heap()->undefined_value(), isolate());
   }
 }
 

src/identity-map.cc

@@ -151,7 +151,7 @@ void IdentityMapBase::Rehash() {
   for (auto pair : reinsert) {
     int index = InsertIndex(pair.first);
     DCHECK_GE(index, 0);
-    DCHECK_NULL(values_[index]);
+    DCHECK_NE(heap_->not_mapped_symbol(), values_[index]);
     values_[index] = pair.second;
   }
 }

src/json-stringifier.h

@@ -222,6 +222,7 @@ MaybeHandle<Object> BasicJsonStringifier::StringifyString(
     SerializeStringUnchecked_(object->GetFlatContent().ToOneByteVector(),
                               &no_extend);
     no_extend.Append('\"');
+    return no_extend.Finalize();
   } else {
     result = isolate->factory()
                  ->NewRawTwoByteString(worst_case_length)
@@ -232,8 +233,8 @@ MaybeHandle<Object> BasicJsonStringifier::StringifyString(
     SerializeStringUnchecked_(object->GetFlatContent().ToUC16Vector(),
                               &no_extend);
     no_extend.Append('\"');
+    return no_extend.Finalize();
   }
-  return result;
 }
 
 

src/mips/macro-assembler-mips.cc

@@ -25,8 +25,8 @@ MacroAssembler::MacroAssembler(Isolate* arg_isolate, void* buffer, int size)
       has_frame_(false),
       has_double_zero_reg_set_(false) {
   if (isolate() != NULL) {
-    code_object_ = Handle<Object>(isolate()->heap()->undefined_value(),
-                                  isolate());
+    code_object_ =
+        Handle<Object>::New(isolate()->heap()->undefined_value(), isolate());
   }
 }
 

src/mips64/macro-assembler-mips64.cc

@@ -23,8 +23,8 @@ MacroAssembler::MacroAssembler(Isolate* arg_isolate, void* buffer, int size)
       has_frame_(false),
       has_double_zero_reg_set_(false) {
   if (isolate() != NULL) {
-    code_object_ = Handle<Object>(isolate()->heap()->undefined_value(),
-                                  isolate());
+    code_object_ =
+        Handle<Object>::New(isolate()->heap()->undefined_value(), isolate());
   }
 }
 

src/ppc/macro-assembler-ppc.cc

@@ -26,7 +26,7 @@ MacroAssembler::MacroAssembler(Isolate* arg_isolate, void* buffer, int size)
       has_frame_(false) {
   if (isolate() != NULL) {
     code_object_ =
-        Handle<Object>(isolate()->heap()->undefined_value(), isolate());
+        Handle<Object>::New(isolate()->heap()->undefined_value(), isolate());
   }
 }
 

src/string-builder.cc

@@ -52,7 +52,7 @@ IncrementalStringBuilder::IncrementalStringBuilder(Isolate* isolate)
       part_length_(kInitialPartLength),
       current_index_(0) {
   // Create an accumulator handle starting with the empty string.
-  accumulator_ = Handle<String>(isolate->heap()->empty_string(), isolate);
+  accumulator_ = Handle<String>::New(isolate->heap()->empty_string(), isolate);
   current_part_ =
       factory()->NewRawOneByteString(part_length_).ToHandleChecked();
 }

src/string-builder.h

@@ -346,10 +346,12 @@ class IncrementalStringBuilder {
       DCHECK(string->length() >= required_length);
     }
 
-    ~NoExtendString() {
+    Handle<String> Finalize() {
       Handle<SeqString> string = Handle<SeqString>::cast(string_);
       int length = NoExtend<DestChar>::written();
-      *string_.location() = *SeqString::Truncate(string, length);
+      Handle<String> result = SeqString::Truncate(string, length);
+      string_ = Handle<String>();
+      return result;
     }
 
    private:

src/x64/macro-assembler-x64.cc

@@ -23,8 +23,8 @@ MacroAssembler::MacroAssembler(Isolate* arg_isolate, void* buffer, int size)
       has_frame_(false),
       root_array_available_(true) {
   if (isolate() != NULL) {
-    code_object_ = Handle<Object>(isolate()->heap()->undefined_value(),
-                                  isolate());
+    code_object_ =
+        Handle<Object>::New(isolate()->heap()->undefined_value(), isolate());
   }
 }
 

src/x87/macro-assembler-x87.cc

@@ -25,8 +25,8 @@ MacroAssembler::MacroAssembler(Isolate* arg_isolate, void* buffer, int size)
       has_frame_(false) {
   if (isolate() != NULL) {
     // TODO(titzer): should we just use a null handle here instead?
-    code_object_ = Handle<Object>(isolate()->heap()->undefined_value(),
-                                  isolate());
+    code_object_ =
+        Handle<Object>::New(isolate()->heap()->undefined_value(), isolate());
   }
 }
 

test/cctest/test-identity-map.cc

@@ -336,5 +336,85 @@ TEST(ExplicitGC) {
   }
 }
 
+
+TEST(CanonicalHandleScope) {
+  Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = CcTest::heap();
+  HandleScope outer(isolate);
+  CanonicalHandleScope outer_canonical(isolate);
+
+  // Deduplicate smi handles.
+  List<Handle<Object> > smi_handles;
+  for (int i = 0; i < 100; i++) {
+    smi_handles.Add(Handle<Object>(Smi::FromInt(i), isolate));
+  }
+  Object** next_handle = isolate->handle_scope_data()->next;
+  for (int i = 0; i < 100; i++) {
+    Handle<Object> new_smi = Handle<Object>(Smi::FromInt(i), isolate);
+    Handle<Object> old_smi = smi_handles[i];
+    CHECK_EQ(new_smi.location(), old_smi.location());
+  }
+  // Check that no new handles have been allocated.
+  CHECK_EQ(next_handle, isolate->handle_scope_data()->next);
+
+  // Deduplicate root list items.
+  Handle<String> empty_string(heap->empty_string());
+  Handle<Map> free_space_map(heap->free_space_map());
+  Handle<Symbol> uninitialized_symbol(heap->uninitialized_symbol());
+  CHECK_EQ(isolate->factory()->empty_string().location(),
+           empty_string.location());
+  CHECK_EQ(isolate->factory()->free_space_map().location(),
+           free_space_map.location());
+  CHECK_EQ(isolate->factory()->uninitialized_symbol().location(),
+           uninitialized_symbol.location());
+  // Check that no new handles have been allocated.
+  CHECK_EQ(next_handle, isolate->handle_scope_data()->next);
+
+  // Test ordinary heap objects.
+  Handle<HeapNumber> number1 = isolate->factory()->NewHeapNumber(3.3);
+  Handle<String> string1 =
+      isolate->factory()->NewStringFromAsciiChecked("test");
+  next_handle = isolate->handle_scope_data()->next;
+  Handle<HeapNumber> number2(*number1);
+  Handle<String> string2(*string1);
+  CHECK_EQ(number1.location(), number2.location());
+  CHECK_EQ(string1.location(), string2.location());
+  heap->CollectAllGarbage();
+  Handle<HeapNumber> number3(*number2);
+  Handle<String> string3(*string2);
+  CHECK_EQ(number1.location(), number3.location());
+  CHECK_EQ(string1.location(), string3.location());
+  // Check that no new handles have been allocated.
+  CHECK_EQ(next_handle, isolate->handle_scope_data()->next);
+
+  // Inner handle scope do not create canonical handles.
+  {
+    HandleScope inner(isolate);
+    Handle<HeapNumber> number4(*number1);
+    Handle<String> string4(*string1);
+    CHECK_NE(number1.location(), number4.location());
+    CHECK_NE(string1.location(), string4.location());
+
+    // Nested canonical scope does not conflict with outer canonical scope,
+    // but does not canonicalize across scopes.
+    CanonicalHandleScope inner_canonical(isolate);
+    Handle<HeapNumber> number5(*number4);
+    Handle<String> string5(*string4);
+    CHECK_NE(number4.location(), number5.location());
+    CHECK_NE(string4.location(), string5.location());
+    CHECK_NE(number1.location(), number5.location());
+    CHECK_NE(string1.location(), string5.location());
+
+    Handle<HeapNumber> number6(*number1);
+    Handle<String> string6(*string1);
+    CHECK_NE(number4.location(), number6.location());
+    CHECK_NE(string4.location(), string6.location());
+    CHECK_NE(number1.location(), number6.location());
+    CHECK_NE(string1.location(), string6.location());
+    CHECK_EQ(number5.location(), number6.location());
+    CHECK_EQ(string5.location(), string6.location());
+  }
+}
+
 }  // namespace internal
 }  // namespace v8