Commit f585415a authored by Michael Stanton's avatar Michael Stanton Committed by Commit Bot

Revert "[TurboFan] Remove maximum inlining levels check from inlining heuristics"

This reverts commit ecd3a2ea.

Reason for revert: Bug 779509, a crash with chrome.

Original change's description:
> [TurboFan] Remove maximum inlining levels check from inlining heuristics
> 
> We have a check on maximum number of levels that can be inlined. This
> in some cases causes performance cliffs, when we cannot inline a small
> function because it has exceeded the number of levels. This cl removes
> that check. The intuition is that, having gone down several levels in
> a particular line stopping inlining that chain and exploring a new
> call site may not be beneficial.
> 
> Bug: v8:6871
> Change-Id: I120056db38e78ce48dff010b6cf994259238582a
> Reviewed-on: https://chromium-review.googlesource.com/741705
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#49009}

TBR=mythria@chromium.org,bmeurer@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: v8:6871
Change-Id: I4766f911cb326c224af110be5c0dd7a44362a880
Reviewed-on: https://chromium-review.googlesource.com/743785Reviewed-by: 's avatarMichael Stanton <mvstanton@chromium.org>
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49037}
parent 7ae0a2f9
...@@ -139,6 +139,24 @@ Reduction JSInliningHeuristic::Reduce(Node* node) { ...@@ -139,6 +139,24 @@ Reduction JSInliningHeuristic::Reduce(Node* node) {
} }
if (!can_inline) return NoChange(); if (!can_inline) return NoChange();
// Stop inlining once the maximum allowed level is reached.
int level = 0;
for (Node* frame_state = NodeProperties::GetFrameStateInput(node);
frame_state->opcode() == IrOpcode::kFrameState;
frame_state = NodeProperties::GetFrameStateInput(frame_state)) {
FrameStateInfo const& frame_info = OpParameter<FrameStateInfo>(frame_state);
if (FrameStateFunctionInfo::IsJSFunctionType(frame_info.type())) {
if (++level > FLAG_max_inlining_levels) {
TRACE(
"Not considering call site #%d:%s, because inlining depth "
"%d exceeds maximum allowed level %d\n",
node->id(), node->op()->mnemonic(), level,
FLAG_max_inlining_levels);
return NoChange();
}
}
}
// Gather feedback on how often this call site has been hit before. // Gather feedback on how often this call site has been hit before.
if (node->opcode() == IrOpcode::kJSCall) { if (node->opcode() == IrOpcode::kJSCall) {
CallParameters const p = CallParametersOf(node->op()); CallParameters const p = CallParametersOf(node->op());
......
...@@ -236,6 +236,7 @@ int DebugFrameHelper::FindIndexedNonNativeFrame(StackTraceFrameIterator* it, ...@@ -236,6 +236,7 @@ int DebugFrameHelper::FindIndexedNonNativeFrame(StackTraceFrameIterator* it,
int count = -1; int count = -1;
for (; !it->done(); it->Advance()) { for (; !it->done(); it->Advance()) {
std::vector<FrameSummary> frames; std::vector<FrameSummary> frames;
frames.reserve(FLAG_max_inlining_levels + 1);
it->frame()->Summarize(&frames); it->frame()->Summarize(&frames);
for (size_t i = frames.size(); i != 0; i--) { for (size_t i = frames.size(); i != 0; i--) {
// Omit functions from native and extension scripts. // Omit functions from native and extension scripts.
......
...@@ -29,6 +29,7 @@ DebugStackTraceIterator::DebugStackTraceIterator(Isolate* isolate, int index) ...@@ -29,6 +29,7 @@ DebugStackTraceIterator::DebugStackTraceIterator(Isolate* isolate, int index)
is_top_frame_(true) { is_top_frame_(true) {
if (iterator_.done()) return; if (iterator_.done()) return;
std::vector<FrameSummary> frames; std::vector<FrameSummary> frames;
frames.reserve(FLAG_max_inlining_levels + 1);
iterator_.frame()->Summarize(&frames); iterator_.frame()->Summarize(&frames);
inlined_frame_index_ = static_cast<int>(frames.size()); inlined_frame_index_ = static_cast<int>(frames.size());
Advance(); Advance();
...@@ -60,6 +61,7 @@ void DebugStackTraceIterator::Advance() { ...@@ -60,6 +61,7 @@ void DebugStackTraceIterator::Advance() {
iterator_.Advance(); iterator_.Advance();
if (iterator_.done()) break; if (iterator_.done()) break;
std::vector<FrameSummary> frames; std::vector<FrameSummary> frames;
frames.reserve(FLAG_max_inlining_levels + 1);
iterator_.frame()->Summarize(&frames); iterator_.frame()->Summarize(&frames);
inlined_frame_index_ = static_cast<int>(frames.size()); inlined_frame_index_ = static_cast<int>(frames.size());
} }
......
...@@ -414,6 +414,7 @@ DEFINE_BOOL(turbo_splitting, true, "split nodes during scheduling in TurboFan") ...@@ -414,6 +414,7 @@ DEFINE_BOOL(turbo_splitting, true, "split nodes during scheduling in TurboFan")
DEFINE_BOOL(function_context_specialization, false, DEFINE_BOOL(function_context_specialization, false,
"enable function context specialization in TurboFan") "enable function context specialization in TurboFan")
DEFINE_BOOL(turbo_inlining, true, "enable inlining in TurboFan") DEFINE_BOOL(turbo_inlining, true, "enable inlining in TurboFan")
DEFINE_INT(max_inlining_levels, 5, "maximum number of inlining levels")
DEFINE_INT(max_inlined_bytecode_size, 500, DEFINE_INT(max_inlined_bytecode_size, 500,
"maximum size of bytecode for a single inlining") "maximum size of bytecode for a single inlining")
DEFINE_INT(max_inlined_bytecode_size_cumulative, 1000, DEFINE_INT(max_inlined_bytecode_size_cumulative, 1000,
...@@ -426,6 +427,7 @@ DEFINE_FLOAT(min_inlining_frequency, 0.15, "minimum frequency for inlining") ...@@ -426,6 +427,7 @@ DEFINE_FLOAT(min_inlining_frequency, 0.15, "minimum frequency for inlining")
DEFINE_BOOL(polymorphic_inlining, true, "polymorphic inlining") DEFINE_BOOL(polymorphic_inlining, true, "polymorphic inlining")
DEFINE_BOOL(stress_inline, false, DEFINE_BOOL(stress_inline, false,
"set high thresholds for inlining to inline as much as possible") "set high thresholds for inlining to inline as much as possible")
DEFINE_VALUE_IMPLICATION(stress_inline, max_inlining_levels, 999999)
DEFINE_VALUE_IMPLICATION(stress_inline, max_inlined_bytecode_size, 999999) DEFINE_VALUE_IMPLICATION(stress_inline, max_inlined_bytecode_size, 999999)
DEFINE_VALUE_IMPLICATION(stress_inline, max_inlined_bytecode_size_cumulative, DEFINE_VALUE_IMPLICATION(stress_inline, max_inlined_bytecode_size_cumulative,
999999) 999999)
......
...@@ -1268,6 +1268,7 @@ FrameSummary::~FrameSummary() { ...@@ -1268,6 +1268,7 @@ FrameSummary::~FrameSummary() {
FrameSummary FrameSummary::GetTop(const StandardFrame* frame) { FrameSummary FrameSummary::GetTop(const StandardFrame* frame) {
std::vector<FrameSummary> frames; std::vector<FrameSummary> frames;
frames.reserve(FLAG_max_inlining_levels + 1);
frame->Summarize(&frames); frame->Summarize(&frames);
DCHECK_LT(0, frames.size()); DCHECK_LT(0, frames.size());
return frames.back(); return frames.back();
...@@ -1287,6 +1288,7 @@ FrameSummary FrameSummary::GetSingle(const StandardFrame* frame) { ...@@ -1287,6 +1288,7 @@ FrameSummary FrameSummary::GetSingle(const StandardFrame* frame) {
FrameSummary FrameSummary::Get(const StandardFrame* frame, int index) { FrameSummary FrameSummary::Get(const StandardFrame* frame, int index) {
DCHECK_LE(0, index); DCHECK_LE(0, index);
std::vector<FrameSummary> frames; std::vector<FrameSummary> frames;
frames.reserve(FLAG_max_inlining_levels + 1);
frame->Summarize(&frames); frame->Summarize(&frames);
DCHECK_GT(frames.size(), index); DCHECK_GT(frames.size(), index);
return frames[index]; return frames[index];
......
...@@ -390,6 +390,7 @@ class FrameArrayBuilder { ...@@ -390,6 +390,7 @@ class FrameArrayBuilder {
void AppendStandardFrame(StandardFrame* frame) { void AppendStandardFrame(StandardFrame* frame) {
std::vector<FrameSummary> frames; std::vector<FrameSummary> frames;
frames.reserve(FLAG_max_inlining_levels + 1);
frame->Summarize(&frames); frame->Summarize(&frames);
// A standard frame may include many summarized frames (due to inlining). // A standard frame may include many summarized frames (due to inlining).
for (size_t i = frames.size(); i != 0 && !full(); i--) { for (size_t i = frames.size(); i != 0 && !full(); i--) {
...@@ -801,6 +802,7 @@ Handle<FixedArray> Isolate::CaptureCurrentStackTrace( ...@@ -801,6 +802,7 @@ Handle<FixedArray> Isolate::CaptureCurrentStackTrace(
// Set initial size to the maximum inlining level + 1 for the outermost // Set initial size to the maximum inlining level + 1 for the outermost
// function. // function.
std::vector<FrameSummary> frames; std::vector<FrameSummary> frames;
frames.reserve(FLAG_max_inlining_levels + 1);
frame->Summarize(&frames); frame->Summarize(&frames);
for (size_t i = frames.size(); i != 0 && frames_seen < limit; i--) { for (size_t i = frames.size(); i != 0 && frames_seen < limit; i--) {
FrameSummary& frame = frames[i - 1]; FrameSummary& frame = frames[i - 1];
...@@ -1599,6 +1601,7 @@ bool Isolate::ComputeLocation(MessageLocation* target) { ...@@ -1599,6 +1601,7 @@ bool Isolate::ComputeLocation(MessageLocation* target) {
// baseline code. For optimized code this will use the deoptimization // baseline code. For optimized code this will use the deoptimization
// information to get canonical location information. // information to get canonical location information.
std::vector<FrameSummary> frames; std::vector<FrameSummary> frames;
frames.reserve(FLAG_max_inlining_levels + 1);
frame->Summarize(&frames); frame->Summarize(&frames);
FrameSummary& summary = frames.back(); FrameSummary& summary = frames.back();
int pos = summary.SourcePosition(); int pos = summary.SourcePosition();
......
...@@ -434,6 +434,7 @@ RUNTIME_FUNCTION(Runtime_GetFrameCount) { ...@@ -434,6 +434,7 @@ RUNTIME_FUNCTION(Runtime_GetFrameCount) {
} }
std::vector<FrameSummary> frames; std::vector<FrameSummary> frames;
frames.reserve(FLAG_max_inlining_levels + 1);
for (StackTraceFrameIterator it(isolate, id); !it.done(); it.Advance()) { for (StackTraceFrameIterator it(isolate, id); !it.done(); it.Advance()) {
frames.clear(); frames.clear();
it.frame()->Summarize(&frames); it.frame()->Summarize(&frames);
......
...@@ -370,6 +370,7 @@ bool ComputeLocation(Isolate* isolate, MessageLocation* target) { ...@@ -370,6 +370,7 @@ bool ComputeLocation(Isolate* isolate, MessageLocation* target) {
// baseline code. For optimized code this will use the deoptimization // baseline code. For optimized code this will use the deoptimization
// information to get canonical location information. // information to get canonical location information.
std::vector<FrameSummary> frames; std::vector<FrameSummary> frames;
frames.reserve(FLAG_max_inlining_levels + 1);
it.frame()->Summarize(&frames); it.frame()->Summarize(&frames);
auto& summary = frames.back().AsJavaScript(); auto& summary = frames.back().AsJavaScript();
Handle<SharedFunctionInfo> shared(summary.function()->shared()); Handle<SharedFunctionInfo> shared(summary.function()->shared());
......
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