Commit d1768823 authored by Alexei Filippov's avatar Alexei Filippov Committed by Commit Bot

Revert "[runtime-call-stats] Fix a long standing crash in RuntimeCallStats::Leave"

This reverts commit 9b157602.

Reason for revert:
Seems to be the cause of 100% crashes of runtime-call-stats layout_test on Windows. https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win7/builds/54947

Original change's description:
> [runtime-call-stats] Fix a long standing crash in RuntimeCallStats::Leave
> 
> There must be a matching Leave for each Enter. Otherwise it ends up
> with a dead stack-allocated object in the timer chain.
> 
> Drive-by: There was also a bug in
> RuntimeCallTimerScope::RuntimeCallTimerScope(HeapObject* ...) did create a
> local object instead of calling an overloaded constructor.
> 
> BUG=chromium:669329
> 
> Change-Id: I9aa1c574a854af8beab3d8097efab3a726ad1c8d
> Reviewed-on: https://chromium-review.googlesource.com/634511
> Commit-Queue: Alexei Filippov <alph@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#47613}

TBR=rmcilroy@chromium.org,alph@chromium.org,cbruni@chromium.org,rmcilroy@google.com

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

Bug: chromium:669329
Change-Id: I57b4fcd2e7bf92a68824d2ac5f40cc74deee0b25
Reviewed-on: https://chromium-review.googlesource.com/636762Reviewed-by: 's avatarAlexei Filippov <alph@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47631}
parent 7e11781b
...@@ -65,25 +65,20 @@ base::TimeTicks RuntimeCallTimer::Now() { ...@@ -65,25 +65,20 @@ base::TimeTicks RuntimeCallTimer::Now() {
RuntimeCallTimerScope::RuntimeCallTimerScope( RuntimeCallTimerScope::RuntimeCallTimerScope(
Isolate* isolate, RuntimeCallStats::CounterId counter_id) { Isolate* isolate, RuntimeCallStats::CounterId counter_id) {
if (V8_LIKELY(!FLAG_runtime_stats)) return; if (V8_UNLIKELY(FLAG_runtime_stats)) {
stats_ = isolate->counters()->runtime_call_stats(); Initialize(isolate->counters()->runtime_call_stats(), counter_id);
RuntimeCallStats::Enter(stats_, &timer_, counter_id); }
} }
RuntimeCallTimerScope::RuntimeCallTimerScope( RuntimeCallTimerScope::RuntimeCallTimerScope(
HeapObject* heap_object, RuntimeCallStats::CounterId counter_id) HeapObject* heap_object, RuntimeCallStats::CounterId counter_id) {
: RuntimeCallTimerScope(heap_object->GetIsolate(), counter_id) {} RuntimeCallTimerScope(heap_object->GetIsolate(), counter_id);
}
RuntimeCallTimerScope::RuntimeCallTimerScope( RuntimeCallTimerScope::RuntimeCallTimerScope(
RuntimeCallStats* stats, RuntimeCallStats::CounterId counter_id) { RuntimeCallStats* stats, RuntimeCallStats::CounterId counter_id) {
if (V8_LIKELY(!FLAG_runtime_stats)) return; if (V8_UNLIKELY(FLAG_runtime_stats)) {
stats_ = stats; Initialize(stats, counter_id);
RuntimeCallStats::Enter(stats_, &timer_, counter_id);
}
RuntimeCallTimerScope::~RuntimeCallTimerScope() {
if (V8_UNLIKELY(stats_ != nullptr)) {
RuntimeCallStats::Leave(stats_, &timer_);
} }
} }
......
...@@ -485,10 +485,26 @@ void RuntimeCallStats::Enter(RuntimeCallStats* stats, RuntimeCallTimer* timer, ...@@ -485,10 +485,26 @@ void RuntimeCallStats::Enter(RuntimeCallStats* stats, RuntimeCallTimer* timer,
// static // static
void RuntimeCallStats::Leave(RuntimeCallStats* stats, RuntimeCallTimer* timer) { void RuntimeCallStats::Leave(RuntimeCallStats* stats, RuntimeCallTimer* timer) {
CHECK(stats->current_timer_.Value() == timer); if (stats->current_timer_.Value() == timer) {
stats->current_timer_.SetValue(timer->Stop()); stats->current_timer_.SetValue(timer->Stop());
RuntimeCallTimer* cur_timer = stats->current_timer_.Value(); } else {
stats->current_counter_.SetValue(cur_timer ? cur_timer->counter() : nullptr); // Must be a Threading cctest. Walk the chain of Timers to find the
// buried one that's leaving. We don't care about keeping nested timings
// accurate, just avoid crashing by keeping the chain intact.
RuntimeCallTimer* next = stats->current_timer_.Value();
while (next && next->parent() != timer) next = next->parent();
if (next == nullptr) return;
next->set_parent(timer->Stop());
}
{
RuntimeCallTimer* cur_timer = stats->current_timer_.Value();
if (cur_timer == nullptr) {
stats->current_counter_.SetValue(nullptr);
} else {
stats->current_counter_.SetValue(cur_timer->counter());
}
}
} }
void RuntimeCallStats::Add(RuntimeCallStats* other) { void RuntimeCallStats::Add(RuntimeCallStats* other) {
......
...@@ -971,9 +971,20 @@ class RuntimeCallTimerScope { ...@@ -971,9 +971,20 @@ class RuntimeCallTimerScope {
RuntimeCallStats::CounterId counter_id); RuntimeCallStats::CounterId counter_id);
inline RuntimeCallTimerScope(RuntimeCallStats* stats, inline RuntimeCallTimerScope(RuntimeCallStats* stats,
RuntimeCallStats::CounterId counter_id); RuntimeCallStats::CounterId counter_id);
inline ~RuntimeCallTimerScope();
inline ~RuntimeCallTimerScope() {
if (V8_UNLIKELY(stats_ != nullptr)) {
RuntimeCallStats::Leave(stats_, &timer_);
}
}
private: private:
V8_INLINE void Initialize(RuntimeCallStats* stats,
RuntimeCallStats::CounterId counter_id) {
stats_ = stats;
RuntimeCallStats::Enter(stats_, &timer_, counter_id);
}
RuntimeCallStats* stats_ = nullptr; RuntimeCallStats* stats_ = nullptr;
RuntimeCallTimer timer_; RuntimeCallTimer timer_;
}; };
......
...@@ -36,32 +36,31 @@ class InterpreterCompilationJob final : public CompilationJob { ...@@ -36,32 +36,31 @@ class InterpreterCompilationJob final : public CompilationJob {
class TimerScope final { class TimerScope final {
public: public:
TimerScope(RuntimeCallStats* stats, RuntimeCallStats::CounterId counter_id) TimerScope(RuntimeCallStats* stats, RuntimeCallStats::CounterId counter_id)
: stats_(stats), runtime_stats_enabled_(FLAG_runtime_stats) { : stats_(stats) {
if (V8_UNLIKELY(runtime_stats_enabled_)) { if (V8_UNLIKELY(FLAG_runtime_stats)) {
RuntimeCallStats::Enter(stats_, &timer_, counter_id); RuntimeCallStats::Enter(stats_, &timer_, counter_id);
} }
} }
explicit TimerScope(RuntimeCallCounter* counter) explicit TimerScope(RuntimeCallCounter* counter) : stats_(nullptr) {
: stats_(nullptr), runtime_stats_enabled_(FLAG_runtime_stats) { if (V8_UNLIKELY(FLAG_runtime_stats)) {
if (V8_UNLIKELY(runtime_stats_enabled_)) {
timer_.Start(counter, nullptr); timer_.Start(counter, nullptr);
} }
} }
~TimerScope() { ~TimerScope() {
if (V8_LIKELY(!runtime_stats_enabled_)) return; if (V8_UNLIKELY(FLAG_runtime_stats)) {
if (stats_) { if (stats_) {
RuntimeCallStats::Leave(stats_, &timer_); RuntimeCallStats::Leave(stats_, &timer_);
} else { } else {
timer_.Stop(); timer_.Stop();
}
} }
} }
private: private:
RuntimeCallStats* stats_; RuntimeCallStats* stats_;
RuntimeCallTimer timer_; RuntimeCallTimer timer_;
bool runtime_stats_enabled_;
}; };
BytecodeGenerator* generator() { return &generator_; } BytecodeGenerator* generator() { return &generator_; }
......
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