From 06f05ec23178033faff311874d5989d5c9090110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marja=20H=C3=B6ltt=C3=A4?= Date: Mon, 29 May 2017 17:00:17 +0200 Subject: [PATCH] [parser] Skipping inner funcs: make more functions skippable. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Enable aggressive lazy inner funcs (make non-declaration funcs lazy, ie let f = function() { ... } when --experimental-preparser-scope-analysis is on. - Turn on variable tracking for lazy top level functions: this makes their inner functions skippable. - Test fix for an testing bug uncovered by this work: when restoring the data for the relevant scope, don't assume it's the outermost scope for which we have data. - Fix: if we abort lazy parsing a function, we shouldn't produce any data for it. BUG=v8:5516 Change-Id: I0606fbabb5886dc57dbb53ab5f3fb894ff5d032e Reviewed-on: https://chromium-review.googlesource.com/518165 Reviewed-by: Daniel Vogelheim Commit-Queue: Marja Hölttä Cr-Commit-Position: refs/heads/master@{#45615} --- src/ast/scopes.cc | 7 +-- src/flag-definitions.h | 3 +- src/parsing/preparsed-scope-data.cc | 2 +- src/parsing/preparser.cc | 5 +- test/cctest/parsing/test-preparser.cc | 4 +- test/mjsunit/skipping-inner-functions.js | 58 ++++++++++++++++++++++++ 6 files changed, 70 insertions(+), 9 deletions(-) diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc index cfb32c4597..f7c69d9522 100644 --- a/src/ast/scopes.cc +++ b/src/ast/scopes.cc @@ -1530,8 +1530,10 @@ void DeclarationScope::AnalyzePartially( PreParsedScopeData* preparsed_scope_data) { DCHECK(!force_eager_compilation_); VariableProxy* unresolved = nullptr; + bool need_preparsed_scope_data = FLAG_experimental_preparser_scope_analysis && + preparsed_scope_data->Producing(); - if (!outer_scope_->is_script_scope()) { + if (!outer_scope_->is_script_scope() || need_preparsed_scope_data) { // Try to resolve unresolved variables for this Scope and migrate those // which cannot be resolved inside. It doesn't make sense to try to resolve // them in the outer Scopes here, because they are incomplete. @@ -1549,8 +1551,7 @@ void DeclarationScope::AnalyzePartially( arguments_ = nullptr; } - if (FLAG_experimental_preparser_scope_analysis && - preparsed_scope_data->Producing()) { + if (need_preparsed_scope_data) { // Store the information needed for allocating the locals of this scope // and its inner scopes. preparsed_scope_data->SaveData(this); diff --git a/src/flag-definitions.h b/src/flag-definitions.h index c860f1dbba..24d97c9282 100644 --- a/src/flag-definitions.h +++ b/src/flag-definitions.h @@ -965,7 +965,8 @@ DEFINE_BOOL(aggressive_lazy_inner_functions, false, DEFINE_IMPLICATION(aggressive_lazy_inner_functions, lazy_inner_functions) DEFINE_BOOL(experimental_preparser_scope_analysis, false, "perform scope analysis for preparsed inner functions") -DEFINE_IMPLICATION(experimental_preparser_scope_analysis, lazy_inner_functions) +DEFINE_IMPLICATION(experimental_preparser_scope_analysis, + aggressive_lazy_inner_functions) // simulator-arm.cc, simulator-arm64.cc and simulator-mips.cc DEFINE_BOOL(trace_sim, false, "Trace simulator execution") diff --git a/src/parsing/preparsed-scope-data.cc b/src/parsing/preparsed-scope-data.cc index 76e9b967d5..6a677fe83a 100644 --- a/src/parsing/preparsed-scope-data.cc +++ b/src/parsing/preparsed-scope-data.cc @@ -142,7 +142,7 @@ void PreParsedScopeData::RestoreData(Scope* scope, uint32_t* index_ptr) const { DCHECK_EQ(data.uses_super_property, scope->AsDeclarationScope()->uses_super_property()); uint32_t index_from_data = 0; - FindFunctionData(scope->start_position(), &index_from_data); + DCHECK(FindFunctionData(scope->start_position(), &index_from_data)); DCHECK_EQ(index_from_data, index); } #endif diff --git a/src/parsing/preparser.cc b/src/parsing/preparser.cc index c408af88c9..4fb32611b1 100644 --- a/src/parsing/preparser.cc +++ b/src/parsing/preparser.cc @@ -137,7 +137,8 @@ PreParser::PreParseResult PreParser::PreParseFunction( parsing_module_ = parsing_module; use_counts_ = use_counts; DCHECK(!track_unresolved_variables_); - track_unresolved_variables_ = is_inner_function; + track_unresolved_variables_ = + is_inner_function || FLAG_experimental_preparser_scope_analysis; #ifdef DEBUG function_scope->set_is_being_lazily_parsed(true); #endif @@ -213,7 +214,7 @@ PreParser::PreParseResult PreParser::PreParseFunction( function_scope->DeclareArguments(ast_value_factory()); if (FLAG_experimental_preparser_scope_analysis && - preparsed_scope_data_ != nullptr) { + preparsed_scope_data_ != nullptr && result != kLazyParsingAborted) { // We're not going to skip this function, but it might contain skippable // functions inside it. preparsed_scope_data_->AddFunction( diff --git a/test/cctest/parsing/test-preparser.cc b/test/cctest/parsing/test-preparser.cc index c28195cb01..0a6394a8f9 100644 --- a/test/cctest/parsing/test-preparser.cc +++ b/test/cctest/parsing/test-preparser.cc @@ -770,8 +770,8 @@ TEST(PreParserScopeAnalysis) { CHECK_NULL(unallocated_scope->sibling()); CHECK(unallocated_scope->is_function_scope()); - uint32_t index = 0; - lazy_info.preparsed_scope_data()->RestoreData(unallocated_scope, &index); + lazy_info.preparsed_scope_data()->RestoreData( + unallocated_scope->AsDeclarationScope()); i::ScopeTestHelper::AllocateWithoutVariableResolution(unallocated_scope); i::ScopeTestHelper::CompareScopes( diff --git a/test/mjsunit/skipping-inner-functions.js b/test/mjsunit/skipping-inner-functions.js index 1c5538567f..c95d33c9e9 100644 --- a/test/mjsunit/skipping-inner-functions.js +++ b/test/mjsunit/skipping-inner-functions.js @@ -35,3 +35,61 @@ lazy(9)(); assertEquals(19, result); })(); + +(function TestCtxtAllocatingNonSimpleParams1() { + var result = 0; + + function lazy([other_param1, ctxt_alloc_param, other_param2]) { + function skip_me() { + result = ctxt_alloc_param; + } + return skip_me; + } + // Test that parameters and variables of the outer function get context + // allocated even if we skip the inner function. + lazy([30, 29, 28])(); + assertEquals(29, result); +})(); + +(function TestCtxtAllocatingNonSimpleParams2() { + var result = 0; + + function lazy({a: other_param1, b: ctxt_alloc_param, c: other_param2}) { + function skip_me() { + result = ctxt_alloc_param; + } + return skip_me; + } + // Test that parameters and variables of the outer function get context + // allocated even if we skip the inner function. + lazy({a: 31, b: 32, c: 33})(); + assertEquals(32, result); +})(); + +(function TestCtxtAllocatingNonSimpleParams3() { + var result = 0; + + function lazy(...ctxt_alloc_param) { + function skip_me() { + result = ctxt_alloc_param; + } + return skip_me; + } + // Test that parameters and variables of the outer function get context + // allocated even if we skip the inner function. + lazy(34, 35)(); + assertEquals([34, 35], result); +})(); + +// Skippable top level functions. +let result = 0; +function lazy_top_level(ctxt_alloc_param) { + let ctxt_alloc_var = 24; + function skip_me() { + result = ctxt_alloc_param + ctxt_alloc_var; + } + skip_me(); +} + +lazy_top_level(10); +assertEquals(34, result);