Commit fffcbaea authored by Camillo Bruni's avatar Camillo Bruni Committed by V8 LUCI CQ
Browse files

[counters] Fix reentrant timers for V8.Execute

This CL fixes a long standing issue where reentering TimedHistograms
scopes would cause spurious measurements. Only the non-nested scopes
yielded correct results.

Due to the changed numbers, the V8.Execute histogram is renamed to
V8.ExecuteMicroSeconds. Note that this histogram is also guarded
behind the --slow-histograms flag due to the additional overhead.

Unlike before, it does no longer include time for external callbacks
and only measures self time. The following example illustrates the
new behaviour:

1. Enter V8:           |--+.......+--| self-time: 4 units (reported)
2. Exit V8 (callback):    |-+...+-|    self-time: 2 units (ignored)
3. Re-enter V8:             |---|      self-time: 3 units (reported)

This would result in 2 histogram entries with 4 time units for the first
V8 slice and 3 units for the nested part. Note that the callback time
itself is ignored.

This CL attempts to clean up how TimedHistograms work:
- Histogram: the base class
- TimedHistograms: used for time-related histograms that are not nested
- NestedTimeHistograms: Extends TimedHistograms and is used for nested
  histograms

This CL changes Histograms to not measure time themselves. Measurements
happen in the *HistogramScopes:
- BaseTimedHistogramScope: Base functionality
- TimedHistogramScope: For non-nested measurements
- NestedTimedHistogramScope: For nested measurements
- PauseNestedTimedHistogramScope: Ignore time during a given scope.
  This is used to pause timers during callbacks.

Additional changes:
- ExternalCallbackScope now contains a PauseNestedTimedHistogramScope
  and always sets VMState<EXTERNAL>

Bug: v8:11946
Change-Id: I45e4b7ff77b5948b605dd50539044cb26222fa21
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3001345


Reviewed-by: default avatarOmer Katz <omerkatz@chromium.org>
Reviewed-by: default avatarThibaud Michaud <thibaudm@chromium.org>
Reviewed-by: default avatarVictor Gomes <victorgomes@chromium.org>
Reviewed-by: default avatarLeszek Swirski <leszeks@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76111}
parent 549eff77
Supports Markdown
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