From bcbb553db0573e0ef3ed2a6b92d47a03e67c8c07 Mon Sep 17 00:00:00 2001 From: Leszek Swirski Date: Wed, 15 Jan 2020 12:59:56 +0100 Subject: [PATCH] [offthread] Add OffThreadFactory support to AST strings Add support for internalizing an AstValueFactory using the off-thread factory. Includes adding ConsString support to OffThreadFactory. This introduces a Handle union wrapper, which is used in locations that can store a Handle or an OffThreadHandle. This is used in this patch for the internalized "string" field of AST strings, and will be able to be used for other similar fields in other classes (e.g. the ScopeInfo handle in Scope, object boilerplate descriptor handles, the inferred name handle on FunctionLiterals, etc.). It has a Factory-templated getter which returns the appropriate handle for the factory, and a debug-only tag to make sure the right getter is used at runtime. This union wrapper currently decomposes implicitly to a Handle if the getter is not called, to minimise code changes, but this implicit conversion will likely be removed for clarity. Bug: chromium:1011762 Change-Id: I5dd3a7bbdc483b66f5ff687e0079c545b636dc13 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993971 Commit-Queue: Leszek Swirski Reviewed-by: Ulan Degenbaev Reviewed-by: Toon Verwaest Cr-Commit-Position: refs/heads/master@{#65816} --- src/DEPS | 1 + src/ast/ast-value-factory.cc | 61 +++++-- src/ast/ast-value-factory.h | 49 ++++-- src/ast/ast.cc | 18 +- src/ast/ast.h | 12 +- src/ast/modules.cc | 11 +- src/ast/prettyprinter.cc | 3 +- src/ast/scopes.cc | 4 +- src/ast/variables.h | 2 +- src/codegen/compiler.cc | 8 +- src/debug/debug-scopes.cc | 2 +- src/debug/liveedit.cc | 2 +- src/execution/messages.cc | 7 +- src/handles/handles.h | 72 +++++++- src/heap/factory-base.cc | 103 +++++++++++ src/heap/factory-base.h | 10 ++ src/heap/factory.cc | 115 +------------ src/heap/factory.h | 11 +- src/heap/off-thread-factory-inl.h | 25 +++ src/heap/off-thread-factory.cc | 18 ++ src/heap/off-thread-factory.h | 8 + src/interpreter/bytecode-generator.cc | 2 +- src/interpreter/constant-array-builder.cc | 2 +- src/interpreter/interpreter.cc | 5 +- src/objects/literal-objects.cc | 3 +- src/objects/scope-info.cc | 3 +- src/parsing/parser.cc | 4 +- src/parsing/parsing.cc | 2 +- .../pending-compilation-error-handler.cc | 4 +- test/cctest/interpreter/test-interpreter.cc | 160 +++++++++--------- test/cctest/test-parsing.cc | 8 +- .../heap/off-thread-factory-unittest.cc | 87 ++++++++-- .../bytecode-array-builder-unittest.cc | 9 +- .../bytecode-array-iterator-unittest.cc | 2 +- ...bytecode-array-random-iterator-unittest.cc | 14 +- .../constant-array-builder-unittest.cc | 14 +- 36 files changed, 554 insertions(+), 307 deletions(-) create mode 100644 src/heap/off-thread-factory-inl.h diff --git a/src/DEPS b/src/DEPS index 3723d3e9a2..3e802dac97 100644 --- a/src/DEPS +++ b/src/DEPS @@ -16,6 +16,7 @@ include_rules = [ "+src/heap/heap-inl.h", "+src/heap/heap-write-barrier-inl.h", "+src/heap/heap-write-barrier.h", + "+src/heap/off-thread-factory-inl.h", "+src/heap/off-thread-factory.h", "+src/heap/read-only-heap-inl.h", "+src/heap/read-only-heap.h", diff --git a/src/ast/ast-value-factory.cc b/src/ast/ast-value-factory.cc index 54fdeaa0b3..7c1cafd08b 100644 --- a/src/ast/ast-value-factory.cc +++ b/src/ast/ast-value-factory.cc @@ -27,6 +27,9 @@ #include "src/ast/ast-value-factory.h" +#include "src/common/globals.h" +#include "src/heap/factory-inl.h" +#include "src/heap/off-thread-factory-inl.h" #include "src/objects/objects-inl.h" #include "src/objects/objects.h" #include "src/strings/char-predicates-inl.h" @@ -54,20 +57,42 @@ class OneByteStringStream { } // namespace -void AstRawString::Internalize(Isolate* isolate) { +void AstRawString::Internalize(Factory* factory) { DCHECK(!has_string_); if (literal_bytes_.length() == 0) { - set_string(isolate->factory()->empty_string()); + set_string(factory->empty_string()); } else if (is_one_byte()) { OneByteStringKey key(hash_field_, literal_bytes_); - set_string(StringTable::LookupKey(isolate, &key)); + set_string(factory->InternalizeStringWithKey(&key)); } else { TwoByteStringKey key(hash_field_, Vector::cast(literal_bytes_)); - set_string(StringTable::LookupKey(isolate, &key)); + set_string(factory->InternalizeStringWithKey(&key)); } } +void AstRawString::Internalize(OffThreadFactory* factory) { + DCHECK(!has_string_); + if (literal_bytes_.length() == 0) { + set_string(factory->empty_string()); + return; + } + + // For the off-thread case, we already de-duplicated the AstRawStrings during + // construction and don't have access to the main thread string table yet, so + // we just unconditionally create strings and will internalize them properly + // during merging. + OffThreadHandle string; + if (is_one_byte()) { + string = factory->NewOneByteInternalizedString( + Vector::cast(literal_bytes_), hash_field()); + } else { + string = factory->NewTwoByteInternalizedString( + Vector::cast(literal_bytes_), hash_field()); + } + set_string(string); +} + bool AstRawString::AsArrayIndex(uint32_t* index) const { // The StringHasher will set up the hash in such a way that we can use it to // figure out whether the string is convertible to an array index. @@ -134,22 +159,28 @@ bool AstRawString::Compare(void* a, void* b) { } } -void AstConsString::Internalize(Isolate* isolate) { +template +void AstConsString::Internalize(Factory* factory) { if (IsEmpty()) { - set_string(isolate->factory()->empty_string()); + set_string(factory->empty_string()); return; } // AstRawStrings are internalized before AstConsStrings, so // AstRawString::string() will just work. - Handle tmp(segment_.string->string()); + FactoryHandle tmp(segment_.string->string().get()); for (AstConsString::Segment* current = segment_.next; current != nullptr; current = current->next) { - tmp = isolate->factory() - ->NewConsString(current->string->string(), tmp) + tmp = factory + ->NewConsString(current->string->string().get(), tmp, + AllocationType::kOld) .ToHandleChecked(); } set_string(tmp); } +template EXPORT_TEMPLATE_DEFINE( + V8_BASE_EXPORT) void AstConsString::Internalize(Factory* factory); +template EXPORT_TEMPLATE_DEFINE(V8_BASE_EXPORT) void AstConsString::Internalize< + OffThreadFactory>(OffThreadFactory* factory); std::forward_list AstConsString::ToRawStrings() const { std::forward_list result; @@ -250,24 +281,30 @@ AstConsString* AstValueFactory::NewConsString(const AstRawString* str1, return NewConsString()->AddString(zone_, str1)->AddString(zone_, str2); } -void AstValueFactory::Internalize(Isolate* isolate) { +template +void AstValueFactory::Internalize(Factory* factory) { // Strings need to be internalized before values, because values refer to // strings. for (AstRawString* current = strings_; current != nullptr;) { AstRawString* next = current->next(); - current->Internalize(isolate); + current->Internalize(factory); current = next; } // AstConsStrings refer to AstRawStrings. for (AstConsString* current = cons_strings_; current != nullptr;) { AstConsString* next = current->next(); - current->Internalize(isolate); + current->Internalize(factory); current = next; } ResetStrings(); } +template EXPORT_TEMPLATE_DEFINE( + V8_BASE_EXPORT) void AstValueFactory::Internalize(Factory* + factory); +template EXPORT_TEMPLATE_DEFINE(V8_BASE_EXPORT) void AstValueFactory:: + Internalize(OffThreadFactory* factory); AstRawString* AstValueFactory::GetString(uint32_t hash_field, bool is_one_byte, Vector literal_bytes) { diff --git a/src/ast/ast-value-factory.h b/src/ast/ast-value-factory.h index c26a762278..0c2d1aaee2 100644 --- a/src/ast/ast-value-factory.h +++ b/src/ast/ast-value-factory.h @@ -34,6 +34,7 @@ #include "src/common/globals.h" #include "src/execution/isolate.h" #include "src/heap/factory.h" +#include "src/heap/off-thread-factory.h" #include "src/numbers/conversions.h" // Ast(Raw|Cons)String and AstValueFactory are for storing strings and @@ -56,7 +57,8 @@ class AstRawString final : public ZoneObject { V8_EXPORT_PRIVATE bool IsOneByteEqualTo(const char* data) const; uint16_t FirstCharacter() const; - void Internalize(Isolate* isolate); + void Internalize(Factory* factory); + void Internalize(OffThreadFactory* factory); // Access the physical representation: bool is_one_byte() const { return is_one_byte_; } @@ -68,10 +70,9 @@ class AstRawString final : public ZoneObject { uint32_t Hash() const { return hash_field_ >> Name::kHashShift; } // This function can be called after internalizing. - V8_INLINE Handle string() const { - DCHECK_NOT_NULL(string_); + V8_INLINE HandleOrOffThreadHandle string() const { DCHECK(has_string_); - return Handle(string_); + return string_; } private: @@ -96,20 +97,18 @@ class AstRawString final : public ZoneObject { return &next_; } - void set_string(Handle string) { + void set_string(HandleOrOffThreadHandle string) { DCHECK(!string.is_null()); DCHECK(!has_string_); - string_ = string.location(); + string_ = string; #ifdef DEBUG has_string_ = true; #endif } - // {string_} is stored as Address* instead of a Handle so it can be - // stored in a union with {next_}. union { AstRawString* next_; - Address* string_; + HandleOrOffThreadHandle string_; }; Vector literal_bytes_; // Memory owned by Zone. @@ -144,11 +143,13 @@ class AstConsString final : public ZoneObject { return segment_.string == nullptr; } - void Internalize(Isolate* isolate); + template + void Internalize(Factory* factory); - V8_INLINE Handle string() const { - DCHECK_NOT_NULL(string_); - return Handle(string_); + // This function can be called after internalizing. + V8_INLINE HandleOrOffThreadHandle string() const { + DCHECK(has_string_); + return string_; } std::forward_list ToRawStrings() const; @@ -161,12 +162,20 @@ class AstConsString final : public ZoneObject { AstConsString* next() const { return next_; } AstConsString** next_location() { return &next_; } + void set_string(HandleOrOffThreadHandle string) { + DCHECK(!string.is_null()); + DCHECK(!has_string_); + string_ = string; +#ifdef DEBUG + has_string_ = true; +#endif + } + // {string_} is stored as Address* instead of a Handle so it can be // stored in a union with {next_}. - void set_string(Handle string) { string_ = string.location(); } union { AstConsString* next_; - Address* string_; + HandleOrOffThreadHandle string_; }; struct Segment { @@ -174,6 +183,13 @@ class AstConsString final : public ZoneObject { AstConsString::Segment* next; }; Segment segment_; + +#ifdef DEBUG + // (Debug-only:) Verify the object life-cylce: Some functions may only be + // called after internalization (that is, after a v8::internal::String has + // been set); some only before. + bool has_string_ = false; +#endif }; enum class AstSymbol : uint8_t { kHomeObjectSymbol }; @@ -313,7 +329,8 @@ class AstValueFactory { V8_EXPORT_PRIVATE AstConsString* NewConsString(const AstRawString* str1, const AstRawString* str2); - V8_EXPORT_PRIVATE void Internalize(Isolate* isolate); + template + V8_EXPORT_PRIVATE void Internalize(Factory* factory); #define F(name, str) \ const AstRawString* name##_string() const { \ diff --git a/src/ast/ast.cc b/src/ast/ast.cc index 9b792ee92d..bae5a1d1ed 100644 --- a/src/ast/ast.cc +++ b/src/ast/ast.cc @@ -226,10 +226,6 @@ bool FunctionLiteral::SafeToSkipArgumentsAdaptor() const { scope()->rest_parameter() == nullptr; } -Handle FunctionLiteral::name(Isolate* isolate) const { - return raw_name_ ? raw_name_->string() : isolate->factory()->empty_string(); -} - int FunctionLiteral::start_position() const { return scope()->start_position(); } @@ -520,7 +516,8 @@ void ObjectLiteral::BuildBoilerplateDescription(Isolate* isolate) { Handle key = key_literal->AsArrayIndex(&element_index) ? isolate->factory()->NewNumberFromUint(element_index) - : Handle::cast(key_literal->AsRawPropertyName()->string()); + : Handle::cast( + key_literal->AsRawPropertyName()->string().get()); Handle value = GetBoilerplateValue(property->value(), isolate); @@ -703,11 +700,11 @@ Handle GetTemplateObject::GetOrBuildDescription( bool raw_and_cooked_match = true; for (int i = 0; i < raw_strings->length(); ++i) { if (this->cooked_strings()->at(i) == nullptr || - *this->raw_strings()->at(i)->string() != - *this->cooked_strings()->at(i)->string()) { + *this->raw_strings()->at(i)->string().get() != + *this->cooked_strings()->at(i)->string().get()) { raw_and_cooked_match = false; } - raw_strings->set(i, *this->raw_strings()->at(i)->string()); + raw_strings->set(i, *this->raw_strings()->at(i)->string().get()); } Handle cooked_strings = raw_strings; if (!raw_and_cooked_match) { @@ -715,7 +712,8 @@ Handle GetTemplateObject::GetOrBuildDescription( this->cooked_strings()->length(), AllocationType::kOld); for (int i = 0; i < cooked_strings->length(); ++i) { if (this->cooked_strings()->at(i) != nullptr) { - cooked_strings->set(i, *this->cooked_strings()->at(i)->string()); + cooked_strings->set( + i, *this->cooked_strings()->at(i)->string().get()); } else { cooked_strings->set(i, ReadOnlyRoots(isolate).undefined_value()); } @@ -900,7 +898,7 @@ Handle Literal::BuildValue(Isolate* isolate) const { case kHeapNumber: return isolate->factory()->NewNumber(number_); case kString: - return string_->string(); + return string_->string().get(); case kSymbol: return isolate->factory()->home_object_symbol(); case kBoolean: diff --git a/src/ast/ast.h b/src/ast/ast.h index 5141b510b2..39a453ea6f 100644 --- a/src/ast/ast.h +++ b/src/ast/ast.h @@ -1176,7 +1176,7 @@ class MaterializedLiteral : public Expression { // Node for capturing a regexp literal. class RegExpLiteral final : public MaterializedLiteral { public: - Handle pattern() const { return pattern_->string(); } + Handle pattern() const { return pattern_->string().get(); } const AstRawString* raw_pattern() const { return pattern_; } int flags() const { return flags_; } @@ -1493,7 +1493,7 @@ class VariableProxy final : public Expression { public: bool IsValidReferenceExpression() const { return !is_new_target(); } - Handle name() const { return raw_name()->string(); } + Handle name() const { return raw_name()->string().get(); } const AstRawString* raw_name() const { return is_resolved() ? var_->raw_name() : raw_name_; } @@ -2205,9 +2205,9 @@ class FunctionLiteral final : public Expression { // Empty handle means that the function does not have a shared name (i.e. // the name will be set dynamically after creation of the function closure). MaybeHandle name() const { - return raw_name_ ? raw_name_->string() : MaybeHandle(); + return raw_name_ ? raw_name_->string().get() + : MaybeHandle(); } - Handle name(Isolate* isolate) const; bool has_shared_name() const { return raw_name_ != nullptr; } const AstConsString* raw_name() const { return raw_name_; } void set_raw_name(const AstConsString* name) { raw_name_ = name; } @@ -2270,7 +2270,7 @@ class FunctionLiteral final : public Expression { return inferred_name_; } if (raw_inferred_name_ != nullptr) { - return raw_inferred_name_->string(); + return raw_inferred_name_->string().get(); } UNREACHABLE(); } @@ -2555,7 +2555,7 @@ class ClassLiteral final : public Expression { class NativeFunctionLiteral final : public Expression { public: - Handle name() const { return name_->string(); } + Handle name() const { return name_->string().get(); } const AstRawString* raw_name() const { return name_; } v8::Extension* extension() const { return extension_; } diff --git a/src/ast/modules.cc b/src/ast/modules.cc index 9c122fca86..533a11524c 100644 --- a/src/ast/modules.cc +++ b/src/ast/modules.cc @@ -86,9 +86,10 @@ void SourceTextModuleDescriptor::AddStarExport( namespace { Handle ToStringOrUndefined(Isolate* isolate, const AstRawString* s) { - return (s == nullptr) ? Handle::cast( - isolate->factory()->undefined_value()) - : Handle::cast(s->string()); + return (s == nullptr) + ? Handle::cast( + isolate->factory()->undefined_value()) + : Handle::cast(s->string().get()); } } // namespace @@ -126,7 +127,7 @@ Handle SourceTextModuleDescriptor::SerializeRegularExports( Handle export_names = isolate->factory()->NewFixedArray(count); data[index + SourceTextModuleInfo::kRegularExportLocalNameOffset] = - it->second->local_name->string(); + it->second->local_name->string().get(); data[index + SourceTextModuleInfo::kRegularExportCellIndexOffset] = handle(Smi::FromInt(it->second->cell_index), isolate); data[index + SourceTextModuleInfo::kRegularExportExportNamesOffset] = @@ -136,7 +137,7 @@ Handle SourceTextModuleDescriptor::SerializeRegularExports( // Collect the export names. int i = 0; for (; it != next; ++it) { - export_names->set(i++, *it->second->export_name->string()); + export_names->set(i++, *it->second->export_name->string().get()); } DCHECK_EQ(i, count); diff --git a/src/ast/prettyprinter.cc b/src/ast/prettyprinter.cc index b0a1d1d212..4d8bd006c5 100644 --- a/src/ast/prettyprinter.cc +++ b/src/ast/prettyprinter.cc @@ -587,10 +587,9 @@ void CallPrinter::PrintLiteral(Handle value, bool quote) { void CallPrinter::PrintLiteral(const AstRawString* value, bool quote) { - PrintLiteral(value->string(), quote); + PrintLiteral(value->string().get(), quote); } - //----------------------------------------------------------------------------- diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc index 7f9e193201..b422e56c4f 100644 --- a/src/ast/scopes.cc +++ b/src/ast/scopes.cc @@ -835,7 +835,7 @@ Variable* Scope::LookupInScopeInfo(const AstRawString* name, Scope* cache) { DCHECK_NULL(cache->variables_.Lookup(name)); DisallowHeapAllocation no_gc; - String name_handle = *name->string(); + String name_handle = *name->string().get(); // The Scope is backed up by ScopeInfo. This means it cannot operate in a // heap-independent mode, and all strings must be internalized immediately. So // it's ok to get the Handle here. @@ -2655,7 +2655,7 @@ Variable* ClassScope::LookupPrivateNameInScopeInfo(const AstRawString* name) { DCHECK_NULL(LookupLocalPrivateName(name)); DisallowHeapAllocation no_gc; - String name_handle = *name->string(); + String name_handle = *name->string().get(); VariableMode mode; InitializationFlag init_flag; MaybeAssignedFlag maybe_assigned_flag; diff --git a/src/ast/variables.h b/src/ast/variables.h index db7c84d7c9..3a8ca8888f 100644 --- a/src/ast/variables.h +++ b/src/ast/variables.h @@ -57,7 +57,7 @@ class Variable final : public ZoneObject { // parameter initializers. void set_scope(Scope* scope) { scope_ = scope; } - Handle name() const { return name_->string(); } + Handle name() const { return name_->string().get(); } const AstRawString* raw_name() const { return name_; } VariableMode mode() const { return VariableModeField::decode(bit_field_); } void set_mode(VariableMode mode) { diff --git a/src/codegen/compiler.cc b/src/codegen/compiler.cc index 8d6a39f658..b73d35fecd 100644 --- a/src/codegen/compiler.cc +++ b/src/codegen/compiler.cc @@ -564,7 +564,7 @@ MaybeHandle GenerateUnoptimizedCodeForToplevel( Isolate* isolate, Handle