path: root/src/v8/0003-Generalize-external-object-resources.patch

From 74974cee335e6c22ea99fd9a4bbb9c7fa7323d80 Mon Sep 17 00:00:00 2001
From: Aaron Kennedy <aaron.kennedy@nokia.com>
Date: Tue, 4 Oct 2011 16:06:09 +1000
Subject: [PATCH 03/12] Generalize external object resources

V8 was already able to manage and finalize an external string
resource.  This change generalizes that mechanism to handle a
single generic external resource - a v8::Object::ExternalResource
derived instance - on normal JSObject's.

This is useful for mapping C++ objects to JS objects where the
C++ object's memory is effectively owned by the JS Object, and
thus needs to destroyed when the JS Object is garbage collected.
The V8 mailing list suggests using a weak persistent handle for
this purpose, but that seems to incur a fairly massive performance
penalty for short lived objects as weak persistent handle callbacks
are not called until the object has been promoted into the old
object space.
---
 include/v8.h        |   25 ++++++++++++++++++++
 src/api.cc          |   56 +++++++++++++++++++++++++++++++++++++++++++++
 src/factory.cc      |   11 +++++++++
 src/heap-inl.h      |   63 +++++++++++++++++++++++++++++++++++---------------
 src/heap.cc         |   29 +++++++++++++++++------
 src/heap.h          |   16 ++++++++-----
 src/mark-compact.cc |   13 +++++-----
 src/objects-inl.h   |   35 +++++++++++++++++++++++++++-
 src/objects.h       |   19 ++++++++++++---
 9 files changed, 223 insertions(+), 44 deletions(-)

