From 501255df9cb6241a1f6c8d8a3361b9582fa481ef Mon Sep 17 00:00:00 2001 From: Aaron Kennedy Date: Tue, 4 Oct 2011 16:06:09 +1000 Subject: [PATCH 03/13] 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 key); V8EXPORT bool HasRealNamedProperty(Handle 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 New(Handle 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 obj = Utils::OpenHandle(this); + if (CanBeEncodedAsSmi(resource)) { + obj->SetExternalResourceObject(EncodeAsSmi(resource)); + } else { + obj->SetExternalResourceObject(*isolate->factory()->NewForeign(static_cast((void *)resource))); + } + if (!obj->IsSymbol()) { + isolate->heap()->external_string_table()->AddObject(*obj); + } +} + + +v8::Object::ExternalResource *v8::Object::GetExternalResource() { + i::Handle obj = Utils::OpenHandle(this); + i::Object* value = obj->GetExternalResourceObject(); + if (value->IsSmi()) { + return reinterpret_cast(i::Internals::GetExternalPointerFromSmi(value)); + } else if (value->IsForeign()) { + return reinterpret_cast(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 Factory::CreateApiFunction( Handle construct_stub = isolate()->builtins()->JSConstructStubApi(); int internal_field_count = 0; + bool has_external_resource = false; + if (!obj->instance_template()->IsUndefined()) { Handle instance_template = Handle( 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 Factory::CreateApiFunction( Handle map = Handle(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( - reinterpret_cast(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( + reinterpret_cast(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(Internals::GetExternalPointerFromSmi(value)); + } else if (value->IsForeign()) { + resource = reinterpret_cast(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(last - start)); + external_string_table_.ShrinkNewObjects(static_cast(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.4.1