[counters] RuntimeStats: fix wrong bookkeeping when dynamically changing counters.

src/counters-inl.h

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef V8_COUNTERS_INL_H_
 #define V8_COUNTERS_INL_H_
 
 #include "src/counters.h"
 
 namespace v8 {
 namespace internal {
 
+inline void RuntimeCallTimer::Start(RuntimeCallCounter* counter,

[Igor Sheludko, 2016/11/18 09:55:45]

Remove inline here and below.

[Camillo Bruni, 2016/11/18 16:36:48]

done.

+                                    RuntimeCallTimer* parent) {
+  counter_ = counter;
+  parent_.SetValue(parent);
+  if (FLAG_runtime_stats !=
+      v8::tracing::TracingCategoryObserver::ENABLED_BY_SAMPLING) {
+    timer_.Start();
+  }
+}
+
+inline RuntimeCallTimer* RuntimeCallTimer::Stop() {
+  if (!timer_.IsStarted()) return parent();
+  base::TimeDelta delta = timer_.Elapsed();
+  timer_.Stop();
+  counter_->count++;
+  counter_->time += delta;
+  if (parent()) {
+    // Adjust parent timer so that it does not include sub timer's time.
+    parent()->Subtract(delta);
+  }
+  return parent();
+}
+
+inline void RuntimeCallTimer::Subtract(base::TimeDelta delta) {
+  // Adjust the current timer instead of directly subtracting the sub-timers
+  // from the current counter. This way we can easily the counter of an active
+  // timer scope. Otherwise we would end up subtracting the time from the
+  // previous counter and add the own time to the newly changed counter.
+  timer_.AdjustBy(-delta);
+}
+
+inline void RuntimeCallTimer::Elapsed() {
+  base::TimeDelta delta = timer_.Elapsed();

[Igor Sheludko, 2016/11/18 09:55:45]

Maybe it's better to just call a Restart() here instead of calling Elapsed()
here and Restart() at the end - this way we will not drop a piece of the time on
the floor.


Maybe what we need instead of Elapsed() is a Snapshot() function:
public:
  void RuntimeCallTimer::Snapshot() {
    Snapshot(ElapsedTimer::Now());
  }

private:
  void RuntimeCallTimer::Snapshot(TimeTicks now) {
    base::TimeDelta delta = timer_.Restart(now);  // add this one
    counter_->time += delta;
    if (parent()) {
      parent()->Subtract(delta);
      parent()->Snapshot(now);
    }
  }

[Camillo Bruni, 2016/11/18 16:36:48]

good point, implemented this way.

+  counter_->time += delta;
+  if (parent()) {
+    parent()->counter_->time -= delta;

[Igor Sheludko, 2016/11/18 09:55:45]

Anyway
  parent()->Subtract(delta);
to avoid multiple corrections.

[Camillo Bruni, 2016/11/18 16:36:48]

done

+    parent()->Elapsed();
+  }
+  timer_.Restart();
+}
+
 RuntimeCallTimerScope::RuntimeCallTimerScope(
     Isolate* isolate, RuntimeCallStats::CounterId counter_id) {
   if (V8_UNLIKELY(FLAG_runtime_stats)) {
     Initialize(isolate->counters()->runtime_call_stats(), counter_id);
   }
 }
 
 RuntimeCallTimerScope::RuntimeCallTimerScope(
     HeapObject* heap_object, RuntimeCallStats::CounterId counter_id) {
   RuntimeCallTimerScope(heap_object->GetIsolate(), counter_id);
 }
 
 RuntimeCallTimerScope::RuntimeCallTimerScope(
     RuntimeCallStats* stats, RuntimeCallStats::CounterId counter_id) {
   if (V8_UNLIKELY(FLAG_runtime_stats)) {
     Initialize(stats, counter_id);
   }
 }
 
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_COUNTERS_INL_H_