From 720d9c280a8e7415ed77ac846e955a8e163d834f Mon Sep 17 00:00:00 2001
From: yangguo <yangguo@chromium.org>
Date: Thu, 21 May 2015 03:35:51 -0700
Subject: [PATCH] Debugger: use weak cells to implement ScriptCache.

R=ulan@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#28539}
---
 src/debug.cc      | 104 +++++++++++++++++-------------------------------------
 src/debug.h       |  21 ++++-------
 src/objects-inl.h |   4 +++
 src/objects.cc    |  43 ++++++++++++++++++++++
 src/objects.h     |  27 ++++++++++++--
 5 files changed, 110 insertions(+), 89 deletions(-)

diff --git a/src/debug.cc b/src/debug.cc
index bcff35a..df8eab7 100644
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -493,8 +493,7 @@ int Debug::ArchiveSpacePerThread() {
 }
 
 
-ScriptCache::ScriptCache(Isolate* isolate) : HashMap(HashMap::PointersMatch),
-                                             isolate_(isolate) {
+ScriptCache::ScriptCache(Isolate* isolate) : isolate_(isolate) {
   Heap* heap = isolate_->heap();
   HandleScope scope(isolate_);
 
@@ -502,92 +501,53 @@ ScriptCache::ScriptCache(Isolate* isolate) : HashMap(HashMap::PointersMatch),
   heap->CollectAllGarbage(Heap::kMakeHeapIterableMask, "ScriptCache");
 
   // Scan heap for Script objects.
-  HeapIterator iterator(heap);
-  DisallowHeapAllocation no_allocation;
-
-  for (HeapObject* obj = iterator.next(); obj != NULL; obj = iterator.next()) {
-    if (obj->IsScript() && Script::cast(obj)->HasValidSource()) {
-      Add(Handle<Script>(Script::cast(obj)));
+  List<Handle<Script> > scripts;
+  {
+    HeapIterator iterator(heap, HeapIterator::kFilterUnreachable);
+    DisallowHeapAllocation no_allocation;
+    for (HeapObject* obj = iterator.next(); obj != NULL;
+         obj = iterator.next()) {
+      if (obj->IsScript() && Script::cast(obj)->HasValidSource()) {
+        scripts.Add(Handle<Script>(Script::cast(obj)));
+      }
     }
   }
+
+  GlobalHandles* global_handles = isolate_->global_handles();
+  table_ = Handle<WeakValueHashTable>::cast(global_handles->Create(
+      Object::cast(*WeakValueHashTable::New(isolate_, scripts.length()))));
+  for (int i = 0; i < scripts.length(); i++) Add(scripts[i]);
 }
 
 
 void ScriptCache::Add(Handle<Script> script) {
-  GlobalHandles* global_handles = isolate_->global_handles();
-  // Create an entry in the hash map for the script.
-  int id = script->id()->value();
-  HashMap::Entry* entry =
-      HashMap::LookupOrInsert(reinterpret_cast<void*>(id), Hash(id));
-  if (entry->value != NULL) {
+  HandleScope scope(isolate_);
+  Handle<Smi> id(script->id(), isolate_);
+
 #ifdef DEBUG
-    // The code deserializer may introduce duplicate Script objects.
-    // Assert that the Script objects with the same id have the same name.
-    Handle<Script> found(reinterpret_cast<Script**>(entry->value));
+  Handle<Object> lookup(table_->LookupWeak(id), isolate_);
+  if (!lookup->IsTheHole()) {
+    Handle<Script> found = Handle<Script>::cast(lookup);
     DCHECK(script->id() == found->id());
     DCHECK(!script->name()->IsString() ||
            String::cast(script->name())->Equals(String::cast(found->name())));
-#endif
-    return;
   }
-  // Globalize the script object, make it weak and use the location of the
-  // global handle as the value in the hash map.
-  Handle<Script> script_ =
-      Handle<Script>::cast(global_handles->Create(*script));
-  GlobalHandles::MakeWeak(reinterpret_cast<Object**>(script_.location()),
-                          this,
-                          ScriptCache::HandleWeakScript);
-  entry->value = script_.location();
-}
-
-
-Handle<FixedArray> ScriptCache::GetScripts() {
-  Factory* factory = isolate_->factory();
-  Handle<FixedArray> instances = factory->NewFixedArray(occupancy());
-  int count = 0;
-  for (HashMap::Entry* entry = Start(); entry != NULL; entry = Next(entry)) {
-    DCHECK(entry->value != NULL);
-    if (entry->value != NULL) {
-      instances->set(count, *reinterpret_cast<Script**>(entry->value));
-      count++;
-    }
-  }
-  return instances;
-}
+#endif
 
+  Handle<WeakValueHashTable> new_table =
+      WeakValueHashTable::PutWeak(table_, id, script);
 
-void ScriptCache::Clear() {
-  // Iterate the script cache to get rid of all the weak handles.
-  for (HashMap::Entry* entry = Start(); entry != NULL; entry = Next(entry)) {
-    DCHECK(entry != NULL);
-    Object** location = reinterpret_cast<Object**>(entry->value);
-    DCHECK((*location)->IsScript());
-    GlobalHandles::ClearWeakness(location);
-    GlobalHandles::Destroy(location);
-  }
-  // Clear the content of the hash map.
-  HashMap::Clear();
+  if (new_table.is_identical_to(table_)) return;
+  GlobalHandles* global_handles = isolate_->global_handles();
+  global_handles->Destroy(Handle<Object>::cast(table_).location());
+  table_ = Handle<WeakValueHashTable>::cast(
+      global_handles->Create(Object::cast(*new_table)));
 }
 
 
-void ScriptCache::HandleWeakScript(
-    const v8::WeakCallbackData<v8::Value, void>& data) {
-  // Retrieve the script identifier.
-  Handle<Object> object = Utils::OpenHandle(*data.GetValue());
-  int id = Handle<Script>::cast(object)->id()->value();
-  void* key = reinterpret_cast<void*>(id);
-  uint32_t hash = Hash(id);
-
-  // Remove the corresponding entry from the cache.
-  ScriptCache* script_cache =
-      reinterpret_cast<ScriptCache*>(data.GetParameter());
-  HashMap::Entry* entry = script_cache->Lookup(key, hash);
-  DCHECK_NOT_NULL(entry);
-  Object** location = reinterpret_cast<Object**>(entry->value);
-  script_cache->Remove(key, hash);
-
-  // Clear the weak handle.
-  GlobalHandles::Destroy(location);
+ScriptCache::~ScriptCache() {
+  isolate_->global_handles()->Destroy(Handle<Object>::cast(table_).location());
+  table_ = Handle<WeakValueHashTable>();
 }
 
 
diff --git a/src/debug.h b/src/debug.h
index 9a9a3ba..fcea2bd 100644
--- a/src/debug.h
+++ b/src/debug.h
@@ -228,31 +228,22 @@ class BreakLocation {
 // to it is created and that weak handle is stored in the cache. The weak handle
 // callback takes care of removing the script from the cache. The key used in
 // the cache is the script id.
-class ScriptCache : private HashMap {
+class ScriptCache {
  public:
   explicit ScriptCache(Isolate* isolate);
-  virtual ~ScriptCache() { Clear(); }
+  ~ScriptCache();
 
   // Add script to the cache.
   void Add(Handle<Script> script);
 
   // Return the scripts in the cache.
-  Handle<FixedArray> GetScripts();
-
- private:
-  // Calculate the hash value from the key (script id).
-  static uint32_t Hash(int key) {
-    return ComputeIntegerHash(key, v8::internal::kZeroHashSeed);
+  Handle<FixedArray> GetScripts() {
+    return WeakValueHashTable::GetWeakValues(table_);
   }
 
-  // Clear the cache releasing all the weak handles.
-  void Clear();
-
-  // Weak handle callback for scripts in the cache.
-  static void HandleWeakScript(
-      const v8::WeakCallbackData<v8::Value, void>& data);
-
+ private:
   Isolate* isolate_;
+  Handle<WeakValueHashTable> table_;
 };
 
 
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 50d02f9..c086fa3 100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -891,6 +891,9 @@ bool Object::IsWeakHashTable() const {
 }
 
 
+bool Object::IsWeakValueHashTable() const { return IsHashTable(); }
+
+
 bool Object::IsDictionary() const {
   return IsHashTable() &&
       this != HeapObject::cast(this)->GetHeap()->string_table();
@@ -3436,6 +3439,7 @@ CAST_ACCESSOR(UnseededNumberDictionary)
 CAST_ACCESSOR(WeakCell)
 CAST_ACCESSOR(WeakFixedArray)
 CAST_ACCESSOR(WeakHashTable)
+CAST_ACCESSOR(WeakValueHashTable)
 
 
 // static
diff --git a/src/objects.cc b/src/objects.cc
index ecb5bf1..f919c9d 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -16411,6 +16411,49 @@ void WeakHashTable::AddEntry(int entry, Handle<WeakCell> key_cell,
 }
 
 
+#ifdef DEBUG
+Object* WeakValueHashTable::LookupWeak(Handle<Object> key) {
+  Object* value = Lookup(key);
+  if (value->IsWeakCell() && !WeakCell::cast(value)->cleared()) {
+    value = WeakCell::cast(value)->value();
+  }
+  return value;
+}
+#endif  // DEBUG
+
+
+Handle<WeakValueHashTable> WeakValueHashTable::PutWeak(
+    Handle<WeakValueHashTable> table, Handle<Object> key,
+    Handle<HeapObject> value) {
+  Handle<WeakCell> cell = value->GetIsolate()->factory()->NewWeakCell(value);
+  return Handle<WeakValueHashTable>::cast(
+      Put(Handle<ObjectHashTable>::cast(table), key, cell));
+}
+
+
+Handle<FixedArray> WeakValueHashTable::GetWeakValues(
+    Handle<WeakValueHashTable> table) {
+  Isolate* isolate = table->GetIsolate();
+  uint32_t capacity = table->Capacity();
+  Handle<FixedArray> results = isolate->factory()->NewFixedArray(capacity);
+  int length = 0;
+  for (uint32_t i = 0; i < capacity; i++) {
+    uint32_t key_index = table->EntryToIndex(i);
+    Object* key = table->get(key_index);
+    if (!table->IsKey(key)) continue;
+    uint32_t value_index = table->EntryToValueIndex(i);
+    WeakCell* value_cell = WeakCell::cast(table->get(value_index));
+    if (value_cell->cleared()) {
+      table->RemoveEntry(i);
+    } else {
+      results->set(length++, value_cell->value());
+    }
+  }
+  results->Shrink(length);
+  return results;
+}
+
+
 template<class Derived, class Iterator, int entrysize>
 Handle<Derived> OrderedHashTable<Derived, Iterator, entrysize>::Allocate(
     Isolate* isolate, int capacity, PretenureFlag pretenure) {
diff --git a/src/objects.h b/src/objects.h
index 4b7341e..94c04c6 100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -1012,6 +1012,7 @@ template <class C> inline bool Is(Object* obj);
   V(WeakCell)                      \
   V(ObjectHashTable)               \
   V(WeakHashTable)                 \
+  V(WeakValueHashTable)            \
   V(OrderedHashTable)
 
 // Object is the abstract superclass for all classes in the
@@ -3958,7 +3959,7 @@ class ObjectHashTable: public HashTable<ObjectHashTable,
                                         Handle<Object> key,
                                         bool* was_present);
 
- private:
+ protected:
   friend class MarkCompactCollector;
 
   void AddEntry(int entry, Object* key, Object* value);
@@ -4185,6 +4186,8 @@ class WeakHashTable: public HashTable<WeakHashTable,
                                                    Handle<HeapObject> key,
                                                    Handle<HeapObject> value);
 
+  static Handle<FixedArray> GetValues(Handle<WeakHashTable> table);
+
  private:
   friend class MarkCompactCollector;
 
@@ -4197,6 +4200,26 @@ class WeakHashTable: public HashTable<WeakHashTable,
 };
 
 
+class WeakValueHashTable : public ObjectHashTable {
+ public:
+  DECLARE_CAST(WeakValueHashTable)
+
+#ifdef DEBUG
+  // Looks up the value associated with the given key. The hole value is
+  // returned in case the key is not present.
+  Object* LookupWeak(Handle<Object> key);
+#endif  // DEBUG
+
+  // Adds (or overwrites) the value associated with the given key. Mapping a
+  // key to the hole value causes removal of the whole entry.
+  MUST_USE_RESULT static Handle<WeakValueHashTable> PutWeak(
+      Handle<WeakValueHashTable> table, Handle<Object> key,
+      Handle<HeapObject> value);
+
+  static Handle<FixedArray> GetWeakValues(Handle<WeakValueHashTable> table);
+};
+
+
 // JSFunctionResultCache caches results of some JSFunction invocation.
 // It is a fixed array with fixed structure:
 //   [0]: factory function
@@ -4204,7 +4227,7 @@ class WeakHashTable: public HashTable<WeakHashTable,
 //   [2]: current cache size
 //   [3]: dummy field.
 // The rest of array are key/value pairs.
-class JSFunctionResultCache: public FixedArray {
+class JSFunctionResultCache : public FixedArray {
  public:
   static const int kFactoryIndex = 0;
   static const int kFingerIndex = kFactoryIndex + 1;
-- 
2.7.4