Commit 3e47cb87 authored by Mythri's avatar Mythri Committed by Commit Bot

[Turbofan] Use bytecode size for inlining heuristics.

Inlining heuristics in Turbofan used ast node count. Bytecode size
is a better approximation of the size of the graph than the
ast node count. This cl changes the heuristics to use the bytecode
size instead. Also removing the ast_node_count filed in the shared
function info. It was used only for the inlining heuristics.

Also removed the max_inlined_source_size flag which is no longer used.

Bug: 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I8a2d2509c8e8d2779b33b817bb217de203d54ec3
Reviewed-on: https://chromium-review.googlesource.com/570055
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46771}
parent ee15703e
......@@ -10437,9 +10437,8 @@ static void SetFlagsFromString(const char* flags) {
void Testing::PrepareStressRun(int run) {
static const char* kLazyOptimizations =
"--prepare-always-opt "
"--max-inlined-source-size=999999 "
"--max-inlined-nodes=999999 "
"--max-inlined-nodes-cumulative=999999 "
"--max-inlined-bytecode-size=999999 "
"--max-inlined-bytecode-size-cumulative=999999 "
"--noalways-opt";
static const char* kForcedOptimizations = "--always-opt";
......
This diff is collapsed.
......@@ -156,16 +156,12 @@ class FeedbackSlotCache {
class AstProperties final BASE_EMBEDDED {
public:
explicit AstProperties(Zone* zone) : node_count_(0), spec_(zone) {}
int node_count() { return node_count_; }
void add_node_count(int count) { node_count_ += count; }
explicit AstProperties(Zone* zone) : spec_(zone) {}
const FeedbackVectorSpec* get_spec() const { return &spec_; }
FeedbackVectorSpec* get_spec() { return &spec_; }
private:
int node_count_;
FeedbackVectorSpec spec_;
};
......@@ -2513,7 +2509,6 @@ class FunctionLiteral final : public Expression {
void set_ast_properties(AstProperties* ast_properties) {
ast_properties_ = *ast_properties;
}
int ast_node_count() { return ast_properties_.node_count(); }
const FeedbackVectorSpec* feedback_vector_spec() const {
return ast_properties_.get_spec();
}
......
......@@ -456,8 +456,7 @@ void CompilerDispatcherJob::PrepareToCompileOnMainThread() {
void CompilerDispatcherJob::Compile() {
DCHECK(status() == CompileJobStatus::kReadyToCompile);
COMPILER_DISPATCHER_TRACE_SCOPE_WITH_NUM(
tracer_, kCompile, parse_info_->literal()->ast_node_count());
COMPILER_DISPATCHER_TRACE_SCOPE(tracer_, kCompile);
if (trace_compiler_dispatcher_jobs_) {
PrintF("CompilerDispatcherJob[%p]: Compiling\n", static_cast<void*>(this));
}
......@@ -550,8 +549,7 @@ double CompilerDispatcherJob::EstimateRuntimeOfNextStepInMs() const {
return tracer_->EstimatePrepareToCompileInMs();
case CompileJobStatus::kReadyToCompile:
return tracer_->EstimateCompileInMs(
parse_info_->literal()->ast_node_count());
return tracer_->EstimateCompileInMs();
case CompileJobStatus::kCompiled:
return tracer_->EstimateFinalizeCompilingInMs();
......
......@@ -46,7 +46,7 @@ CompilerDispatcherTracer::Scope::~Scope() {
tracer_->RecordPrepareToCompile(elapsed);
break;
case ScopeID::kCompile:
tracer_->RecordCompile(elapsed, num_);
tracer_->RecordCompile(elapsed);
break;
case ScopeID::kFinalizeCompiling:
tracer_->RecordFinalizeCompiling(elapsed);
......@@ -111,10 +111,9 @@ void CompilerDispatcherTracer::RecordPrepareToCompile(double duration_ms) {
prepare_compile_events_.Push(duration_ms);
}
void CompilerDispatcherTracer::RecordCompile(double duration_ms,
size_t ast_size_in_bytes) {
void CompilerDispatcherTracer::RecordCompile(double duration_ms) {
base::LockGuard<base::Mutex> lock(&mutex_);
compile_events_.Push(std::make_pair(ast_size_in_bytes, duration_ms));
compile_events_.Push(duration_ms);
}
void CompilerDispatcherTracer::RecordFinalizeCompiling(double duration_ms) {
......@@ -147,10 +146,9 @@ double CompilerDispatcherTracer::EstimatePrepareToCompileInMs() const {
return Average(prepare_compile_events_);
}
double CompilerDispatcherTracer::EstimateCompileInMs(
size_t ast_size_in_bytes) const {
double CompilerDispatcherTracer::EstimateCompileInMs() const {
base::LockGuard<base::Mutex> lock(&mutex_);
return Estimate(compile_events_, ast_size_in_bytes);
return Average(compile_events_);
}
double CompilerDispatcherTracer::EstimateFinalizeCompilingInMs() const {
......@@ -166,7 +164,7 @@ void CompilerDispatcherTracer::DumpStatistics() const {
"finalize_compiling=%.2lfms\n",
EstimatePrepareToParseInMs(), EstimateParseInMs(1 * KB),
EstimateFinalizeParsingInMs(), EstimateAnalyzeInMs(),
EstimatePrepareToCompileInMs(), EstimateCompileInMs(1 * KB),
EstimatePrepareToCompileInMs(), EstimateCompileInMs(),
EstimateFinalizeCompilingInMs());
}
......
......@@ -65,7 +65,7 @@ class V8_EXPORT_PRIVATE CompilerDispatcherTracer {
void RecordFinalizeParsing(double duration_ms);
void RecordAnalyze(double duration_ms);
void RecordPrepareToCompile(double duration_ms);
void RecordCompile(double duration_ms, size_t ast_size_in_bytes);
void RecordCompile(double duration_ms);
void RecordFinalizeCompiling(double duration_ms);
double EstimatePrepareToParseInMs() const;
......@@ -73,7 +73,7 @@ class V8_EXPORT_PRIVATE CompilerDispatcherTracer {
double EstimateFinalizeParsingInMs() const;
double EstimateAnalyzeInMs() const;
double EstimatePrepareToCompileInMs() const;
double EstimateCompileInMs(size_t ast_size_in_bytes) const;
double EstimateCompileInMs() const;
double EstimateFinalizeCompilingInMs() const;
void DumpStatistics() const;
......@@ -89,7 +89,7 @@ class V8_EXPORT_PRIVATE CompilerDispatcherTracer {
base::RingBuffer<double> finalize_parsing_events_;
base::RingBuffer<double> analyze_events_;
base::RingBuffer<double> prepare_compile_events_;
base::RingBuffer<std::pair<size_t, double>> compile_events_;
base::RingBuffer<double> compile_events_;
base::RingBuffer<double> finalize_compiling_events_;
RuntimeCallStats* runtime_call_stats_;
......
......@@ -388,7 +388,6 @@ void SetSharedFunctionFlagsFromLiteral(FunctionLiteral* literal,
if (!shared_info->HasLength()) {
shared_info->set_length(literal->function_length());
}
shared_info->set_ast_node_count(literal->ast_node_count());
shared_info->set_has_duplicate_parameters(
literal->has_duplicate_parameters());
shared_info->SetExpectedNofPropertiesFromEstimate(literal);
......
......@@ -55,8 +55,13 @@ bool CanInlineFunction(Handle<SharedFunctionInfo> shared) {
// Only choose user code for inlining.
if (!shared->IsUserJavaScript()) return false;
// Quick check on the size of the AST to avoid parsing large candidate.
if (shared->ast_node_count() > FLAG_max_inlined_nodes) {
// If there is no bytecode array, it is either not compiled or it is compiled
// with full-codegen for asm.js pipeline. In either case we don't want to
// inline.
if (!shared->HasBytecodeArray()) return false;
// Quick check on the size of the bytecode to avoid inlining large functions.
if (shared->bytecode_array()->length() > FLAG_max_inlined_bytecode_size) {
return false;
}
......@@ -66,11 +71,12 @@ bool CanInlineFunction(Handle<SharedFunctionInfo> shared) {
}
bool IsSmallInlineFunction(Handle<SharedFunctionInfo> shared) {
// Don't forcibly inline functions that weren't compiled yet.
if (shared->ast_node_count() == 0) return false;
// Forcibly inline small functions.
if (shared->ast_node_count() <= FLAG_max_inlined_nodes_small) return true;
// Don't forcibly inline functions that weren't compiled yet.
if (shared->HasBytecodeArray() && shared->bytecode_array()->length() <=
FLAG_max_inlined_bytecode_size_small) {
return true;
}
return false;
}
......@@ -169,7 +175,8 @@ Reduction JSInliningHeuristic::Reduce(Node* node) {
// Forcibly inline small functions here. In the case of polymorphic inlining
// small_inline is set only when all functions are small.
if (small_inline && cumulative_count_ <= FLAG_max_inlined_nodes_absolute) {
if (small_inline &&
cumulative_count_ <= FLAG_max_inlined_bytecode_size_absolute) {
TRACE("Inlining small function(s) at call site #%d:%s\n", node->id(),
node->op()->mnemonic());
return InlineCandidate(candidate, true);
......@@ -189,7 +196,7 @@ void JSInliningHeuristic::Finalize() {
// on things that aren't called very often.
// TODO(bmeurer): Use std::priority_queue instead of std::set here.
while (!candidates_.empty()) {
if (cumulative_count_ > FLAG_max_inlined_nodes_cumulative) return;
if (cumulative_count_ > FLAG_max_inlined_bytecode_size_cumulative) return;
auto i = candidates_.begin();
Candidate candidate = *i;
candidates_.erase(i);
......@@ -212,7 +219,7 @@ Reduction JSInliningHeuristic::InlineCandidate(Candidate const& candidate,
: handle(candidate.functions[0]->shared());
Reduction const reduction = inliner_.ReduceJSCall(node);
if (reduction.Changed()) {
cumulative_count_ += shared->ast_node_count();
cumulative_count_ += shared->bytecode_array()->length();
}
return reduction;
}
......@@ -296,13 +303,13 @@ Reduction JSInliningHeuristic::InlineCandidate(Candidate const& candidate,
Node* node = calls[i];
if (force_inline ||
(candidate.can_inline_function[i] &&
cumulative_count_ < FLAG_max_inlined_nodes_cumulative)) {
cumulative_count_ < FLAG_max_inlined_bytecode_size_cumulative)) {
Reduction const reduction = inliner_.ReduceJSCall(node);
if (reduction.Changed()) {
// Killing the call node is not strictly necessary, but it is safer to
// make sure we do not resurrect the node.
node->Kill();
cumulative_count_ += function->shared()->ast_node_count();
cumulative_count_ += function->shared()->bytecode_array()->length();
}
}
}
......@@ -343,7 +350,7 @@ void JSInliningHeuristic::PrintCandidates() {
candidate.functions[i].is_null()
? candidate.shared_info
: handle(candidate.functions[i]->shared());
PrintF(" - size:%d, name: %s\n", shared->ast_node_count(),
PrintF(" - size:%d, name: %s\n", shared->bytecode_array()->length(),
shared->DebugName()->ToCString().get());
}
}
......
......@@ -2541,7 +2541,6 @@ Handle<SharedFunctionInfo> Factory::NewSharedFunctionInfo(
share->set_unique_id(isolate()->GetNextUniqueSharedFunctionInfoId());
#endif
share->set_profiler_ticks(0);
share->set_ast_node_count(0);
share->set_counters(0);
// Set integer fields (smi or int, depending on the architecture).
......
......@@ -342,17 +342,15 @@ DEFINE_BOOL(use_local_allocation_folding, false, "only fold in basic blocks")
DEFINE_BOOL(use_write_barrier_elimination, true,
"eliminate write barriers targeting allocations in optimized code")
DEFINE_INT(max_inlining_levels, 5, "maximum number of inlining levels")
DEFINE_INT(max_inlined_source_size, 600,
"maximum source size in bytes considered for a single inlining")
DEFINE_INT(max_inlined_nodes, 230,
"maximum number of AST nodes considered for a single inlining")
DEFINE_INT(max_inlined_nodes_absolute, 1600,
"maximum absolute number of AST nodes considered for inlining "
DEFINE_INT(max_inlined_bytecode_size, 500,
"maximum size of bytecode for a single inlining")
DEFINE_INT(max_inlined_bytecode_size_absolute, 4000,
"maximum absolute size of bytecode considered for inlining "
"(incl. small functions)")
DEFINE_INT(max_inlined_nodes_cumulative, 400,
"maximum cumulative number of AST nodes considered for inlining")
DEFINE_INT(max_inlined_nodes_small, 10,
"maximum number of AST nodes considered for small function inlining")
DEFINE_INT(max_inlined_bytecode_size_cumulative, 1000,
"maximum cumulative size of bytecode considered for inlining")
DEFINE_INT(max_inlined_bytecode_size_small, 30,
"maximum size of bytecode considered for small function inlining")
DEFINE_FLOAT(min_inlining_frequency, 0.15, "minimum frequency for inlining")
DEFINE_BOOL(fast_math, true, "faster (but maybe less accurate) math functions")
DEFINE_BOOL(trace_environment_liveness, false,
......
......@@ -1120,7 +1120,6 @@ void SharedFunctionInfo::SharedFunctionInfoPrint(std::ostream& os) { // NOLINT
os << "\n - formal_parameter_count = " << internal_formal_parameter_count();
os << "\n - expected_nof_properties = " << expected_nof_properties();
os << "\n - language_mode = " << language_mode();
os << "\n - ast_node_count = " << ast_node_count();
os << "\n - instance class name = ";
instance_class_name()->Print(os);
os << " - code = " << Brief(code());
......
......@@ -60,7 +60,6 @@ INT_ACCESSORS(SharedFunctionInfo, compiler_hints, kCompilerHintsOffset)
INT_ACCESSORS(SharedFunctionInfo, opt_count_and_bailout_reason,
kOptCountAndBailoutReasonOffset)
INT_ACCESSORS(SharedFunctionInfo, counters, kCountersOffset)
INT_ACCESSORS(SharedFunctionInfo, ast_node_count, kAstNodeCountOffset)
INT_ACCESSORS(SharedFunctionInfo, profiler_ticks, kProfilerTicksOffset)
bool SharedFunctionInfo::has_shared_name() const {
......
......@@ -278,8 +278,6 @@ class SharedFunctionInfo : public HeapObject {
// drive optimization.
DECL_INT_ACCESSORS(compiler_hints)
DECL_INT_ACCESSORS(ast_node_count)
DECL_INT_ACCESSORS(profiler_ticks)
// Inline cache age is used to infer whether the function survived a context
......@@ -468,7 +466,6 @@ class SharedFunctionInfo : public HeapObject {
V(kCompilerHintsOffset, kInt32Size) \
V(kOptCountAndBailoutReasonOffset, kInt32Size) \
V(kCountersOffset, kInt32Size) \
V(kAstNodeCountOffset, kInt32Size) \
V(kProfilerTicksOffset, kInt32Size) \
/* Total size. */ \
V(kSize, 0)
......
......@@ -2460,7 +2460,9 @@ TEST(TrackHeapAllocationsWithInlining) {
TEST(TrackHeapAllocationsWithoutInlining) {
i::FLAG_turbo_inlining = false;
i::FLAG_max_inlined_source_size = 0; // Disable inlining
// Disable inlining
i::FLAG_max_inlined_bytecode_size = 0;
i::FLAG_max_inlined_bytecode_size_small = 0;
v8::HandleScope scope(v8::Isolate::GetCurrent());
LocalContext env;
......
......@@ -26,8 +26,8 @@
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Flags: --allow-natives-syntax
// Flags: --max-inlined-source-size=999999 --max-inlined-nodes=999999
// Flags: --max-inlined-nodes-cumulative=999999
// Flags: --max-inlined-bytecode-size=999999
// Flags: --max-inlined-bytecode-size-cumulative=999999
// Test that huge constructors (more than 256 this assignments) are
// handled correctly.
......
// Copyright 2012 the V8 project authors. All rights reserved.
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions are
// met:
//
// * Redistributions of source code must retain the above copyright
// notice, this list of conditions and the following disclaimer.
// * Redistributions in binary form must reproduce the above
// copyright notice, this list of conditions and the following
// disclaimer in the documentation and/or other materials provided
// with the distribution.
// * Neither the name of Google Inc. nor the names of its
// contributors may be used to endorse or promote products derived
// from this software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Flags: --allow-natives-syntax
// Test that OSR works properly when using count-based interrupting/profiling.
function osr_this() {
var a = 1;
while (%GetOptimizationCount(osr_this) == 2) ;
return a;
}
assertEquals(1, osr_this());
assertEquals(1, osr_this());
......@@ -2,7 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax --debug-code --gc-interval=201 --verify-heap --max-inlined-source-size=999999 --max-inlined-nodes=999999 --max-inlined-nodes-cumulative=999999
// Flags: --allow-natives-syntax --debug-code --gc-interval=201 --verify-heap
// Flags: --max-inlined-bytecode-size=999999 --max-inlined-bytecode-size-cumulative=999999
// Flags: --opt --no-always-opt
// Begin stripped down and modified version of mjsunit.js for easy minimization in CF.
......
......@@ -16,8 +16,8 @@ TEST(CompilerDispatcherTracerTest, EstimateWithoutSamples) {
EXPECT_EQ(1.0, tracer.EstimateParseInMs(42));
EXPECT_EQ(0.0, tracer.EstimateFinalizeParsingInMs());
EXPECT_EQ(0.0, tracer.EstimatePrepareToCompileInMs());
EXPECT_EQ(1.0, tracer.EstimateCompileInMs(0));
EXPECT_EQ(1.0, tracer.EstimateCompileInMs(42));
EXPECT_EQ(0.0, tracer.EstimateCompileInMs());
EXPECT_EQ(0.0, tracer.EstimateCompileInMs());
EXPECT_EQ(0.0, tracer.EstimateFinalizeCompilingInMs());
}
......
......@@ -474,7 +474,7 @@ TEST_F(CompilerDispatcherTest, CompileOnBackgroundThread) {
// Make compiling super expensive, and advance job as much as possible on the
// foreground thread.
dispatcher.tracer_->RecordCompile(50000.0, 1);
dispatcher.tracer_->RecordCompile(50000.0);
platform.RunIdleTask(10.0, 0.0);
ASSERT_EQ(CompileJobStatus::kReadyToCompile,
GetJobStatus(dispatcher.jobs_.begin()->second));
......@@ -518,7 +518,7 @@ TEST_F(CompilerDispatcherTest, FinishNowWithBackgroundTask) {
// Make compiling super expensive, and advance job as much as possible on the
// foreground thread.
dispatcher.tracer_->RecordCompile(50000.0, 1);
dispatcher.tracer_->RecordCompile(50000.0);
platform.RunIdleTask(10.0, 0.0);
ASSERT_EQ(CompileJobStatus::kReadyToCompile,
GetJobStatus(dispatcher.jobs_.begin()->second));
......@@ -615,7 +615,7 @@ TEST_F(CompilerDispatcherTest, AsyncAbortAllPendingBackgroundTask) {
// Make compiling super expensive, and advance job as much as possible on the
// foreground thread.
dispatcher.tracer_->RecordCompile(50000.0, 1);
dispatcher.tracer_->RecordCompile(50000.0);
platform.RunIdleTask(10.0, 0.0);
ASSERT_EQ(CompileJobStatus::kReadyToCompile,
GetJobStatus(dispatcher.jobs_.begin()->second));
......@@ -663,7 +663,7 @@ TEST_F(CompilerDispatcherTest, AsyncAbortAllRunningBackgroundTask) {
// Make compiling super expensive, and advance job as much as possible on the
// foreground thread.
dispatcher.tracer_->RecordCompile(50000.0, 1);
dispatcher.tracer_->RecordCompile(50000.0);
platform.RunIdleTask(10.0, 0.0);
ASSERT_EQ(CompileJobStatus::kReadyToCompile,
GetJobStatus(dispatcher.jobs_.begin()->second));
......@@ -740,7 +740,7 @@ TEST_F(CompilerDispatcherTest, FinishNowDuringAbortAll) {
// Make compiling super expensive, and advance job as much as possible on the
// foreground thread.
dispatcher.tracer_->RecordCompile(50000.0, 1);
dispatcher.tracer_->RecordCompile(50000.0);
platform.RunIdleTask(10.0, 0.0);
ASSERT_EQ(CompileJobStatus::kReadyToCompile,
GetJobStatus(dispatcher.jobs_.begin()->second));
......@@ -1231,7 +1231,7 @@ TEST_F(CompilerDispatcherTest, CompileMultipleOnBackgroundThread) {
// Make compiling super expensive, and advance job as much as possible on the
// foreground thread.
dispatcher.tracer_->RecordCompile(50000.0, 1);
dispatcher.tracer_->RecordCompile(50000.0);
platform.RunIdleTask(10.0, 0.0);
ASSERT_EQ(dispatcher.jobs_.size(), 2u);
ASSERT_EQ(CompileJobStatus::kReadyToCompile,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment