From 1bf2c7405e33fefe4fe8b63a57957f01718885cf Mon Sep 17 00:00:00 2001 From: "iposva@chromium.org" Date: Wed, 11 Feb 2009 23:52:52 +0000 Subject: [PATCH] Allow the morphing of strings to external strings to avoid having to create copies in the embedding code (aka WebKit V8 bindings) on every external use. Review URL: http://codereview.chromium.org/21117 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1252 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- include/v8.h | 20 ++++++++++ src/api.cc | 93 ++++++++++++++++++++++++++++++++++++++----- src/heap.cc | 48 ++++++++++++----------- src/heap.h | 4 ++ src/mark-compact.cc | 28 +++++++++++++ src/objects-inl.h | 56 ++++++++++++++++++++++++++ src/objects.cc | 102 ++++++++++++++++++++++++++++++++++++++++++------ src/objects.h | 12 +++++- test/cctest/test-api.cc | 70 +++++++++++++++++++++++++++++++++ 9 files changed, 388 insertions(+), 45 deletions(-) diff --git a/include/v8.h b/include/v8.h index c9ec030..5757f2a 100644 --- a/include/v8.h +++ b/include/v8.h @@ -834,6 +834,16 @@ class EXPORT String : public Primitive { * external string resource. */ static Local NewExternal(ExternalStringResource* resource); + + /** + * Associate an external string resource with this string by transforming it + * in place so that existing references to this string in the JavaScript heap + * will use the external string resource. The external string resource's + * character contents needs to be equivalent to this string. + * Returns true if the string has been changed to be an external string. + * The string is not modified if the operation fails. + */ + bool MakeExternal(ExternalStringResource* resource); /** * Creates a new external string using the ascii data defined in the given @@ -844,6 +854,16 @@ class EXPORT String : public Primitive { * external string resource. */ static Local NewExternal(ExternalAsciiStringResource* resource); + + /** + * Associate an external string resource with this string by transforming it + * in place so that existing references to this string in the JavaScript heap + * will use the external string resource. The external string resource's + * character contents needs to be equivalent to this string. + * Returns true if the string has been changed to be an external string. + * The string is not modified if the operation fails. + */ + bool MakeExternal(ExternalAsciiStringResource* resource); /** Creates an undetectable string from the supplied ascii or utf-8 data.*/ static Local NewUndetectable(const char* data, int length = -1); diff --git a/src/api.cc b/src/api.cc index b12d189..2d64444 100644 --- a/src/api.cc +++ b/src/api.cc @@ -2440,22 +2440,58 @@ i::Handle NewExternalAsciiStringHandle( static void DisposeExternalString(v8::Persistent obj, void* parameter) { - v8::String::ExternalStringResource* resource = - reinterpret_cast(parameter); - const size_t total_size = resource->length() * sizeof(*resource->data()); - i::Counters::total_external_string_memory.Decrement(total_size); - delete resource; + i::ExternalTwoByteString* str = + i::ExternalTwoByteString::cast(*Utils::OpenHandle(*obj)); + + // External symbols are deleted when they are pruned out of the symbol + // table. Generally external symbols are not registered with the weak handle + // callbacks unless they are upgraded to a symbol after being externalized. + if (!str->IsSymbol()) { + v8::String::ExternalStringResource* resource = + reinterpret_cast(parameter); + if (resource != NULL) { + const size_t total_size = resource->length() * sizeof(*resource->data()); + i::Counters::total_external_string_memory.Decrement(total_size); + + // The object will continue to live in the JavaScript heap until the + // handle is entirely cleaned out by the next GC. For example the + // destructor for the resource below could bring it back to life again. + // Which is why we make sure to not have a dangling pointer here. + str->set_resource(NULL); + delete resource; + } + } + + // In any case we do not need this handle any longer. obj.Dispose(); } static void DisposeExternalAsciiString(v8::Persistent obj, void* parameter) { - v8::String::ExternalAsciiStringResource* resource = - reinterpret_cast(parameter); - const size_t total_size = resource->length() * sizeof(*resource->data()); - i::Counters::total_external_string_memory.Decrement(total_size); - delete resource; + i::ExternalAsciiString* str = + i::ExternalAsciiString::cast(*Utils::OpenHandle(*obj)); + + // External symbols are deleted when they are pruned out of the symbol + // table. Generally external symbols are not registered with the weak handle + // callbacks unless they are upgraded to a symbol after being externalized. + if (!str->IsSymbol()) { + v8::String::ExternalAsciiStringResource* resource = + reinterpret_cast(parameter); + if (resource != NULL) { + const size_t total_size = resource->length() * sizeof(*resource->data()); + i::Counters::total_external_string_memory.Decrement(total_size); + + // The object will continue to live in the JavaScript heap until the + // handle is entirely cleaned out by the next GC. For example the + // destructor for the resource below could bring it back to life again. + // Which is why we make sure to not have a dangling pointer here. + str->set_resource(NULL); + delete resource; + } + } + + // In any case we do not need this handle any longer. obj.Dispose(); } @@ -2475,6 +2511,24 @@ Local v8::String::NewExternal( } +bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) { + if (IsDeadCheck("v8::String::MakeExternal()")) return false; + if (this->IsExternal()) return false; // Already an external string. + i::Handle obj = Utils::OpenHandle(this); + bool result = obj->MakeExternal(resource); + if (result && !obj->IsSymbol()) { + // Operation was successful and the string is not a symbol. In this case + // we need to make sure that the we call the destructor for the external + // resource when no strong references to the string remain. + i::Handle handle = i::GlobalHandles::Create(*obj); + i::GlobalHandles::MakeWeak(handle.location(), + resource, + &DisposeExternalString); + } + return result; +} + + Local v8::String::NewExternal( v8::String::ExternalAsciiStringResource* resource) { EnsureInitialized("v8::String::NewExternal()"); @@ -2490,6 +2544,25 @@ Local v8::String::NewExternal( } +bool v8::String::MakeExternal( + v8::String::ExternalAsciiStringResource* resource) { + if (IsDeadCheck("v8::String::MakeExternal()")) return false; + if (this->IsExternal()) return false; // Already an external string. + i::Handle obj = Utils::OpenHandle(this); + bool result = obj->MakeExternal(resource); + if (result && !obj->IsSymbol()) { + // Operation was successful and the string is not a symbol. In this case + // we need to make sure that the we call the destructor for the external + // resource when no strong references to the string remain. + i::Handle handle = i::GlobalHandles::Create(*obj); + i::GlobalHandles::MakeWeak(handle.location(), + resource, + &DisposeExternalAsciiString); + } + return result; +} + + Local v8::Object::New() { EnsureInitialized("v8::Object::New()"); LOG_API("Object::New"); diff --git a/src/heap.cc b/src/heap.cc index 8009544..869b33c 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -1519,16 +1519,9 @@ Object* Heap::AllocateExternalStringFromAscii( Object* Heap::AllocateExternalStringFromTwoByte( ExternalTwoByteString::Resource* resource) { - Map* map; int length = resource->length(); - if (length <= String::kMaxShortStringSize) { - map = short_external_string_map(); - } else if (length <= String::kMaxMediumStringSize) { - map = medium_external_string_map(); - } else { - map = long_external_string_map(); - } + Map* map = ExternalTwoByteString::StringMap(length); Object* result = Allocate(map, NEW_SPACE); if (result->IsFailure()) return result; @@ -1542,16 +1535,9 @@ Object* Heap::AllocateExternalStringFromTwoByte( Object* Heap::AllocateExternalSymbolFromTwoByte( ExternalTwoByteString::Resource* resource) { - Map* map; int length = resource->length(); - if (length <= String::kMaxShortStringSize) { - map = short_external_symbol_map(); - } else if (length <= String::kMaxMediumStringSize) { - map = medium_external_symbol_map(); - } else { - map = long_external_symbol_map(); - } + Map* map = ExternalTwoByteString::SymbolMap(length); Object* result = Allocate(map, OLD_DATA_SPACE); if (result->IsFailure()) return result; @@ -1618,6 +1604,18 @@ Object* Heap::AllocateByteArray(int length) { } +void Heap::CreateFillerObjectAt(Address addr, int size) { + if (size == 0) return; + HeapObject* filler = HeapObject::FromAddress(addr); + if (size == kPointerSize) { + filler->set_map(Heap::one_word_filler_map()); + } else { + filler->set_map(Heap::byte_array_map()); + ByteArray::cast(filler)->set_length(ByteArray::LengthFor(size)); + } +} + + Object* Heap::CreateCode(const CodeDesc& desc, ScopeInfo<>* sinfo, Code::Flags flags, @@ -2069,18 +2067,24 @@ Map* Heap::SymbolMapForString(String* string) { return long_sliced_ascii_symbol_map(); } - if (map == short_external_string_map()) return short_external_string_map(); - if (map == medium_external_string_map()) return medium_external_string_map(); - if (map == long_external_string_map()) return long_external_string_map(); + if (map == short_external_string_map()) { + return short_external_symbol_map(); + } + if (map == medium_external_string_map()) { + return medium_external_symbol_map(); + } + if (map == long_external_string_map()) { + return long_external_symbol_map(); + } if (map == short_external_ascii_string_map()) { - return short_external_ascii_string_map(); + return short_external_ascii_symbol_map(); } if (map == medium_external_ascii_string_map()) { - return medium_external_ascii_string_map(); + return medium_external_ascii_symbol_map(); } if (map == long_external_ascii_string_map()) { - return long_external_ascii_string_map(); + return long_external_ascii_symbol_map(); } // No match found. diff --git a/src/heap.h b/src/heap.h index 87323e1..fd98869 100644 --- a/src/heap.h +++ b/src/heap.h @@ -556,6 +556,10 @@ class Heap : public AllStatic { AllocationSpace space, AllocationSpace retry_space); + // Initialize a filler object to keep the ability to iterate over the heap + // when shortening objects. + static void CreateFillerObjectAt(Address addr, int size); + // Makes a new native code object // Returns Failure::RetryAfterGC(requested_bytes, space) if the allocation // failed. On success, the pointer to the Code object is stored in the diff --git a/src/mark-compact.cc b/src/mark-compact.cc index 39ad689..4ab4cfe 100644 --- a/src/mark-compact.cc +++ b/src/mark-compact.cc @@ -390,6 +390,34 @@ class SymbolTableCleaner : public ObjectVisitor { // Visit all HeapObject pointers in [start, end). for (Object** p = start; p < end; p++) { if ((*p)->IsHeapObject() && !HeapObject::cast(*p)->IsMarked()) { + // Check if the symbol being pruned is an external symbol. We need to + // delete the associated external data as this symbol is going away. + + // Since the object is not marked we can access its map word safely + // without having to worry about marking bits in the object header. + Map* map = HeapObject::cast(*p)->map(); + // Since no objects have yet been moved we can safely access the map of + // the object. + uint32_t type = map->instance_type(); + bool is_external = (type & kStringRepresentationMask) == + kExternalStringTag; + if (is_external) { + bool is_two_byte = (type & kStringEncodingMask) == kTwoByteStringTag; + byte* resource_addr = reinterpret_cast(*p) + + ExternalString::kResourceOffset - + kHeapObjectTag; + if (is_two_byte) { + v8::String::ExternalStringResource* resource = + *reinterpret_cast + (resource_addr); + delete resource; + } else { + v8::String::ExternalAsciiStringResource* resource = + *reinterpret_cast + (resource_addr); + delete resource; + } + } // Set the entry to null_value (as deleted). *p = Heap::null_value(); pointers_removed_++; diff --git a/src/objects-inl.h b/src/objects-inl.h index 31fdb4f..66ca97e 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -1639,6 +1639,34 @@ void ExternalAsciiString::set_resource( } +Map* ExternalAsciiString::StringMap(int length) { + Map* map; + // Number of characters: determines the map. + if (length <= String::kMaxShortStringSize) { + map = Heap::short_external_ascii_string_map(); + } else if (length <= String::kMaxMediumStringSize) { + map = Heap::medium_external_ascii_string_map(); + } else { + map = Heap::long_external_ascii_string_map(); + } + return map; +} + + +Map* ExternalAsciiString::SymbolMap(int length) { + Map* map; + // Number of characters: determines the map. + if (length <= String::kMaxShortStringSize) { + map = Heap::short_external_ascii_symbol_map(); + } else if (length <= String::kMaxMediumStringSize) { + map = Heap::medium_external_ascii_symbol_map(); + } else { + map = Heap::long_external_ascii_symbol_map(); + } + return map; +} + + ExternalTwoByteString::Resource* ExternalTwoByteString::resource() { return *reinterpret_cast(FIELD_ADDR(this, kResourceOffset)); } @@ -1650,6 +1678,34 @@ void ExternalTwoByteString::set_resource( } +Map* ExternalTwoByteString::StringMap(int length) { + Map* map; + // Number of characters: determines the map. + if (length <= String::kMaxShortStringSize) { + map = Heap::short_external_string_map(); + } else if (length <= String::kMaxMediumStringSize) { + map = Heap::medium_external_string_map(); + } else { + map = Heap::long_external_string_map(); + } + return map; +} + + +Map* ExternalTwoByteString::SymbolMap(int length) { + Map* map; + // Number of characters: determines the map. + if (length <= String::kMaxShortStringSize) { + map = Heap::short_external_symbol_map(); + } else if (length <= String::kMaxMediumStringSize) { + map = Heap::medium_external_symbol_map(); + } else { + map = Heap::long_external_symbol_map(); + } + return map; +} + + byte ByteArray::get(int index) { ASSERT(index >= 0 && index < this->length()); return READ_BYTE_FIELD(this, kHeaderSize + index * kCharSize); diff --git a/src/objects.cc b/src/objects.cc index ce0b42b..ee0c8e3 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -667,6 +667,92 @@ Object* String::TryFlatten(StringShape shape) { } +bool String::MakeExternal(v8::String::ExternalStringResource* resource) { +#ifdef DEBUG + { // NOLINT (presubmit.py gets confused about if and braces) + // Assert that the resource and the string are equivalent. + ASSERT(static_cast(this->length()) == resource->length()); + SmartPointer smart_chars = this->ToWideCString(); + ASSERT(memcmp(*smart_chars, + resource->data(), + resource->length()*sizeof(**smart_chars)) == 0); + } +#endif // DEBUG + + int size = this->Size(); // Byte size of the original string. + if (size < ExternalString::kSize) { + // The string is too small to fit an external String in its place. This can + // only happen for zero length strings. + return false; + } + ASSERT(size >= ExternalString::kSize); + bool is_symbol = this->IsSymbol(); + int length = this->length(); + + // Morph the object to an external string by adjusting the map and + // reinitializing the fields. + this->set_map(ExternalTwoByteString::StringMap(length)); + ExternalTwoByteString* self = ExternalTwoByteString::cast(this); + self->set_length(length); + self->set_resource(resource); + // Additionally make the object into an external symbol if the original string + // was a symbol to start with. + if (is_symbol) { + self->Hash(); // Force regeneration of the hash value. + // Now morph this external string into a external symbol. + self->set_map(ExternalTwoByteString::SymbolMap(length)); + } + + // Fill the remainder of the string with dead wood. + int new_size = this->Size(); // Byte size of the external String object. + Heap::CreateFillerObjectAt(this->address() + new_size, size - new_size); + return true; +} + + +bool String::MakeExternal(v8::String::ExternalAsciiStringResource* resource) { +#ifdef DEBUG + { // NOLINT (presubmit.py gets confused about if and braces) + // Assert that the resource and the string are equivalent. + ASSERT(static_cast(this->length()) == resource->length()); + SmartPointer smart_chars = this->ToCString(); + ASSERT(memcmp(*smart_chars, + resource->data(), + resource->length()*sizeof(**smart_chars)) == 0); + } +#endif // DEBUG + + int size = this->Size(); // Byte size of the original string. + if (size < ExternalString::kSize) { + // The string is too small to fit an external String in its place. This can + // only happen for zero length strings. + return false; + } + ASSERT(size >= ExternalString::kSize); + bool is_symbol = this->IsSymbol(); + int length = this->length(); + + // Morph the object to an external string by adjusting the map and + // reinitializing the fields. + this->set_map(ExternalAsciiString::StringMap(length)); + ExternalAsciiString* self = ExternalAsciiString::cast(this); + self->set_length(length); + self->set_resource(resource); + // Additionally make the object into an external symbol if the original string + // was a symbol to start with. + if (is_symbol) { + self->Hash(); // Force regeneration of the hash value. + // Now morph this external string into a external symbol. + self->set_map(ExternalAsciiString::SymbolMap(length)); + } + + // Fill the remainder of the string with dead wood. + int new_size = this->Size(); // Byte size of the external String object. + Heap::CreateFillerObjectAt(this->address() + new_size, size - new_size); + return true; +} + + void String::StringShortPrint(StringStream* accumulator) { StringShape shape(this); int len = length(shape); @@ -1927,23 +2013,15 @@ Object* JSObject::NormalizeProperties(PropertyNormalizationMode mode) { if (obj->IsFailure()) return obj; Map* new_map = Map::cast(obj); - // Clear inobject properties if needed by adjusting the instance - // size and putting in a filler or byte array instead of the - // inobject properties. + // Clear inobject properties if needed by adjusting the instance size and + // putting in a filler object instead of the inobject properties. if (mode == CLEAR_INOBJECT_PROPERTIES && map()->inobject_properties() > 0) { int instance_size_delta = map()->inobject_properties() * kPointerSize; int new_instance_size = map()->instance_size() - instance_size_delta; new_map->set_inobject_properties(0); new_map->set_instance_size(new_instance_size); - if (instance_size_delta == kPointerSize) { - WRITE_FIELD(this, new_instance_size, Heap::one_word_filler_map()); - } else { - int byte_array_length = ByteArray::LengthFor(instance_size_delta); - int byte_array_length_offset = new_instance_size + kPointerSize; - WRITE_FIELD(this, new_instance_size, Heap::byte_array_map()); - WRITE_INT_FIELD(this, byte_array_length_offset, byte_array_length); - } - WRITE_BARRIER(this, new_instance_size); + Heap::CreateFillerObjectAt(this->address() + new_instance_size, + instance_size_delta); } new_map->set_unused_property_fields(0); diff --git a/src/objects.h b/src/objects.h index d7d9425..c562ad1 100644 --- a/src/objects.h +++ b/src/objects.h @@ -945,7 +945,7 @@ class MapWord BASE_EMBEDDED { // Bits used by the marking phase of the garbage collector. // - // The first word of a heap object is normall a map pointer. The last two + // The first word of a heap object is normally a map pointer. The last two // bits are tagged as '01' (kHeapObjectTag). We reuse the last two bits to // mark an object as live and/or overflowed: // last bit = 0, marked as alive @@ -3205,6 +3205,10 @@ class String: public HeapObject { uint32_t* index, int length); + // Externalization. + bool MakeExternal(v8::String::ExternalStringResource* resource); + bool MakeExternal(v8::String::ExternalAsciiStringResource* resource); + // Conversion. inline bool AsArrayIndex(uint32_t* index); @@ -3584,6 +3588,9 @@ class ExternalAsciiString: public ExternalString { unsigned* offset, unsigned chars); + // Identify the map for the external string/symbol with a particular length. + static inline Map* StringMap(int length); + static inline Map* SymbolMap(int length); private: DISALLOW_IMPLICIT_CONSTRUCTORS(ExternalAsciiString); }; @@ -3613,6 +3620,9 @@ class ExternalTwoByteString: public ExternalString { unsigned* offset_ptr, unsigned chars); + // Identify the map for the external string/symbol with a particular length. + static inline Map* StringMap(int length); + static inline Map* SymbolMap(int length); private: DISALLOW_IMPLICIT_CONSTRUCTORS(ExternalTwoByteString); }; diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 942ae05..1638baa 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -491,6 +491,76 @@ THREADED_TEST(ScriptUsingAsciiStringResource) { } +THREADED_TEST(ScriptMakingExternalString) { + TestResource::dispose_count = 0; + uint16_t* two_byte_source = AsciiToTwoByteString("1 + 2 * 3"); + { + v8::HandleScope scope; + LocalContext env; + Local source = String::New(two_byte_source); + bool success = source->MakeExternal(new TestResource(two_byte_source)); + CHECK(success); + Local