From 3727a10d2043846ec2b2e03a23fae0d804a24fde Mon Sep 17 00:00:00 2001
From: "erikcorry@chromium.org" <erikcorry@chromium.org>
Date: Wed, 15 Oct 2014 10:11:08 +0000
Subject: [PATCH] Use WeakCell to handle the script wrapper cache

The script wrapper cache used the API weak handles to provide a weak link from Script to ScriptWrapper. We want to change the way API weakness works, and in this context it's best to get rid of users of the API that don't need to be users.

R=ulan@chromium.org
BUG=

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24622 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/factory.cc    |  3 +--
 src/objects-inl.h |  2 +-
 src/objects.cc    | 53 ++++++++++++-----------------------------------
 src/objects.h     |  6 +++---
 src/serialize.cc  |  7 -------
 5 files changed, 18 insertions(+), 53 deletions(-)

diff --git a/src/factory.cc b/src/factory.cc
index 5b196d3ca0..d1deb7519a 100644
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -837,7 +837,6 @@ Handle<Script> Factory::NewScript(Handle<String> source) {
   heap->set_last_script_id(Smi::FromInt(id));
 
   // Create and initialize script object.
-  Handle<Foreign> wrapper = NewForeign(0, TENURED);
   Handle<Script> script = Handle<Script>::cast(NewStruct(SCRIPT_TYPE));
   script->set_source(*source);
   script->set_name(heap->undefined_value());
@@ -846,7 +845,7 @@ Handle<Script> Factory::NewScript(Handle<String> source) {
   script->set_column_offset(Smi::FromInt(0));
   script->set_context_data(heap->undefined_value());
   script->set_type(Smi::FromInt(Script::TYPE_NORMAL));
-  script->set_wrapper(*wrapper);
+  script->set_wrapper(heap->undefined_value());
   script->set_line_ends(heap->undefined_value());
   script->set_eval_from_shared(heap->undefined_value());
   script->set_eval_from_instructions_offset(Smi::FromInt(0));
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 6385c587c5..91bd009224 100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -5407,7 +5407,7 @@ ACCESSORS(Script, id, Smi, kIdOffset)
 ACCESSORS_TO_SMI(Script, line_offset, kLineOffsetOffset)
 ACCESSORS_TO_SMI(Script, column_offset, kColumnOffsetOffset)
 ACCESSORS(Script, context_data, Object, kContextOffset)
-ACCESSORS(Script, wrapper, Foreign, kWrapperOffset)
+ACCESSORS(Script, wrapper, HeapObject, kWrapperOffset)
 ACCESSORS_TO_SMI(Script, type, kTypeOffset)
 ACCESSORS(Script, line_ends, Object, kLineEndsOffset)
 ACCESSORS(Script, eval_from_shared, Object, kEvalFromSharedOffset)
diff --git a/src/objects.cc b/src/objects.cc
index 4f23ea58b9..300b5b7ddd 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -9727,54 +9727,27 @@ Handle<Object> Script::GetNameOrSourceURL(Handle<Script> script) {
 }
 
 
-// Wrappers for scripts are kept alive and cached in weak global
-// handles referred from foreign objects held by the scripts as long as
-// they are used. When they are not used anymore, the garbage
-// collector will call the weak callback on the global handle
-// associated with the wrapper and get rid of both the wrapper and the
-// handle.
-static void ClearWrapperCacheWeakCallback(
-    const v8::WeakCallbackData<v8::Value, void>& data) {
-  Object** location = reinterpret_cast<Object**>(data.GetParameter());
-  JSValue* wrapper = JSValue::cast(*location);
-  Script::cast(wrapper->value())->ClearWrapperCache();
-}
-
-
-void Script::ClearWrapperCache() {
-  Foreign* foreign = wrapper();
-  Object** location = reinterpret_cast<Object**>(foreign->foreign_address());
-  DCHECK_EQ(foreign->foreign_address(), reinterpret_cast<Address>(location));
-  foreign->set_foreign_address(0);
-  GlobalHandles::Destroy(location);
-  GetIsolate()->counters()->script_wrappers()->Decrement();
-}
-
-
 Handle<JSObject> Script::GetWrapper(Handle<Script> script) {
-  if (script->wrapper()->foreign_address() != NULL) {
-    // Return a handle for the existing script wrapper from the cache.
-    return Handle<JSValue>(
-        *reinterpret_cast<JSValue**>(script->wrapper()->foreign_address()));
-  }
   Isolate* isolate = script->GetIsolate();
+  if (!script->wrapper()->IsUndefined()) {
+    Handle<WeakCell> cell(WeakCell::cast(script->wrapper()));
+    if (!cell->value()->IsUndefined()) {
+      // Return a handle for the existing script wrapper from the cache.
+      return handle(JSObject::cast(cell->value()));
+    }
+    // If we found an empty WeakCell, that means the script wrapper was
+    // GCed.  We are not notified directly of that, so we decrement here
+    // so that we at least don't count double for any given script.
+    isolate->counters()->script_wrappers()->Decrement();
+  }
   // Construct a new script wrapper.
   isolate->counters()->script_wrappers()->Increment();
   Handle<JSFunction> constructor = isolate->script_function();
   Handle<JSValue> result =
       Handle<JSValue>::cast(isolate->factory()->NewJSObject(constructor));
-
   result->set_value(*script);
-
-  // Create a new weak global handle and use it to cache the wrapper
-  // for future use. The cache will automatically be cleared by the
-  // garbage collector when it is not used anymore.
-  Handle<Object> handle = isolate->global_handles()->Create(*result);
-  GlobalHandles::MakeWeak(handle.location(),
-                          reinterpret_cast<void*>(handle.location()),
-                          &ClearWrapperCacheWeakCallback);
-  script->wrapper()->set_foreign_address(
-      reinterpret_cast<Address>(handle.location()));
+  Handle<WeakCell> cell = isolate->factory()->NewWeakCell(result);
+  script->set_wrapper(*cell);
   return result;
 }
 
diff --git a/src/objects.h b/src/objects.h
index 69375570ad..5a1830408e 100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -860,6 +860,7 @@ class ObjectVisitor;
 class LookupIterator;
 class StringStream;
 class TypeFeedbackVector;
+class WeakCell;
 // We cannot just say "class HeapType;" if it is created from a template... =8-?
 template<class> class TypeImpl;
 struct HeapTypeConfig;
@@ -6445,8 +6446,8 @@ class Script: public Struct {
   // [context_data]: context data for the context this script was compiled in.
   DECL_ACCESSORS(context_data, Object)
 
-  // [wrapper]: the wrapper cache.
-  DECL_ACCESSORS(wrapper, Foreign)
+  // [wrapper]: the wrapper cache.  This is either undefined or a WeakCell.
+  DECL_ACCESSORS(wrapper, HeapObject)
 
   // [type]: the script type.
   DECL_ACCESSORS(type, Smi)
@@ -6508,7 +6509,6 @@ class Script: public Struct {
 
   // Get the JS object wrapping the given script; create it if none exists.
   static Handle<JSObject> GetWrapper(Handle<Script> script);
-  void ClearWrapperCache();
 
   // Dispatched behavior.
   DECLARE_PRINTER(Script)
diff --git a/src/serialize.cc b/src/serialize.cc
index 70b69c23c0..42ca6b9b22 100644
--- a/src/serialize.cc
+++ b/src/serialize.cc
@@ -2017,13 +2017,6 @@ void CodeSerializer::SerializeObject(Object* o, HowToCode how_to_code,
 void CodeSerializer::SerializeHeapObject(HeapObject* heap_object,
                                          HowToCode how_to_code,
                                          WhereToPoint where_to_point) {
-  if (heap_object->IsScript()) {
-    // The wrapper cache uses a Foreign object to point to a global handle.
-    // However, the object visitor expects foreign objects to point to external
-    // references.  Clear the cache to avoid this issue.
-    Script::cast(heap_object)->ClearWrapperCache();
-  }
-
   if (FLAG_trace_code_serializer) {
     PrintF("Encoding heap object: ");
     heap_object->ShortPrint();