From 5883bf21253efaa591420bfcdc2f5a5eb6b68ce4 Mon Sep 17 00:00:00 2001 From: rmcilroy Date: Tue, 17 Jan 2017 02:20:47 -0800 Subject: [PATCH] [Parser] Introduce AstStringConstants to share constants across AstValueFactory Creates an AstStringConstants container which pre-initializes the string constants used by AstValueFactory. This ensures that all AstValueFactories will produce the same AstValue objects for constants, and so they can be used by the BytecodeGenerator without having to pass the AstValueFactory to it, enabling construction off-thread. BUG=v8:5203 Review-Url: https://codereview.chromium.org/2630343002 Cr-Original-Commit-Position: refs/heads/master@{#42381} Committed: https://chromium.googlesource.com/v8/v8/+/d611496b8ed30af787d8668f96b400617c858508 Review-Url: https://codereview.chromium.org/2630343002 Cr-Commit-Position: refs/heads/master@{#42394} --- src/ast/ast-value-factory.cc | 1 - src/ast/ast-value-factory.h | 84 ++++++++++++++++++++------- src/heap-symbols.h | 18 ++++++ src/interpreter/bytecode-generator.cc | 11 ++-- src/isolate.cc | 9 +++ src/isolate.h | 7 +++ src/parsing/parser.cc | 3 +- test/cctest/asmjs/test-asm-typer.cc | 3 +- test/cctest/test-ast.cc | 7 ++- test/cctest/test-parsing.cc | 39 ++++++++----- 10 files changed, 137 insertions(+), 45 deletions(-) diff --git a/src/ast/ast-value-factory.cc b/src/ast/ast-value-factory.cc index 8ea70872cc..4add57955f 100644 --- a/src/ast/ast-value-factory.cc +++ b/src/ast/ast-value-factory.cc @@ -221,7 +221,6 @@ void AstValue::Internalize(Isolate* isolate) { } } - AstRawString* AstValueFactory::GetOneByteStringInternal( Vector literal) { if (literal.length() == 1 && IsInRange(literal[0], 'a', 'z')) { diff --git a/src/ast/ast-value-factory.h b/src/ast/ast-value-factory.h index 8b28c5e4bc..fd9ed71167 100644 --- a/src/ast/ast-value-factory.h +++ b/src/ast/ast-value-factory.h @@ -111,8 +111,9 @@ class AstRawString final : public AstString { } private: - friend class AstValueFactory; friend class AstRawStringInternalizationKey; + friend class AstStringConstants; + friend class AstValueFactory; AstRawString(bool is_one_byte, const Vector& literal_bytes, uint32_t hash) @@ -292,7 +293,6 @@ class AstValue : public ZoneObject { }; }; - // For generating constants. #define STRING_CONSTANTS(F) \ F(anonymous_function, "(anonymous function)") \ @@ -332,6 +332,45 @@ class AstValue : public ZoneObject { F(use_strict, "use strict") \ F(value, "value") +class AstStringConstants final { + public: + AstStringConstants(Isolate* isolate, uint32_t hash_seed) + : zone_(isolate->allocator(), ZONE_NAME), hash_seed_(hash_seed) { + DCHECK(ThreadId::Current().Equals(isolate->thread_id())); +#define F(name, str) \ + { \ + const char* data = str; \ + Vector literal(reinterpret_cast(data), \ + static_cast(strlen(data))); \ + uint32_t hash = StringHasher::HashSequentialString( \ + literal.start(), literal.length(), hash_seed_); \ + name##_string_ = new (&zone_) AstRawString(true, literal, hash); \ + /* The Handle returned by the factory is located on the roots */ \ + /* array, not on the temporary HandleScope, so this is safe. */ \ + name##_string_->set_string(isolate->factory()->name##_string()); \ + } + STRING_CONSTANTS(F) +#undef F + } + +#define F(name, str) \ + AstRawString* name##_string() { return name##_string_; } + STRING_CONSTANTS(F) +#undef F + + uint32_t hash_seed() const { return hash_seed_; } + + private: + Zone zone_; + uint32_t hash_seed_; + +#define F(name, str) AstRawString* name##_string_; + STRING_CONSTANTS(F) +#undef F + + DISALLOW_COPY_AND_ASSIGN(AstStringConstants); +}; + #define OTHER_CONSTANTS(F) \ F(true_value) \ F(false_value) \ @@ -341,23 +380,24 @@ class AstValue : public ZoneObject { class AstValueFactory { public: - AstValueFactory(Zone* zone, uint32_t hash_seed) + AstValueFactory(Zone* zone, AstStringConstants* string_constants, + uint32_t hash_seed) : string_table_(AstRawStringCompare), values_(nullptr), strings_(nullptr), strings_end_(&strings_), + string_constants_(string_constants), zone_(zone), hash_seed_(hash_seed) { -#define F(name, str) name##_string_ = NULL; - STRING_CONSTANTS(F) -#undef F -#define F(name) name##_ = NULL; +#define F(name) name##_ = nullptr; OTHER_CONSTANTS(F) #undef F + DCHECK_EQ(hash_seed, string_constants->hash_seed()); std::fill(smis_, smis_ + arraysize(smis_), nullptr); std::fill(one_character_strings_, one_character_strings_ + arraysize(one_character_strings_), nullptr); + InitializeStringConstants(); } Zone* zone() const { return zone_; } @@ -378,15 +418,9 @@ class AstValueFactory { void Internalize(Isolate* isolate); -#define F(name, str) \ - const AstRawString* name##_string() { \ - if (name##_string_ == NULL) { \ - const char* data = str; \ - name##_string_ = GetOneByteString( \ - Vector(reinterpret_cast(data), \ - static_cast(strlen(data)))); \ - } \ - return name##_string_; \ +#define F(name, str) \ + const AstRawString* name##_string() { \ + return string_constants_->name##_string(); \ } STRING_CONSTANTS(F) #undef F @@ -427,6 +461,17 @@ class AstValueFactory { AstRawString* GetString(uint32_t hash, bool is_one_byte, Vector literal_bytes); + void InitializeStringConstants() { +#define F(name, str) \ + AstRawString* raw_string_##name = string_constants_->name##_string(); \ + base::HashMap::Entry* entry_##name = string_table_.LookupOrInsert( \ + raw_string_##name, raw_string_##name->hash()); \ + DCHECK(entry_##name->value == nullptr); \ + entry_##name->value = reinterpret_cast(1); + STRING_CONSTANTS(F) +#undef F + } + static bool AstRawStringCompare(void* a, void* b); // All strings are copied here, one after another (no NULLs inbetween). @@ -440,6 +485,9 @@ class AstValueFactory { AstString* strings_; AstString** strings_end_; + // Holds constant string values which are shared across the isolate. + AstStringConstants* string_constants_; + // Caches for faster access: small numbers, one character lowercase strings // (for minified code). AstValue* smis_[kMaxCachedSmi + 1]; @@ -449,10 +497,6 @@ class AstValueFactory { uint32_t hash_seed_; -#define F(name, str) const AstRawString* name##_string_; - STRING_CONSTANTS(F) -#undef F - #define F(name) AstValue* name##_; OTHER_CONSTANTS(F) #undef F diff --git a/src/heap-symbols.h b/src/heap-symbols.h index e99cf2afeb..cf8739167e 100644 --- a/src/heap-symbols.h +++ b/src/heap-symbols.h @@ -6,6 +6,7 @@ #define V8_HEAP_SYMBOLS_H_ #define INTERNALIZED_STRING_LIST(V) \ + V(anonymous_function_string, "(anonymous function)") \ V(anonymous_string, "anonymous") \ V(apply_string, "apply") \ V(arguments_string, "arguments") \ @@ -14,6 +15,8 @@ V(Array_string, "Array") \ V(ArrayIterator_string, "Array Iterator") \ V(assign_string, "assign") \ + V(async_string, "async") \ + V(await_string, "await") \ V(array_to_string, "[object Array]") \ V(boolean_to_string, "[object Boolean]") \ V(date_to_string, "[object Date]") \ @@ -57,7 +60,12 @@ V(did_handle_string, "didHandle") \ V(display_name_string, "displayName") \ V(done_string, "done") \ + V(dot_catch_string, ".catch") \ + V(dot_for_string, ".for") \ + V(dot_generator_object_string, ".generator_object") \ + V(dot_iterator_string, ".iterator") \ V(dot_result_string, ".result") \ + V(dot_switch_tag_string, ".switch_tag") \ V(dot_string, ".") \ V(exec_string, "exec") \ V(entries_string, "entries") \ @@ -78,6 +86,7 @@ V(getOwnPropertyDescriptors_string, "getOwnPropertyDescriptors") \ V(getPrototypeOf_string, "getPrototypeOf") \ V(get_string, "get") \ + V(get_space_string, "get ") \ V(global_string, "global") \ V(has_string, "has") \ V(hour_string, "hour") \ @@ -100,6 +109,7 @@ V(keys_string, "keys") \ V(lastIndex_string, "lastIndex") \ V(length_string, "length") \ + V(let_string, "let") \ V(line_string, "line") \ V(literal_string, "literal") \ V(Map_string, "Map") \ @@ -111,7 +121,9 @@ V(month_string, "month") \ V(multiline_string, "multiline") \ V(name_string, "name") \ + V(native_string, "native") \ V(nan_string, "NaN") \ + V(new_target_string, ".new.target") \ V(next_string, "next") \ V(not_equal, "not-equal") \ V(null_string, "null") \ @@ -136,9 +148,11 @@ V(RegExp_string, "RegExp") \ V(reject_string, "reject") \ V(resolve_string, "resolve") \ + V(return_string, "return") \ V(script_string, "script") \ V(second_string, "second") \ V(setPrototypeOf_string, "setPrototypeOf") \ + V(set_space_string, "set ") \ V(set_string, "set") \ V(Set_string, "Set") \ V(source_mapping_url_string, "source_mapping_url") \ @@ -147,6 +161,7 @@ V(source_url_string, "source_url") \ V(stack_string, "stack") \ V(stackTraceLimit_string, "stackTraceLimit") \ + V(star_default_star_string, "*default*") \ V(sticky_string, "sticky") \ V(strict_compare_ic_string, "===") \ V(string_string, "string") \ @@ -156,6 +171,7 @@ V(symbol_species_string, "[Symbol.species]") \ V(SyntaxError_string, "SyntaxError") \ V(then_string, "then") \ + V(this_function_string, ".this_function") \ V(this_string, "this") \ V(throw_string, "throw") \ V(timed_out, "timed-out") \ @@ -177,6 +193,8 @@ V(undefined_string, "undefined") \ V(undefined_to_string, "[object Undefined]") \ V(unicode_string, "unicode") \ + V(use_asm_string, "use asm") \ + V(use_strict_string, "use strict") \ V(URIError_string, "URIError") \ V(valueOf_string, "valueOf") \ V(values_string, "values") \ diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc index 84cd94393e..2de429dbcf 100644 --- a/src/interpreter/bytecode-generator.cc +++ b/src/interpreter/bytecode-generator.cc @@ -587,13 +587,10 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info) loop_depth_(0), home_object_symbol_(info->isolate()->factory()->home_object_symbol()), iterator_symbol_(info->isolate()->factory()->iterator_symbol()), - empty_fixed_array_(info->isolate()->factory()->empty_fixed_array()) { - AstValueFactory* ast_value_factory = info->parse_info()->ast_value_factory(); - const AstRawString* prototype_string = ast_value_factory->prototype_string(); - ast_value_factory->Internalize(info->isolate()); - prototype_string_ = prototype_string->string(); - undefined_string_ = ast_value_factory->undefined_string(); -} + prototype_string_(info->isolate()->factory()->prototype_string()), + empty_fixed_array_(info->isolate()->factory()->empty_fixed_array()), + undefined_string_( + info->isolate()->ast_string_constants()->undefined_string()) {} Handle BytecodeGenerator::FinalizeBytecode(Isolate* isolate) { AllocateDeferredConstants(isolate); diff --git a/src/isolate.cc b/src/isolate.cc index 0c0af8472a..a77a871fed 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -9,6 +9,7 @@ #include // NOLINT(readability/streams) #include +#include "src/ast/ast-value-factory.h" #include "src/ast/context-slot-cache.h" #include "src/base/hashmap.h" #include "src/base/platform/platform.h" @@ -2375,6 +2376,9 @@ void Isolate::Deinit() { delete interpreter_; interpreter_ = NULL; + delete ast_string_constants_; + ast_string_constants_ = nullptr; + delete cpu_profiler_; cpu_profiler_ = NULL; @@ -2698,6 +2702,11 @@ bool Isolate::Init(Deserializer* des) { time_millis_at_init_ = heap_.MonotonicallyIncreasingTimeInMs(); + { + HandleScope scope(this); + ast_string_constants_ = new AstStringConstants(this, heap()->HashSeed()); + } + if (!create_heap_objects) { // Now that the heap is consistent, it's OK to generate the code for the // deopt entry table that might have been referred to by optimized code in diff --git a/src/isolate.h b/src/isolate.h index bf6b4f2da0..64fadbb24f 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -35,6 +35,7 @@ namespace internal { class AccessCompilerData; class AddressToIndexHashMap; +class AstStringConstants; class BasicBlockProfiler; class Bootstrapper; class CancelableTaskManager; @@ -1155,6 +1156,10 @@ class Isolate { return cancelable_task_manager_; } + AstStringConstants* ast_string_constants() const { + return ast_string_constants_; + } + interpreter::Interpreter* interpreter() const { return interpreter_; } AccountingAllocator* allocator() { return allocator_; } @@ -1397,6 +1402,8 @@ class Isolate { std::unique_ptr code_event_dispatcher_; FunctionEntryHook function_entry_hook_; + AstStringConstants* ast_string_constants_; + interpreter::Interpreter* interpreter_; CompilerDispatcher* compiler_dispatcher_; diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc index 1979f054c1..814acb4c1e 100644 --- a/src/parsing/parser.cc +++ b/src/parsing/parser.cc @@ -559,7 +559,8 @@ Parser::Parser(ParseInfo* info) } if (info->ast_value_factory() == NULL) { // info takes ownership of AstValueFactory. - info->set_ast_value_factory(new AstValueFactory(zone(), info->hash_seed())); + info->set_ast_value_factory(new AstValueFactory( + zone(), info->isolate()->ast_string_constants(), info->hash_seed())); info->set_ast_value_factory_owned(); ast_value_factory_ = info->ast_value_factory(); ast_node_factory_.set_ast_value_factory(ast_value_factory_); diff --git a/test/cctest/asmjs/test-asm-typer.cc b/test/cctest/asmjs/test-asm-typer.cc index a0db745e97..892c968d1d 100644 --- a/test/cctest/asmjs/test-asm-typer.cc +++ b/test/cctest/asmjs/test-asm-typer.cc @@ -47,7 +47,8 @@ class AsmTyperHarnessBuilder { handles_(), zone_(handles_.main_zone()), isolate_(CcTest::i_isolate()), - ast_value_factory_(zone_, isolate_->heap()->HashSeed()), + ast_value_factory_(zone_, isolate_->ast_string_constants(), + isolate_->heap()->HashSeed()), factory_(isolate_->factory()), source_code_( factory_->NewStringFromUtf8(CStrVector(source)).ToHandleChecked()), diff --git a/test/cctest/test-ast.cc b/test/cctest/test-ast.cc index 99cbb950ca..c027e88a52 100644 --- a/test/cctest/test-ast.cc +++ b/test/cctest/test-ast.cc @@ -30,6 +30,7 @@ #include "src/v8.h" #include "src/ast/ast.h" +#include "src/isolate.h" #include "src/objects-inl.h" #include "src/zone/accounting-allocator.h" #include "test/cctest/cctest.h" @@ -37,12 +38,16 @@ using namespace v8::internal; TEST(List) { + v8::V8::Initialize(); + Isolate* isolate = CcTest::i_isolate(); + List* list = new List(0); CHECK_EQ(0, list->length()); v8::internal::AccountingAllocator allocator; Zone zone(&allocator, ZONE_NAME); - AstValueFactory value_factory(&zone, 0); + AstValueFactory value_factory(&zone, isolate->ast_string_constants(), + isolate->heap()->HashSeed()); AstNodeFactory factory(&value_factory); AstNode* node = factory.NewEmptyStatement(kNoSourcePosition); list->Add(node); diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index 84869ca0cc..d668dd377b 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -175,7 +175,8 @@ TEST(ScanHTMLEndComments) { scanner.Initialize(stream.get()); i::Zone zone(CcTest::i_isolate()->allocator(), ZONE_NAME); i::AstValueFactory ast_value_factory( - &zone, CcTest::i_isolate()->heap()->HashSeed()); + &zone, CcTest::i_isolate()->ast_string_constants(), + CcTest::i_isolate()->heap()->HashSeed()); i::PendingCompilationErrorHandler pending_error_handler; i::PreParser preparser( &zone, &scanner, stack_limit, &ast_value_factory, @@ -193,7 +194,8 @@ TEST(ScanHTMLEndComments) { scanner.Initialize(stream.get()); i::Zone zone(CcTest::i_isolate()->allocator(), ZONE_NAME); i::AstValueFactory ast_value_factory( - &zone, CcTest::i_isolate()->heap()->HashSeed()); + &zone, CcTest::i_isolate()->ast_string_constants(), + CcTest::i_isolate()->heap()->HashSeed()); i::PendingCompilationErrorHandler pending_error_handler; i::PreParser preparser( &zone, &scanner, stack_limit, &ast_value_factory, @@ -364,7 +366,8 @@ TEST(StandAlonePreParser) { i::Zone zone(CcTest::i_isolate()->allocator(), ZONE_NAME); i::AstValueFactory ast_value_factory( - &zone, CcTest::i_isolate()->heap()->HashSeed()); + &zone, CcTest::i_isolate()->ast_string_constants(), + CcTest::i_isolate()->heap()->HashSeed()); i::PendingCompilationErrorHandler pending_error_handler; i::PreParser preparser( &zone, &scanner, stack_limit, &ast_value_factory, @@ -400,7 +403,8 @@ TEST(StandAlonePreParserNoNatives) { // Preparser defaults to disallowing natives syntax. i::Zone zone(CcTest::i_isolate()->allocator(), ZONE_NAME); i::AstValueFactory ast_value_factory( - &zone, CcTest::i_isolate()->heap()->HashSeed()); + &zone, CcTest::i_isolate()->ast_string_constants(), + CcTest::i_isolate()->heap()->HashSeed()); i::PendingCompilationErrorHandler pending_error_handler; i::PreParser preparser(&zone, &scanner, stack_limit, &ast_value_factory, &pending_error_handler, @@ -466,8 +470,9 @@ TEST(RegressChromium62639) { i::Scanner scanner(CcTest::i_isolate()->unicode_cache()); scanner.Initialize(stream.get()); i::Zone zone(CcTest::i_isolate()->allocator(), ZONE_NAME); - i::AstValueFactory ast_value_factory(&zone, - CcTest::i_isolate()->heap()->HashSeed()); + i::AstValueFactory ast_value_factory( + &zone, CcTest::i_isolate()->ast_string_constants(), + CcTest::i_isolate()->heap()->HashSeed()); i::PendingCompilationErrorHandler pending_error_handler; i::PreParser preparser(&zone, &scanner, CcTest::i_isolate()->stack_guard()->real_climit(), @@ -541,8 +546,9 @@ TEST(PreParseOverflow) { scanner.Initialize(stream.get()); i::Zone zone(CcTest::i_isolate()->allocator(), ZONE_NAME); - i::AstValueFactory ast_value_factory(&zone, - CcTest::i_isolate()->heap()->HashSeed()); + i::AstValueFactory ast_value_factory( + &zone, CcTest::i_isolate()->ast_string_constants(), + CcTest::i_isolate()->heap()->HashSeed()); i::PendingCompilationErrorHandler pending_error_handler; i::PreParser preparser(&zone, &scanner, stack_limit, &ast_value_factory, &pending_error_handler, @@ -642,8 +648,9 @@ void TestScanRegExp(const char* re_source, const char* expected) { CHECK(scanner.ScanRegExpPattern()); scanner.Next(); // Current token is now the regexp literal. i::Zone zone(CcTest::i_isolate()->allocator(), ZONE_NAME); - i::AstValueFactory ast_value_factory(&zone, - CcTest::i_isolate()->heap()->HashSeed()); + i::AstValueFactory ast_value_factory( + &zone, CcTest::i_isolate()->ast_string_constants(), + CcTest::i_isolate()->heap()->HashSeed()); const i::AstRawString* current_symbol = scanner.CurrentSymbol(&ast_value_factory); ast_value_factory.Internalize(CcTest::i_isolate()); @@ -1330,7 +1337,8 @@ void TestParserSyncWithFlags(i::Handle source, i::ScannerStream::For(source)); i::Zone zone(CcTest::i_isolate()->allocator(), ZONE_NAME); i::AstValueFactory ast_value_factory( - &zone, CcTest::i_isolate()->heap()->HashSeed()); + &zone, CcTest::i_isolate()->ast_string_constants(), + CcTest::i_isolate()->heap()->HashSeed()); i::PreParser preparser(&zone, &scanner, stack_limit, &ast_value_factory, &pending_error_handler, isolate->counters()->runtime_call_stats()); @@ -3164,7 +3172,8 @@ TEST(SerializationOfMaybeAssignmentFlag) { i::Handle o = v8::Utils::OpenHandle(*v); i::Handle f = i::Handle::cast(o); i::Context* context = f->context(); - i::AstValueFactory avf(&zone, isolate->heap()->HashSeed()); + i::AstValueFactory avf(&zone, isolate->ast_string_constants(), + isolate->heap()->HashSeed()); const i::AstRawString* name = avf.GetOneByteString("result"); avf.Internalize(isolate); i::Handle str = name->string(); @@ -3213,7 +3222,8 @@ TEST(IfArgumentsArrayAccessedThenParametersMaybeAssigned) { i::Handle o = v8::Utils::OpenHandle(*v); i::Handle f = i::Handle::cast(o); i::Context* context = f->context(); - i::AstValueFactory avf(&zone, isolate->heap()->HashSeed()); + i::AstValueFactory avf(&zone, isolate->ast_string_constants(), + isolate->heap()->HashSeed()); const i::AstRawString* name_x = avf.GetOneByteString("x"); avf.Internalize(isolate); @@ -3509,7 +3519,8 @@ namespace { i::Scope* DeserializeFunctionScope(i::Isolate* isolate, i::Zone* zone, i::Handle m, const char* name) { - i::AstValueFactory avf(zone, isolate->heap()->HashSeed()); + i::AstValueFactory avf(zone, isolate->ast_string_constants(), + isolate->heap()->HashSeed()); i::Handle f = i::Handle::cast( i::JSReceiver::GetProperty(isolate, m, name).ToHandleChecked()); i::DeclarationScope* script_scope =