From fd57811644ac825ff7c93e2465745a41e82dbc58 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Tue, 15 Jul 2014 08:46:47 +0000 Subject: [PATCH] Fix up internalized strings after deserializing user code. R=mvstanton@chromium.org Review URL: https://codereview.chromium.org/387343002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22398 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/objects.cc | 27 ---------------- src/objects.h | 1 + src/serialize.cc | 63 ++++++++++++++++++++++++++++++++----- src/serialize.h | 7 +++++ test/cctest/test-serialize.cc | 73 +++++++++++++++++++++++++++++++++++++------ 5 files changed, 127 insertions(+), 44 deletions(-) diff --git a/src/objects.cc b/src/objects.cc index 34339f1..584dba5 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -14053,33 +14053,6 @@ int JSObject::GetEnumElementKeys(FixedArray* storage) { } -// StringKey simply carries a string object as key. -class StringKey : public HashTableKey { - public: - explicit StringKey(String* string) : - string_(string), - hash_(HashForObject(string)) { } - - bool IsMatch(Object* string) { - // We know that all entries in a hash table had their hash keys created. - // Use that knowledge to have fast failure. - if (hash_ != HashForObject(string)) { - return false; - } - return string_->Equals(String::cast(string)); - } - - uint32_t Hash() { return hash_; } - - uint32_t HashForObject(Object* other) { return String::cast(other)->Hash(); } - - Object* AsObject(Heap* heap) { return string_; } - - String* string_; - uint32_t hash_; -}; - - // StringSharedKeys are used as keys in the eval cache. class StringSharedKey : public HashTableKey { public: diff --git a/src/objects.h b/src/objects.h index bedcd2b..74ea5c7 100644 --- a/src/objects.h +++ b/src/objects.h @@ -9328,6 +9328,7 @@ class String: public Name { private: friend class Name; + friend class StringTableInsertionKey; static Handle SlowFlatten(Handle cons, PretenureFlag tenure); diff --git a/src/serialize.cc b/src/serialize.cc index 89ba1be..fe84894 100644 --- a/src/serialize.cc +++ b/src/serialize.cc @@ -718,6 +718,7 @@ class CodeAddressMap: public CodeEventLogger { Deserializer::Deserializer(SnapshotByteSource* source) : isolate_(NULL), + deserialize_code_(false), source_(source), external_reference_decoder_(NULL) { for (int i = 0; i < LAST_SPACE + 1; i++) { @@ -832,6 +833,54 @@ void Deserializer::RelinkAllocationSite(AllocationSite* site) { } +// Used to insert a deserialized internalized string into the string table. +class StringTableInsertionKey : public HashTableKey { + public: + explicit StringTableInsertionKey(String* string) + : string_(string), hash_(HashForObject(string)) { + ASSERT(string->IsInternalizedString()); + } + + virtual bool IsMatch(Object* string) { + // We know that all entries in a hash table had their hash keys created. + // Use that knowledge to have fast failure. + if (hash_ != HashForObject(string)) return false; + // We want to compare the content of two internalized strings here. + return string_->SlowEquals(String::cast(string)); + } + + virtual uint32_t Hash() V8_OVERRIDE { return hash_; } + + virtual uint32_t HashForObject(Object* key) V8_OVERRIDE { + return String::cast(key)->Hash(); + } + + MUST_USE_RESULT virtual Handle AsHandle(Isolate* isolate) + V8_OVERRIDE { + return handle(string_, isolate); + } + + String* string_; + uint32_t hash_; +}; + + +HeapObject* Deserializer::ProcessObjectFromSerializedCode(HeapObject* obj) { + if (obj->IsString()) { + String* string = String::cast(obj); + // Uninitialize hash field as the hash seed may have changed. + string->set_hash_field(String::kEmptyHashField); + if (string->IsInternalizedString()) { + DisallowHeapAllocation no_gc; + HandleScope scope(isolate_); + StringTableInsertionKey key(string); + return *StringTable::LookupKey(isolate_, &key); + } + } + return obj; +} + + // This routine writes the new object into the pointer provided and then // returns true if the new object was in young space and false otherwise. // The reason for this strange interface is that otherwise the object is @@ -843,7 +892,6 @@ void Deserializer::ReadObject(int space_number, Address address = Allocate(space_number, size); HeapObject* obj = HeapObject::FromAddress(address); isolate_->heap()->OnAllocationEvent(obj, size); - *write_back = obj; Object** current = reinterpret_cast(address); Object** limit = current + (size >> kPointerSizeLog2); if (FLAG_log_snapshot_positions) { @@ -854,10 +902,12 @@ void Deserializer::ReadObject(int space_number, // TODO(mvstanton): consider treating the heap()->allocation_sites_list() // as a (weak) root. If this root is relocated correctly, // RelinkAllocationSite() isn't necessary. - if (obj->IsAllocationSite()) { - RelinkAllocationSite(AllocationSite::cast(obj)); - } + if (obj->IsAllocationSite()) RelinkAllocationSite(AllocationSite::cast(obj)); + + // Fix up strings from serialized user code. + if (deserialize_code_) obj = ProcessObjectFromSerializedCode(obj); + *write_back = obj; #ifdef DEBUG bool is_codespace = (space_number == CODE_SPACE); ASSERT(obj->IsCode() == is_codespace); @@ -921,13 +971,13 @@ void Deserializer::ReadChunk(Object** current, emit_write_barrier = (space_number == NEW_SPACE); \ new_object = GetAddressFromEnd(data & kSpaceMask); \ } else if (where == kBuiltin) { \ + ASSERT(deserialize_code_); \ int builtin_id = source_->GetInt(); \ ASSERT_LE(0, builtin_id); \ ASSERT_LT(builtin_id, Builtins::builtin_count); \ Builtins::Name name = static_cast(builtin_id); \ new_object = isolate->builtins()->builtin(name); \ emit_write_barrier = false; \ - PrintF("BUILTIN how within %d, %d\n", how, within); \ } else { \ ASSERT(where == kBackrefWithSkip); \ int skip = source_->GetInt(); \ @@ -1859,8 +1909,6 @@ void CodeSerializer::SerializeObject(Object* o, HowToCode how_to_code, // TODO(yangguo) wire up stubs from stub cache. // TODO(yangguo) wire up script source. - // TODO(yangguo) wire up internalized strings - ASSERT(!heap_object->IsInternalizedString()); // TODO(yangguo) We cannot deal with different hash seeds yet. ASSERT(!heap_object->IsHashTable()); @@ -1916,6 +1964,7 @@ Object* CodeSerializer::Deserialize(Isolate* isolate, ScriptData* data) { SerializedCodeData scd(data); SnapshotByteSource payload(scd.Payload(), scd.PayloadLength()); Deserializer deserializer(&payload); + deserializer.ExpectSerializedCode(); STATIC_ASSERT(NEW_SPACE == 0); // TODO(yangguo) what happens if remaining new space is too small? for (int i = NEW_SPACE; i <= PROPERTY_CELL_SPACE; i++) { diff --git a/src/serialize.h b/src/serialize.h index 65ca34e..78be9d2 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -252,6 +252,10 @@ class Deserializer: public SerializerDeserializer { void FlushICacheForNewCodeObjects(); + // Call this to indicate that the serialized data represents user code. + // There are some more wiring up required in this case. + void ExpectSerializedCode() { deserialize_code_ = true; } + private: virtual void VisitPointers(Object** start, Object** end); @@ -272,6 +276,8 @@ class Deserializer: public SerializerDeserializer { Object** start, Object** end, int space, Address object_address); void ReadObject(int space_number, Object** write_back); + HeapObject* ProcessObjectFromSerializedCode(HeapObject* obj); + // This routine both allocates a new object, and also keeps // track of where objects have been allocated so that we can // fix back references when deserializing. @@ -291,6 +297,7 @@ class Deserializer: public SerializerDeserializer { // Cached current isolate. Isolate* isolate_; + bool deserialize_code_; SnapshotByteSource* source_; // This is the address of the next object that will be allocated in each diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc index 3177fd6..7d228ca 100644 --- a/test/cctest/test-serialize.cc +++ b/test/cctest/test-serialize.cc @@ -675,11 +675,10 @@ int CountBuiltins() { } -TEST(SerializeToplevel) { +TEST(SerializeToplevelOnePlusOne) { FLAG_serialize_toplevel = true; + LocalContext context; v8::HandleScope scope(CcTest::isolate()); - v8::Local context = CcTest::NewContext(PRINT_EXTENSION); - v8::Context::Scope context_scope(context); const char* source1 = "1 + 1"; const char* source2 = "1 + 2"; // Use alternate string to verify caching. @@ -702,21 +701,75 @@ TEST(SerializeToplevel) { int builtins_count = CountBuiltins(); - Handle info = + Handle copy = Compiler::CompileScript(source2_string, Handle(), 0, 0, false, Handle(isolate->native_context()), NULL, &cache, CONSUME_CACHED_DATA, NOT_NATIVES_CODE); - CHECK_NE(*orig, *info); - Handle fun = + CHECK_NE(*orig, *copy); + Handle copy_fun = isolate->factory()->NewFunctionFromSharedFunctionInfo( - info, isolate->native_context()); + copy, isolate->native_context()); Handle global(isolate->context()->global_object()); - Handle result = - Execution::Call(isolate, fun, global, 0, NULL).ToHandleChecked(); - CHECK_EQ(2, Handle::cast(result)->value()); + Handle copy_result = + Execution::Call(isolate, copy_fun, global, 0, NULL).ToHandleChecked(); + CHECK_EQ(2, Handle::cast(copy_result)->value()); CHECK_EQ(builtins_count, CountBuiltins()); delete cache; } + + +TEST(SerializeToplevelInternalizedString) { + FLAG_serialize_toplevel = true; + LocalContext context; + v8::HandleScope scope(CcTest::isolate()); + + const char* source1 = "'string1'"; + const char* source2 = "'string2'"; // Use alternate string to verify caching. + + Isolate* isolate = CcTest::i_isolate(); + Handle source1_string = isolate->factory() + ->NewStringFromUtf8(CStrVector(source1)) + .ToHandleChecked(); + + Handle source2_string = isolate->factory() + ->NewStringFromUtf8(CStrVector(source2)) + .ToHandleChecked(); + Handle global(isolate->context()->global_object()); + ScriptData* cache = NULL; + + Handle orig = + Compiler::CompileScript(source1_string, Handle(), 0, 0, false, + Handle(isolate->native_context()), NULL, + &cache, PRODUCE_CACHED_DATA, NOT_NATIVES_CODE); + Handle orig_fun = + isolate->factory()->NewFunctionFromSharedFunctionInfo( + orig, isolate->native_context()); + Handle orig_result = + Execution::Call(isolate, orig_fun, global, 0, NULL).ToHandleChecked(); + CHECK(orig_result->IsInternalizedString()); + + int builtins_count = CountBuiltins(); + + Handle copy = + Compiler::CompileScript(source2_string, Handle(), 0, 0, false, + Handle(isolate->native_context()), NULL, + &cache, CONSUME_CACHED_DATA, NOT_NATIVES_CODE); + CHECK_NE(*orig, *copy); + Handle copy_fun = + isolate->factory()->NewFunctionFromSharedFunctionInfo( + copy, isolate->native_context()); + CHECK_NE(*orig_fun, *copy_fun); + Handle copy_result = + Execution::Call(isolate, copy_fun, global, 0, NULL).ToHandleChecked(); + CHECK(orig_result.is_identical_to(copy_result)); + Handle expected = + isolate->factory()->NewStringFromAsciiChecked("string1"); + + CHECK(Handle::cast(copy_result)->Equals(*expected)); + CHECK_EQ(builtins_count, CountBuiltins()); + + delete cache; +} -- 2.7.4