Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps: backport rehash strings after deserialization #14345

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -425,12 +425,12 @@ parser.add_option('--without-perfctr',
# Dummy option for backwards compatibility
parser.add_option('--with-snapshot',
action='store_true',
dest='with_snapshot',
dest='unused_with_snapshot',
help=optparse.SUPPRESS_HELP)

parser.add_option('--without-snapshot',
action='store_true',
dest='unused_without_snapshot',
dest='without_snapshot',
help=optparse.SUPPRESS_HELP)

parser.add_option('--without-ssl',
Expand Down Expand Up @@ -813,7 +813,7 @@ def configure_node(o):
cross_compiling = (options.cross_compiling
if options.cross_compiling is not None
else target_arch != host_arch)
want_snapshots = 1 if options.with_snapshot else 0
want_snapshots = not options.without_snapshot
o['variables']['want_separate_host_toolset'] = int(
cross_compiling and want_snapshots)
o['variables']['want_separate_host_toolset_mkpeephole'] = int(
Expand Down Expand Up @@ -960,7 +960,7 @@ def configure_v8(o):
o['variables']['v8_optimized_debug'] = 0 # Compile with -O0 in debug builds.
o['variables']['v8_random_seed'] = 0 # Use a random seed for hash tables.
o['variables']['v8_promise_internal_field_count'] = 1 # Add internal field to promises for async hooks.
o['variables']['v8_use_snapshot'] = b(options.with_snapshot)
o['variables']['v8_use_snapshot'] = 'false' if options.without_snapshot else 'true'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: use b(want_snapshots)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a revert of the commit that disabled snapshots so I'd rather not change that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack.

o['variables']['v8_trace_maps'] = 1 if options.trace_maps else 0
o['variables']['node_use_v8_platform'] = b(not options.without_v8_platform)
o['variables']['node_use_bundled_v8'] = b(not options.without_bundled_v8)
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/include/v8-version.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#define V8_MAJOR_VERSION 5
#define V8_MINOR_VERSION 9
#define V8_BUILD_NUMBER 211
#define V8_PATCH_LEVEL 38
#define V8_PATCH_LEVEL 39

// Use 1 for candidates and 0 otherwise.
// (Boolean macro values are not supported by all preprocessors.)
Expand Down
13 changes: 11 additions & 2 deletions deps/v8/src/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,9 @@ StartupData SnapshotCreator::CreateBlob(
isolate->heap()->SetSerializedGlobalProxySizes(*global_proxy_sizes);
}

// We might rehash strings and re-sort descriptors. Clear the lookup cache.
isolate->descriptor_lookup_cache()->Clear();

// If we don't do this then we end up with a stray root pointing at the
// context even after we have disposed of the context.
isolate->heap()->CollectAllAvailableGarbage(
Expand Down Expand Up @@ -642,22 +645,28 @@ StartupData SnapshotCreator::CreateBlob(
// Serialize each context with a new partial serializer.
i::List<i::SnapshotData*> context_snapshots(num_additional_contexts + 1);

// TODO(6593): generalize rehashing, and remove this flag.
bool can_be_rehashed = true;

{
// The default snapshot does not support embedder fields.
i::PartialSerializer partial_serializer(
isolate, &startup_serializer, v8::SerializeInternalFieldsCallback());
partial_serializer.Serialize(&default_context, false);
can_be_rehashed = can_be_rehashed && partial_serializer.can_be_rehashed();
context_snapshots.Add(new i::SnapshotData(&partial_serializer));
}

for (int i = 0; i < num_additional_contexts; i++) {
i::PartialSerializer partial_serializer(
isolate, &startup_serializer, data->embedder_fields_serializers_[i]);
partial_serializer.Serialize(&contexts[i], true);
can_be_rehashed = can_be_rehashed && partial_serializer.can_be_rehashed();
context_snapshots.Add(new i::SnapshotData(&partial_serializer));
}

startup_serializer.SerializeWeakReferencesAndDeferred();
can_be_rehashed = can_be_rehashed && startup_serializer.can_be_rehashed();

#ifdef DEBUG
if (i::FLAG_external_reference_stats) {
Expand All @@ -666,8 +675,8 @@ StartupData SnapshotCreator::CreateBlob(
#endif // DEBUG

i::SnapshotData startup_snapshot(&startup_serializer);
StartupData result =
i::Snapshot::CreateSnapshotBlob(&startup_snapshot, &context_snapshots);
StartupData result = i::Snapshot::CreateSnapshotBlob(
&startup_snapshot, &context_snapshots, can_be_rehashed);

// Delete heap-allocated context snapshot instances.
for (const auto& context_snapshot : context_snapshots) {
Expand Down
6 changes: 6 additions & 0 deletions deps/v8/src/bootstrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,8 @@ Handle<JSFunction> Genesis::GetThrowTypeErrorIntrinsic(
DCHECK(false);
}

JSObject::MigrateSlowToFast(function, 0, "Bootstrapping");

return function;
}

Expand Down Expand Up @@ -1385,6 +1387,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
sloppy_function_map_writable_prototype_->SetConstructor(*function_fun);
strict_function_map_writable_prototype_->SetConstructor(*function_fun);
class_function_map_->SetConstructor(*function_fun);

JSObject::MigrateSlowToFast(function_fun, 0, "Bootstrapping");
}

{
Expand Down Expand Up @@ -2204,6 +2208,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
info->set_length(1);
native_context()->set_promise_reject_shared_fun(*info);
}

JSObject::MigrateSlowToFast(promise_fun, 0, "Bootstrapping");
}

{ // -- R e g E x p
Expand Down
2 changes: 2 additions & 0 deletions deps/v8/src/flag-definitions.h
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,8 @@ DEFINE_BOOL(abort_on_stack_overflow, false,
DEFINE_BOOL(randomize_hashes, true,
"randomize hashes to avoid predictable hash collisions "
"(with snapshots this option cannot override the baked-in seed)")
DEFINE_BOOL(rehash_snapshot, true,
"rehash strings from the snapshot to override the baked-in seed")
DEFINE_INT(hash_seed, 0,
"Fixed seed to use to hash property keys (0 means random)"
"(with snapshots this option cannot override the baked-in seed)")
Expand Down
17 changes: 9 additions & 8 deletions deps/v8/src/heap/heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5546,14 +5546,7 @@ bool Heap::SetUp() {

// Set up the seed that is used to randomize the string hash function.
DCHECK(hash_seed() == 0);
if (FLAG_randomize_hashes) {
if (FLAG_hash_seed == 0) {
int rnd = isolate()->random_number_generator()->NextInt();
set_hash_seed(Smi::FromInt(rnd & Name::kHashBitMask));
} else {
set_hash_seed(Smi::FromInt(FLAG_hash_seed));
}
}
if (FLAG_randomize_hashes) InitializeHashSeed();

for (int i = 0; i < static_cast<int>(v8::Isolate::kUseCounterFeatureCount);
i++) {
Expand Down Expand Up @@ -5591,6 +5584,14 @@ bool Heap::SetUp() {
return true;
}

void Heap::InitializeHashSeed() {
if (FLAG_hash_seed == 0) {
int rnd = isolate()->random_number_generator()->NextInt();
set_hash_seed(Smi::FromInt(rnd & Name::kHashBitMask));
} else {
set_hash_seed(Smi::FromInt(FLAG_hash_seed));
}
}

bool Heap::CreateHeapObjects() {
// Create initial maps.
Expand Down
3 changes: 3 additions & 0 deletions deps/v8/src/heap/heap.h
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,9 @@ class Heap {
// without actually creating any objects.
bool SetUp();

// (Re-)Initialize hash seed from flag or RNG.
void InitializeHashSeed();

// Bootstraps the object heap with the core set of objects required to run.
// Returns whether it succeeded.
bool CreateHeapObjects();
Expand Down
2 changes: 2 additions & 0 deletions deps/v8/src/js/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -1334,6 +1334,8 @@ var unscopables = {
keys: true,
};

%ToFastProperties(unscopables);

%AddNamedProperty(GlobalArray.prototype, unscopablesSymbol, unscopables,
DONT_ENUM | READ_ONLY);

Expand Down
11 changes: 10 additions & 1 deletion deps/v8/src/objects.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8823,7 +8823,13 @@ void Map::TraceAllTransitions(Map* map) {

void Map::ConnectTransition(Handle<Map> parent, Handle<Map> child,
Handle<Name> name, SimpleTransitionFlag flag) {
if (!parent->GetBackPointer()->IsUndefined(parent->GetIsolate())) {
Isolate* isolate = parent->GetIsolate();
// Do not track transitions during bootstrap except for element transitions.
if (isolate->bootstrapper()->IsActive() &&
!name.is_identical_to(isolate->factory()->elements_transition_symbol())) {
return;
}
if (!parent->GetBackPointer()->IsUndefined(isolate)) {
parent->set_owns_descriptors(false);
} else {
// |parent| is initial map and it must keep the ownership, there must be no
Expand Down Expand Up @@ -16779,6 +16785,9 @@ template class Dictionary<UnseededNumberDictionary,
UnseededNumberDictionaryShape,
uint32_t>;

template void
HashTable<GlobalDictionary, GlobalDictionaryShape, Handle<Name> >::Rehash(Handle<Name> key);

template Handle<SeededNumberDictionary>
Dictionary<SeededNumberDictionary, SeededNumberDictionaryShape, uint32_t>::New(
Isolate*, int at_least_space_for, PretenureFlag pretenure,
Expand Down
72 changes: 72 additions & 0 deletions deps/v8/src/snapshot/deserializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ void Deserializer::Deserialize(Isolate* isolate) {
LOG_CODE_EVENT(isolate_, LogCodeObjects());
LOG_CODE_EVENT(isolate_, LogBytecodeHandlers());
LOG_CODE_EVENT(isolate_, LogCompiledFunctions());

if (FLAG_rehash_snapshot && can_rehash_) Rehash();
}

MaybeHandle<Object> Deserializer::DeserializePartial(
Expand Down Expand Up @@ -151,6 +153,9 @@ MaybeHandle<Object> Deserializer::DeserializePartial(
// changed and logging should be added to notify the profiler et al of the
// new code, which also has to be flushed from instruction cache.
CHECK_EQ(start_address, code_space->top());

if (FLAG_rehash_snapshot && can_rehash_) RehashContext(Context::cast(root));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my own curiosity, root is logically always a Context here? I followed the logic down to Deserializer::ReadData() but things got complex fast from there on.


return Handle<Object>(root, isolate);
}

Expand All @@ -177,6 +182,63 @@ MaybeHandle<HeapObject> Deserializer::DeserializeObject(Isolate* isolate) {
}
}

// We only really just need HashForObject here.
class StringRehashKey : public HashTableKey {
public:
uint32_t HashForObject(Object* other) override {
return String::cast(other)->Hash();
}

static uint32_t StringHash(Object* obj) {
UNREACHABLE();
return String::cast(obj)->Hash();
}

bool IsMatch(Object* string) override {
UNREACHABLE();
return false;
}

uint32_t Hash() override {
UNREACHABLE();
return 0;
}

Handle<Object> AsHandle(Isolate* isolate) override {
UNREACHABLE();
return isolate->factory()->empty_string();
}
};

void Deserializer::Rehash() {
DCHECK(can_rehash_);
isolate_->heap()->InitializeHashSeed();
if (FLAG_profile_deserialization) {
PrintF("Re-initializing hash seed to %x\n",
isolate_->heap()->hash_seed()->value());
}
StringRehashKey string_rehash_key;
isolate_->heap()->string_table()->Rehash(&string_rehash_key);
SortMapDescriptors();
}

void Deserializer::RehashContext(Context* context) {
DCHECK(can_rehash_);
for (const auto& array : transition_arrays_) array->Sort();
Handle<Name> dummy = isolate_->factory()->empty_string();
context->global_object()->global_dictionary()->Rehash(dummy);
SortMapDescriptors();
}

void Deserializer::SortMapDescriptors() {
for (const auto& address : allocated_maps_) {
Map* map = Map::cast(HeapObject::FromAddress(address));
if (map->instance_descriptors()->number_of_descriptors() > 1) {
map->instance_descriptors()->Sort();
}
}
}

Deserializer::~Deserializer() {
#ifdef DEBUG
// Do not perform checks if we aborted deserialization.
Expand Down Expand Up @@ -367,6 +429,16 @@ HeapObject* Deserializer::PostProcessNewObject(HeapObject* obj, int space) {
string->resource()));
isolate_->heap()->RegisterExternalString(string);
}
if (FLAG_rehash_snapshot && can_rehash_ && !deserializing_user_code()) {
if (obj->IsString()) {
// Uninitialize hash field as we are going to reinitialize the hash seed.
String* string = String::cast(obj);
string->set_hash_field(String::kEmptyHashField);
} else if (obj->IsTransitionArray() &&
TransitionArray::cast(obj)->number_of_entries() > 1) {
transition_arrays_.Add(TransitionArray::cast(obj));
}
}
// Check alignment.
DCHECK_EQ(0, Heap::GetFillToAlign(obj->address(), obj->RequiredAlignment()));
return obj;
Expand Down
18 changes: 17 additions & 1 deletion deps/v8/src/snapshot/deserializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ class Deserializer : public SerializerDeserializer {
external_reference_table_(NULL),
deserialized_large_objects_(0),
deserializing_user_code_(deserializing_user_code),
next_alignment_(kWordAligned) {
next_alignment_(kWordAligned),
can_rehash_(false) {
DecodeReservation(data->Reservations());
}

Expand All @@ -62,6 +63,8 @@ class Deserializer : public SerializerDeserializer {
attached_objects_.Add(attached_object);
}

void SetRehashability(bool v) { can_rehash_ = v; }

private:
void VisitPointers(Object** start, Object** end) override;

Expand Down Expand Up @@ -117,6 +120,15 @@ class Deserializer : public SerializerDeserializer {
// snapshot by chunk index and offset.
HeapObject* GetBackReferencedObject(int space);

// Rehash after deserializing an isolate.
void Rehash();

// Rehash after deserializing a context.
void RehashContext(Context* context);

// Sort descriptors of deserialized maps using new string hashes.
void SortMapDescriptors();

// Cached current isolate.
Isolate* isolate_;

Expand Down Expand Up @@ -144,11 +156,15 @@ class Deserializer : public SerializerDeserializer {
List<AccessorInfo*> accessor_infos_;
List<Handle<String> > new_internalized_strings_;
List<Handle<Script> > new_scripts_;
List<TransitionArray*> transition_arrays_;

bool deserializing_user_code_;

AllocationAlignment next_alignment_;

// TODO(6593): generalize rehashing, and remove this flag.
bool can_rehash_;

DISALLOW_COPY_AND_ASSIGN(Deserializer);
};

Expand Down
Loading