From 5504068f49343e067bc8d9c83f93304605873b63 Mon Sep 17 00:00:00 2001 From: Jaroslav Sevcik Date: Mon, 20 May 2019 17:29:33 +0000 Subject: [PATCH] Revert "[cleanup] Remove the now-unused deopt_count from feedback vector." This reverts commit ad1fcd43430e8ab0558c33ac96672b4f0e412f55. Reason for revert: Breaks waterfall. Original change's description: > [cleanup] Remove the now-unused deopt_count from feedback vector. > > Bug: v8:9183 > Change-Id: Iceeccc8ab1e4e77b428e7e2feec39bff3317f241 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1617675 > Commit-Queue: Jaroslav Sevcik > Reviewed-by: Michael Starzinger > Cr-Commit-Position: refs/heads/master@{#61665} TBR=mstarzinger@chromium.org,jarin@chromium.org Change-Id: Iea0e6a329f55a3a941f0b976925b2abdf7eece38 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: v8:9183 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1619867 Reviewed-by: Jaroslav Sevcik Commit-Queue: Jaroslav Sevcik Cr-Commit-Position: refs/heads/master@{#61666} --- src/builtins/base.tq | 1 + src/deoptimizer/deoptimizer.cc | 18 ++++++++++++++---- src/feedback-vector-inl.h | 8 ++++++++ src/feedback-vector.cc | 2 ++ src/feedback-vector.h | 4 ++++ src/heap/factory.cc | 1 + src/runtime/runtime-test.cc | 9 +++++++++ src/runtime/runtime.h | 1 + ...native-context-specialization-hole-check.js | 2 +- test/mjsunit/deopt-recursive-eager-once.js | 4 ++++ test/mjsunit/deopt-recursive-lazy-once.js | 3 +++ test/mjsunit/deopt-recursive-soft-once.js | 5 +++++ test/mjsunit/deopt-unlinked.js | 5 +++++ test/mjsunit/regress/regress-crbug-937734.js | 5 ++--- 14 files changed, 60 insertions(+), 8 deletions(-) diff --git a/src/builtins/base.tq b/src/builtins/base.tq index a911d03e8a..b92f27368e 100644 --- a/src/builtins/base.tq +++ b/src/builtins/base.tq @@ -1257,6 +1257,7 @@ extern class FeedbackVector extends HeapObject { length: int32; invocation_count: int32; profiler_ticks: int32; + deopt_count: int32; } extern class FeedbackCell extends Struct { diff --git a/src/deoptimizer/deoptimizer.cc b/src/deoptimizer/deoptimizer.cc index e61d45a74e..1597804770 100644 --- a/src/deoptimizer/deoptimizer.cc +++ b/src/deoptimizer/deoptimizer.cc @@ -429,6 +429,7 @@ void Deoptimizer::DeoptimizeFunction(JSFunction function, Code code) { function->feedback_vector()->EvictOptimizedCodeMarkedForDeoptimization( function->shared(), "unlinking code marked for deopt"); if (!code->deopt_already_counted()) { + function->feedback_vector()->increment_deopt_count(); code->set_deopt_already_counted(true); } DeoptimizeMarkedCodeForContext(function->context()->native_context()); @@ -492,10 +493,19 @@ Deoptimizer::Deoptimizer(Isolate* isolate, JSFunction function, DCHECK(AllowHeapAllocation::IsAllowed()); disallow_heap_allocation_ = new DisallowHeapAllocation(); #endif // DEBUG - if ((compiled_code_->kind() != Code::OPTIMIZED_FUNCTION || - !compiled_code_->deopt_already_counted()) && - deopt_kind_ == DeoptimizeKind::kSoft) { - isolate->counters()->soft_deopts_executed()->Increment(); + if (compiled_code_->kind() != Code::OPTIMIZED_FUNCTION || + !compiled_code_->deopt_already_counted()) { + // If the function is optimized, and we haven't counted that deopt yet, then + // increment the function's deopt count so that we can avoid optimising + // functions that deopt too often. + + if (deopt_kind_ == DeoptimizeKind::kSoft) { + // Soft deopts shouldn't count against the overall deoptimization count + // that can eventually lead to disabling optimization for a function. + isolate->counters()->soft_deopts_executed()->Increment(); + } else if (!function.is_null()) { + function->feedback_vector()->increment_deopt_count(); + } } if (compiled_code_->kind() == Code::OPTIMIZED_FUNCTION) { compiled_code_->set_deopt_already_counted(true); diff --git a/src/feedback-vector-inl.h b/src/feedback-vector-inl.h index 55839ba7e0..16d2f22fc4 100644 --- a/src/feedback-vector-inl.h +++ b/src/feedback-vector-inl.h @@ -108,6 +108,7 @@ ACCESSORS(FeedbackVector, closure_feedback_cell_array, ClosureFeedbackCellArray, INT32_ACCESSORS(FeedbackVector, length, kLengthOffset) INT32_ACCESSORS(FeedbackVector, invocation_count, kInvocationCountOffset) INT32_ACCESSORS(FeedbackVector, profiler_ticks, kProfilerTicksOffset) +INT32_ACCESSORS(FeedbackVector, deopt_count, kDeoptCountOffset) bool FeedbackVector::is_empty() const { return length() == 0; } @@ -117,6 +118,13 @@ FeedbackMetadata FeedbackVector::metadata() const { void FeedbackVector::clear_invocation_count() { set_invocation_count(0); } +void FeedbackVector::increment_deopt_count() { + int count = deopt_count(); + if (count < std::numeric_limits::max()) { + set_deopt_count(count + 1); + } +} + Code FeedbackVector::optimized_code() const { MaybeObject slot = optimized_code_weak_or_smi(); DCHECK(slot->IsSmi() || slot->IsWeakOrCleared()); diff --git a/src/feedback-vector.cc b/src/feedback-vector.cc index 32ca2e5539..dd2210c285 100644 --- a/src/feedback-vector.cc +++ b/src/feedback-vector.cc @@ -246,6 +246,7 @@ Handle FeedbackVector::New( : OptimizationMarker::kNone))); DCHECK_EQ(vector->invocation_count(), 0); DCHECK_EQ(vector->profiler_ticks(), 0); + DCHECK_EQ(vector->deopt_count(), 0); // Ensure we can skip the write barrier Handle uninitialized_sentinel = UninitializedSentinel(isolate); @@ -367,6 +368,7 @@ void FeedbackVector::EvictOptimizedCodeMarkedForDeoptimization( PrintF("]\n"); } if (!code->deopt_already_counted()) { + increment_deopt_count(); code->set_deopt_already_counted(true); } ClearOptimizedCode(); diff --git a/src/feedback-vector.h b/src/feedback-vector.h index 22feb2faa4..42536b218b 100644 --- a/src/feedback-vector.h +++ b/src/feedback-vector.h @@ -208,7 +208,11 @@ class FeedbackVector : public HeapObject { // runtime profiler. DECL_INT32_ACCESSORS(profiler_ticks) + // [deopt_count]: The number of times this function has deoptimized. + DECL_INT32_ACCESSORS(deopt_count) + inline void clear_invocation_count(); + inline void increment_deopt_count(); inline Code optimized_code() const; inline OptimizationMarker optimization_marker() const; diff --git a/src/heap/factory.cc b/src/heap/factory.cc index b4e965f5ab..bb9ff08725 100644 --- a/src/heap/factory.cc +++ b/src/heap/factory.cc @@ -525,6 +525,7 @@ Handle Factory::NewFeedbackVector( vector->set_length(length); vector->set_invocation_count(0); vector->set_profiler_ticks(0); + vector->set_deopt_count(0); vector->set_closure_feedback_cell_array(*closure_feedback_cell_array); // TODO(leszeks): Initialize based on the feedback metadata. diff --git a/src/runtime/runtime-test.cc b/src/runtime/runtime-test.cc index 1e15743fab..4c66a09e5f 100644 --- a/src/runtime/runtime-test.cc +++ b/src/runtime/runtime-test.cc @@ -572,6 +572,15 @@ RUNTIME_FUNCTION(Runtime_UnblockConcurrentRecompilation) { return ReadOnlyRoots(isolate).undefined_value(); } +RUNTIME_FUNCTION(Runtime_GetDeoptCount) { + HandleScope scope(isolate); + DCHECK_EQ(1, args.length()); + CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0); + // Functions without a feedback vector have never deoptimized. + if (!function->has_feedback_vector()) return Smi::kZero; + return Smi::FromInt(function->feedback_vector()->deopt_count()); +} + static void ReturnThis(const v8::FunctionCallbackInfo& args) { args.GetReturnValue().Set(args.This()); } diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 1db0dfac07..ef0b7f1d00 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -444,6 +444,7 @@ namespace internal { F(DisassembleFunction, 1, 1) \ F(FreezeWasmLazyCompilation, 1, 1) \ F(GetCallable, 0, 1) \ + F(GetDeoptCount, 1, 1) \ F(GetInitializerFunction, 1, 1) \ F(GetOptimizationStatus, -1, 1) \ F(GetUndetectable, 0, 1) \ diff --git a/test/mjsunit/compiler/native-context-specialization-hole-check.js b/test/mjsunit/compiler/native-context-specialization-hole-check.js index 94b30db2d0..7ccdfffa05 100644 --- a/test/mjsunit/compiler/native-context-specialization-hole-check.js +++ b/test/mjsunit/compiler/native-context-specialization-hole-check.js @@ -48,4 +48,4 @@ g(); g(); %OptimizeFunctionOnNextCall(g); g(); -assertUnoptimized(g); +assertTrue(%GetDeoptCount(g) > 0); diff --git a/test/mjsunit/deopt-recursive-eager-once.js b/test/mjsunit/deopt-recursive-eager-once.js index f0bca16c1d..5fa2c3c4f8 100644 --- a/test/mjsunit/deopt-recursive-eager-once.js +++ b/test/mjsunit/deopt-recursive-eager-once.js @@ -16,6 +16,8 @@ function foo(i, deopt = false) { } %PrepareFunctionForOptimization(foo); +assertEquals(0, %GetDeoptCount(foo)); + %PrepareFunctionForOptimization(foo); foo(10); foo(10); @@ -23,7 +25,9 @@ foo(10); foo(10); assertOptimized(foo); +assertEquals(0, %GetDeoptCount(foo)); foo(10, true); assertUnoptimized(foo); +assertEquals(1, %GetDeoptCount(foo)); diff --git a/test/mjsunit/deopt-recursive-lazy-once.js b/test/mjsunit/deopt-recursive-lazy-once.js index 06f62efc51..19acdd77c6 100644 --- a/test/mjsunit/deopt-recursive-lazy-once.js +++ b/test/mjsunit/deopt-recursive-lazy-once.js @@ -17,13 +17,16 @@ function foo(i, deopt = false) { %PrepareFunctionForOptimization(foo); %PrepareFunctionForOptimization(foo); +assertEquals(0, %GetDeoptCount(foo)); foo(10); foo(10); %OptimizeFunctionOnNextCall(foo); foo(10); assertOptimized(foo); +assertEquals(0, %GetDeoptCount(foo)); foo(10, true); assertUnoptimized(foo); +assertEquals(1, %GetDeoptCount(foo)); diff --git a/test/mjsunit/deopt-recursive-soft-once.js b/test/mjsunit/deopt-recursive-soft-once.js index cb27a8d733..9ee761042c 100644 --- a/test/mjsunit/deopt-recursive-soft-once.js +++ b/test/mjsunit/deopt-recursive-soft-once.js @@ -16,6 +16,8 @@ function foo(i, deopt = false, deoptobj = null) { } } +assertEquals(0, %GetDeoptCount(foo)); + %PrepareFunctionForOptimization(foo); foo(10); foo(10); @@ -23,7 +25,10 @@ foo(10); foo(10); assertOptimized(foo); +assertEquals(0, %GetDeoptCount(foo)); foo(10, true, { bar: function(){} }); assertUnoptimized(foo); +// Soft deopts don't count to the deopt count. +assertEquals(0, %GetDeoptCount(foo)); diff --git a/test/mjsunit/deopt-unlinked.js b/test/mjsunit/deopt-unlinked.js index 06a5cc4041..435ea4f739 100644 --- a/test/mjsunit/deopt-unlinked.js +++ b/test/mjsunit/deopt-unlinked.js @@ -9,18 +9,23 @@ function foo() {} +assertEquals(0, %GetDeoptCount(foo)); + foo(); foo(); %OptimizeFunctionOnNextCall(foo); foo(); assertOptimized(foo); +assertEquals(0, %GetDeoptCount(foo)); // Unlink the function. %DeoptimizeFunction(foo); assertUnoptimized(foo); +assertEquals(1, %GetDeoptCount(foo)); foo(); assertUnoptimized(foo); +assertEquals(1, %GetDeoptCount(foo)); diff --git a/test/mjsunit/regress/regress-crbug-937734.js b/test/mjsunit/regress/regress-crbug-937734.js index 2972956db3..26ab8645a6 100644 --- a/test/mjsunit/regress/regress-crbug-937734.js +++ b/test/mjsunit/regress/regress-crbug-937734.js @@ -2,16 +2,15 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// Flags: --allow-natives-syntax --opt +// Flags: --allow-natives-syntax function foo() { return 1 in [0]; } -%PrepareFunctionForOptimization(foo); foo(); foo(); %OptimizeFunctionOnNextCall(foo); foo(); -assertOptimized(foo); +assertEquals(0, %GetDeoptCount(foo));