diff --git a/include/v8.h b/include/v8.h
index d2e6c32..3ef4dd6 100644
--- a/include/v8.h
+++ b/include/v8.h
@@ -1597,6 +1597,25 @@ class Object : public Value {
   /** Sets a native pointer in an internal field. */
   V8EXPORT void SetPointerInInternalField(int index, void* value);
 
+  class V8EXPORT ExternalResource { // NOLINT
+   public:
+    ExternalResource() {}
+    virtual ~ExternalResource() {}
+
+   protected:
+    virtual void Dispose() { delete this; }
+
+   private:
+    // Disallow copying and assigning.
+    ExternalResource(const ExternalResource&);
+    void operator=(const ExternalResource&);
+
+    friend class v8::internal::Heap;
+  };
+
+  V8EXPORT void SetExternalResource(ExternalResource *);
+  V8EXPORT ExternalResource *GetExternalResource();
+
   // Testers for local properties.
   V8EXPORT bool HasOwnProperty(Handle<String> key);
   V8EXPORT bool HasRealNamedProperty(Handle<String> key);
@@ -2466,6 +2485,12 @@ class V8EXPORT ObjectTemplate : public Template {
    */
   void SetInternalFieldCount(int value);
 
+  /**
+   * Sets whether the object can store an "external resource" object.
+   */
+  bool HasExternalResource();
+  void SetHasExternalResource(bool value);
+
  private:
   ObjectTemplate();
   static Local<ObjectTemplate> New(Handle<FunctionTemplate> constructor);
diff --git a/src/api.cc b/src/api.cc
index e0f3b5a..7d54252 100644
--- a/src/api.cc
+++ b/src/api.cc
@@ -1436,6 +1436,34 @@ void ObjectTemplate::SetInternalFieldCount(int value) {
 }
 
 
+bool ObjectTemplate::HasExternalResource()
+{
+  if (IsDeadCheck(Utils::OpenHandle(this)->GetIsolate(),
+                  "v8::ObjectTemplate::HasExternalResource()")) {
+    return 0;
+  }
+  return !Utils::OpenHandle(this)->has_external_resource()->IsUndefined();
+}
+
+
+void ObjectTemplate::SetHasExternalResource(bool value)
+{
+  i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
+  if (IsDeadCheck(isolate, "v8::ObjectTemplate::SetHasExternalResource()")) {
+    return;
+  }
+  ENTER_V8(isolate);
+  if (value) {
+    EnsureConstructor(this);
+  }
+  if (value) {
+      Utils::OpenHandle(this)->set_has_external_resource(i::Smi::FromInt(1));
+  } else {
+      Utils::OpenHandle(this)->set_has_external_resource(Utils::OpenHandle(this)->GetHeap()->undefined_value());
+  }
+}
+
+
 // --- S c r i p t D a t a ---
 
 
@@ -4029,6 +4057,34 @@ void v8::Object::SetPointerInInternalField(int index, void* value) {
 }
 
 
+void v8::Object::SetExternalResource(v8::Object::ExternalResource *resource) {
+  i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
+  ENTER_V8(isolate);
+  i::Handle<i::JSObject> obj = Utils::OpenHandle(this);
+  if (CanBeEncodedAsSmi(resource)) {
+    obj->SetExternalResourceObject(EncodeAsSmi(resource));
+  } else {
+    obj->SetExternalResourceObject(*isolate->factory()->NewForeign(static_cast<i::Address>((void *)resource)));
+  }
+  if (!obj->IsSymbol()) {
+    isolate->heap()->external_string_table()->AddObject(*obj);
+  }
+}
+
+
+v8::Object::ExternalResource *v8::Object::GetExternalResource() {
+  i::Handle<i::JSObject> obj = Utils::OpenHandle(this);
+  i::Object* value = obj->GetExternalResourceObject();
+  if (value->IsSmi()) {
+    return reinterpret_cast<v8::Object::ExternalResource*>(i::Internals::GetExternalPointerFromSmi(value));
+  } else if (value->IsForeign()) {
+    return reinterpret_cast<v8::Object::ExternalResource*>(i::Foreign::cast(value)->address());
+  } else {
+    return NULL;
+  }
+}
+
+
 // --- E n v i r o n m e n t ---
 
 
diff --git a/src/factory.cc b/src/factory.cc
index 1b95ed1..8c96944 100644
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -1152,15 +1152,21 @@ Handle<JSFunction> Factory::CreateApiFunction(
   Handle<Code> construct_stub = isolate()->builtins()->JSConstructStubApi();
 
   int internal_field_count = 0;
+  bool has_external_resource = false;
+
   if (!obj->instance_template()->IsUndefined()) {
     Handle<ObjectTemplateInfo> instance_template =
         Handle<ObjectTemplateInfo>(
             ObjectTemplateInfo::cast(obj->instance_template()));
     internal_field_count =
         Smi::cast(instance_template->internal_field_count())->value();
+    has_external_resource =
+        !instance_template->has_external_resource()->IsUndefined();
   }
 
   int instance_size = kPointerSize * internal_field_count;
+  if (has_external_resource) instance_size += kPointerSize;
+
   InstanceType type = INVALID_TYPE;
   switch (instance_type) {
     case JavaScriptObject:
@@ -1195,6 +1201,11 @@ Handle<JSFunction> Factory::CreateApiFunction(
 
   Handle<Map> map = Handle<Map>(result->initial_map());
 
+  // Mark as having external data object if needed
+  if (has_external_resource) {
+    map->set_has_external_resource(true);
+  }
+
   // Mark as undetectable if needed.
   if (obj->undetectable()) {
     map->set_is_undetectable();
diff --git a/src/heap-inl.h b/src/heap-inl.h
index 4c55d63..bca57cb 100644
--- a/src/heap-inl.h
+++ b/src/heap-inl.h
@@ -222,21 +222,36 @@ MaybeObject* Heap::NumberFromUint32(uint32_t value) {
 }
 
 
-void Heap::FinalizeExternalString(String* string) {
-  ASSERT(string->IsExternalString());
-  v8::String::ExternalStringResourceBase** resource_addr =
-      reinterpret_cast<v8::String::ExternalStringResourceBase**>(
-          reinterpret_cast<byte*>(string) +
-          ExternalString::kResourceOffset -
-          kHeapObjectTag);
-
-  // Dispose of the C++ object if it has not already been disposed.
-  if (*resource_addr != NULL) {
-    (*resource_addr)->Dispose();
+void Heap::FinalizeExternalString(HeapObject* string) {
+  ASSERT(string->IsExternalString() || string->map()->has_external_resource());
+
+  if (string->IsExternalString()) {
+    v8::String::ExternalStringResourceBase** resource_addr =
+        reinterpret_cast<v8::String::ExternalStringResourceBase**>(
+            reinterpret_cast<byte*>(string) +
+            ExternalString::kResourceOffset -
+            kHeapObjectTag);
+    
+    // Dispose of the C++ object if it has not already been disposed.
+    if (*resource_addr != NULL) {
+      (*resource_addr)->Dispose();
+    }
+    
+    // Clear the resource pointer in the string.
+    *resource_addr = NULL;
+  } else {
+    JSObject *object = JSObject::cast(string);
+    Object *value = object->GetExternalResourceObject();
+    v8::Object::ExternalResource *resource = 0;
+    if (value->IsSmi()) {
+        resource = reinterpret_cast<v8::Object::ExternalResource*>(Internals::GetExternalPointerFromSmi(value));
+    } else if (value->IsForeign()) {
+        resource = reinterpret_cast<v8::Object::ExternalResource*>(Foreign::cast(value)->address());
+    } 
+    if (resource) {
+        resource->Dispose();
+    }
   }
-
-  // Clear the resource pointer in the string.
-  *resource_addr = NULL;
 }
 
 
@@ -555,6 +570,16 @@ void ExternalStringTable::AddString(String* string) {
 }
 
 
+void ExternalStringTable::AddObject(HeapObject* object) {
+  ASSERT(object->map()->has_external_resource());
+  if (heap_->InNewSpace(object)) {
+    new_space_strings_.Add(object);
+  } else {
+    old_space_strings_.Add(object);
+  }
+}
+
+
 void ExternalStringTable::Iterate(ObjectVisitor* v) {
   if (!new_space_strings_.is_empty()) {
     Object** start = &new_space_strings_[0];
@@ -583,14 +608,14 @@ void ExternalStringTable::Verify() {
 }
 
 
-void ExternalStringTable::AddOldString(String* string) {
-  ASSERT(string->IsExternalString());
-  ASSERT(!heap_->InNewSpace(string));
-  old_space_strings_.Add(string);
+void ExternalStringTable::AddOldObject(HeapObject* object) {
+  ASSERT(object->IsExternalString() || object->map()->has_external_resource());
+  ASSERT(!heap_->InNewSpace(object));
+  old_space_strings_.Add(object);
 }
 
 
-void ExternalStringTable::ShrinkNewStrings(int position) {
+void ExternalStringTable::ShrinkNewObjects(int position) {
   new_space_strings_.Rewind(position);
   if (FLAG_verify_heap) {
     Verify();
diff --git a/src/heap.cc b/src/heap.cc
index d287ead..53a0f27 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -1099,18 +1099,18 @@ void Heap::Scavenge() {
 }
 
 
-String* Heap::UpdateNewSpaceReferenceInExternalStringTableEntry(Heap* heap,
-                                                                Object** p) {
+HeapObject* Heap::UpdateNewSpaceReferenceInExternalStringTableEntry(Heap* heap,
+                                                                    Object** p) {
   MapWord first_word = HeapObject::cast(*p)->map_word();
 
   if (!first_word.IsForwardingAddress()) {
     // Unreachable external string can be finalized.
-    heap->FinalizeExternalString(String::cast(*p));
+    heap->FinalizeExternalString(HeapObject::cast(*p));
     return NULL;
   }
 
   // String is still reachable.
-  return String::cast(first_word.ToForwardingAddress());
+  return HeapObject::cast(first_word.ToForwardingAddress());
 }
 
 
@@ -1128,11 +1128,11 @@ void Heap::UpdateNewSpaceReferencesInExternalStringTable(
 
   for (Object** p = start; p < end; ++p) {
     ASSERT(InFromSpace(*p));
-    String* target = updater_func(this, p);
+    HeapObject* target = updater_func(this, p);
 
     if (target == NULL) continue;
 
-    ASSERT(target->IsExternalString());
+    ASSERT(target->IsExternalString() || target->map()->has_external_resource());
 
     if (InNewSpace(target)) {
       // String is still in new space.  Update the table entry.
@@ -1140,12 +1140,12 @@ void Heap::UpdateNewSpaceReferencesInExternalStringTable(
       ++last;
     } else {
       // String got promoted.  Move it to the old string list.
-      external_string_table_.AddOldString(target);
+      external_string_table_.AddOldObject(target);
     }
   }
 
   ASSERT(last <= end);
-  external_string_table_.ShrinkNewStrings(static_cast<int>(last - start));
+  external_string_table_.ShrinkNewObjects(static_cast<int>(last - start));
 }
 
 
@@ -6367,6 +6367,19 @@ void ExternalStringTable::CleanUp() {
 
 
 void ExternalStringTable::TearDown() {
+  for (int i = 0; i < new_space_strings_.length(); ++i) { 
+    if (new_space_strings_[i] == heap_->raw_unchecked_null_value()) continue;
+    HeapObject *object = HeapObject::cast(new_space_strings_[i]);
+    if (!object->IsExternalString())
+        heap_->FinalizeExternalString(object);
+  }
+  for (int i = 0; i < old_space_strings_.length(); ++i) { 
+    if (old_space_strings_[i] == heap_->raw_unchecked_null_value()) continue;
+    HeapObject *object = HeapObject::cast(old_space_strings_[i]);
+    if (!object->IsExternalString())
+        heap_->FinalizeExternalString(object);
+  }
+
   new_space_strings_.Free();
   old_space_strings_.Free();
 }
diff --git a/src/heap.h b/src/heap.h
index 7c0b0ea..5e90964 100644
--- a/src/heap.h
+++ b/src/heap.h
@@ -245,8 +245,8 @@ class Isolate;
 class WeakObjectRetainer;
 
 
-typedef String* (*ExternalStringTableUpdaterCallback)(Heap* heap,
-                                                      Object** pointer);
+typedef HeapObject* (*ExternalStringTableUpdaterCallback)(Heap* heap,
+                                                          Object** pointer);
 
 class StoreBufferRebuilder {
  public:
@@ -331,10 +331,14 @@ typedef void (*ScavengingCallback)(Map* map,
 // External strings table is a place where all external strings are
 // registered.  We need to keep track of such strings to properly
 // finalize them.
+// The ExternalStringTable can contain both strings and objects with
+// external resources.  It was not renamed to make the patch simpler.
 class ExternalStringTable {
  public:
   // Registers an external string.
   inline void AddString(String* string);
+  // Registers an external object.
+  inline void AddObject(HeapObject* string);
 
   inline void Iterate(ObjectVisitor* v);
 
@@ -352,10 +356,10 @@ class ExternalStringTable {
 
   inline void Verify();
 
-  inline void AddOldString(String* string);
+  inline void AddOldObject(HeapObject* string);
 
   // Notifies the table that only a prefix of the new list is valid.
-  inline void ShrinkNewStrings(int position);
+  inline void ShrinkNewObjects(int position);
 
   // To speed up scavenge collections new space string are kept
   // separate from old space strings.
@@ -851,7 +855,7 @@ class Heap {
 
   // Finalizes an external string by deleting the associated external
   // data and clearing the resource pointer.
-  inline void FinalizeExternalString(String* string);
+  inline void FinalizeExternalString(HeapObject* string);
 
   // Allocates an uninitialized object.  The memory is non-executable if the
   // hardware and OS allow.
@@ -1656,7 +1660,7 @@ class Heap {
   // Performs a minor collection in new generation.
   void Scavenge();
 
-  static String* UpdateNewSpaceReferenceInExternalStringTableEntry(
+  static HeapObject* UpdateNewSpaceReferenceInExternalStringTableEntry(
       Heap* heap,
       Object** pointer);
 
diff --git a/src/mark-compact.cc b/src/mark-compact.cc
index b41b033..bf0aab8 100644
--- a/src/mark-compact.cc
+++ b/src/mark-compact.cc
@@ -1513,8 +1513,9 @@ class SymbolTableCleaner : public ObjectVisitor {
 
         // Since no objects have yet been moved we can safely access the map of
         // the object.
-        if (o->IsExternalString()) {
-          heap_->FinalizeExternalString(String::cast(*p));
+        if (o->IsExternalString() ||
+            (o->IsHeapObject() && HeapObject::cast(o)->map()->has_external_resource())) {
+          heap_->FinalizeExternalString(HeapObject::cast(*p));
         }
         // Set the entry to null_value (as deleted).
         *p = heap_->null_value();
@@ -2487,15 +2488,15 @@ static void UpdatePointer(HeapObject** p, HeapObject* object) {
 }
 
 
-static String* UpdateReferenceInExternalStringTableEntry(Heap* heap,
-                                                         Object** p) {
+static HeapObject* UpdateReferenceInExternalStringTableEntry(Heap* heap,
+                                                             Object** p) {
   MapWord map_word = HeapObject::cast(*p)->map_word();
 
   if (map_word.IsForwardingAddress()) {
-    return String::cast(map_word.ToForwardingAddress());
+    return HeapObject::cast(map_word.ToForwardingAddress());
   }
 
-  return String::cast(*p);
+  return HeapObject::cast(*p);
 }
 
 
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 1cfea84..6a80c9c 100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1343,7 +1343,7 @@ int JSObject::GetInternalFieldCount() {
   // Make sure to adjust for the number of in-object properties. These
   // properties do contribute to the size, but are not internal fields.
   return ((Size() - GetHeaderSize()) >> kPointerSizeLog2) -
-         map()->inobject_properties();
+         map()->inobject_properties() - (map()->has_external_resource()?1:0);
 }
 
 
@@ -1373,6 +1373,23 @@ void JSObject::SetInternalField(int index, Object* value) {
 }
 
 
+void JSObject::SetExternalResourceObject(Object *value) {
+  ASSERT(map()->has_external_resource());
+  int offset = GetHeaderSize() + kPointerSize * GetInternalFieldCount();
+  WRITE_FIELD(this, offset, value);
+  WRITE_BARRIER(GetHeap(), this, offset, value);
+}
+
+
+Object *JSObject::GetExternalResourceObject() { 
+  if (map()->has_external_resource()) {
+    return READ_FIELD(this, GetHeaderSize() + kPointerSize * GetInternalFieldCount());
+  } else {
+    return GetHeap()->undefined_value();
+  }
+}
+
+
 // Access fast-case object properties at index. The use of these routines
 // is needed to correctly distinguish between properties stored in-object and
 // properties stored in the properties array.
@@ -2755,6 +2772,20 @@ bool Map::is_shared() {
   return ((1 << kIsShared) & bit_field3()) != 0;
 }
  
+void Map::set_has_external_resource(bool value) {
+  if (value) {
+    set_bit_field(bit_field() | (1 << kHasExternalResource));
+  } else {
+    set_bit_field(bit_field() & ~(1 << kHasExternalResource));
+  }
+}
+
+bool Map::has_external_resource()
+{
+    return ((1 << kHasExternalResource) & bit_field()) != 0;
+}
+ 
+
 void Map::set_named_interceptor_is_fallback(bool value)
 {
   if (value) {
@@ -3301,6 +3332,8 @@ ACCESSORS(FunctionTemplateInfo, flag, Smi, kFlagOffset)
 ACCESSORS(ObjectTemplateInfo, constructor, Object, kConstructorOffset)
 ACCESSORS(ObjectTemplateInfo, internal_field_count, Object,
           kInternalFieldCountOffset)
+ACCESSORS(ObjectTemplateInfo, has_external_resource, Object,
+          kHasExternalResourceOffset)
 
 ACCESSORS(SignatureInfo, receiver, Object, kReceiverOffset)
 ACCESSORS(SignatureInfo, args, Object, kArgsOffset)
diff --git a/src/objects.h b/src/objects.h
index ed40061..c38d461 100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -1760,6 +1760,9 @@ class JSObject: public JSReceiver {
   inline Object* GetInternalField(int index);
   inline void SetInternalField(int index, Object* value);
 
+  inline void SetExternalResourceObject(Object *);
+  inline Object *GetExternalResourceObject();
+
   // The following lookup functions skip interceptors.
   void LocalLookupRealNamedProperty(String* name, LookupResult* result);
   void LookupRealNamedProperty(String* name, LookupResult* result);
@@ -4171,11 +4174,11 @@ class Map: public HeapObject {
 
   // Tells whether the instance has a call-as-function handler.
   inline void set_has_instance_call_handler() {
-    set_bit_field(bit_field() | (1 << kHasInstanceCallHandler));
+    set_bit_field3(bit_field3() | (1 << kHasInstanceCallHandler));
   }
 
   inline bool has_instance_call_handler() {
-    return ((1 << kHasInstanceCallHandler) & bit_field()) != 0;
+    return ((1 << kHasInstanceCallHandler) & bit_field3()) != 0;
   }
 
   inline void set_is_extensible(bool value);
@@ -4247,6 +4250,11 @@ class Map: public HeapObject {
   inline void set_named_interceptor_is_fallback(bool value);
   inline bool named_interceptor_is_fallback();
 
+  // Tells whether the instance has the space for an external resource
+  // object
+  inline void set_has_external_resource(bool value);
+  inline bool has_external_resource();
+
   // [prototype]: implicit prototype object.
   DECL_ACCESSORS(prototype, Object)
 
@@ -4487,7 +4495,7 @@ class Map: public HeapObject {
   static const int kHasNamedInterceptor = 3;
   static const int kHasIndexedInterceptor = 4;
   static const int kIsUndetectable = 5;
-  static const int kHasInstanceCallHandler = 6;
+  static const int kHasExternalResource = 6;
   static const int kIsAccessCheckNeeded = 7;
 
   // Bit positions for bit field 2
@@ -4512,6 +4520,7 @@ class Map: public HeapObject {
   // Bit positions for bit field 3
   static const int kIsShared = 0;
   static const int kNamedInterceptorIsFallback = 1;
+  static const int kHasInstanceCallHandler = 2;
 
   // Layout of the default cache. It holds alternating name and code objects.
   static const int kCodeCacheEntrySize = 2;
@@ -7539,6 +7548,7 @@ class ObjectTemplateInfo: public TemplateInfo {
  public:
   DECL_ACCESSORS(constructor, Object)
   DECL_ACCESSORS(internal_field_count, Object)
+  DECL_ACCESSORS(has_external_resource, Object)
 
   static inline ObjectTemplateInfo* cast(Object* obj);
 
@@ -7555,7 +7565,8 @@ class ObjectTemplateInfo: public TemplateInfo {
   static const int kConstructorOffset = TemplateInfo::kHeaderSize;
   static const int kInternalFieldCountOffset =
       kConstructorOffset + kPointerSize;
-  static const int kSize = kInternalFieldCountOffset + kPointerSize;
+  static const int kHasExternalResourceOffset = kInternalFieldCountOffset + kPointerSize;
+  static const int kSize = kHasExternalResourceOffset + kPointerSize;
 };
 
 
-- 
1.7.7.3