Add a metric for time-to-scheduled on ItemParallelJob::Task |
||
Issue descriptionWe suspect that other background work may preempt (or even prevent altogether) important background tasks from being scheduled. Add a metric to test this hypothesis
,
Feb 1 2018
Yes. This should be independent of V8.
,
Feb 1 2018
We already have latency tasks for TaskScheduler (TaskScheduler.TaskLatencyMicroseconds.Renderer.*). What I'd like to add is multiple ones (suffixed) that are specific to each V8 background task (because one will go up and the other will go down once we prioritize then properly). As such I don't see how in can be independent of V8?
,
Feb 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/954146a5cf923cfa0462f22d4525fbb81b96d761 commit 954146a5cf923cfa0462f22d4525fbb81b96d761 Author: Gabriel Charette <gab@chromium.org> Date: Thu Feb 01 11:55:42 2018 Make TimeTicks::Now() high-resolution whenever possible with low-latency. It was already always high-resolution on POSIX but was never high resolution on Windows. Windows does support low latency high-resolution timers for the majority of our user base. TimeTicks::HighResolutionNow() was only explicitly requested in testing frameworks. As such I left the call in place but made it DCHECK that it's running on a Windows machine on which high-resolution clocks are used. This confirms that none of our test fleet has regressed with this change (the previous HighResolutionNow() used to be slightly more aggressive and also do it in a few configurations where we now fallback to low-resolution now). This implementation was copied as-is (modulo minor v8 API compatibility tweaks). These implementations were the same in the past but had diverged when, sadly, the same bug was fixed separately years apart, in Chromium and V8: chromium: https://codereview.chromium.org/1284053004 + https://codereview.chromium.org/2393953003 v8: https://codereview.chromium.org/1304873011 This is a prerequisite to add metrics around parallel task execution (low-resolution clocks are useless at that level, but we also don't want to incur high-latency clocks on machines that can't afford it cheaply). Bug: chromium:807606 Change-Id: Id18e7be895d8431ebd0e565a1bdf358fe7838489 Reviewed-on: https://chromium-review.googlesource.com/897485 Reviewed-by: Hannes Payer <hpayer@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#51027} [modify] https://crrev.com/954146a5cf923cfa0462f22d4525fbb81b96d761/src/base/platform/time.cc [modify] https://crrev.com/954146a5cf923cfa0462f22d4525fbb81b96d761/src/base/platform/time.h [modify] https://crrev.com/954146a5cf923cfa0462f22d4525fbb81b96d761/test/unittests/base/platform/time-unittest.cc
,
Feb 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/db73d446b91ee8aaa52ddf4d347e29ba9aa0d4f5 commit db73d446b91ee8aaa52ddf4d347e29ba9aa0d4f5 Author: Gabriel Charette <gab@chromium.org> Date: Fri Feb 02 15:38:55 2018 Bring Time(Delta)::Min/Max() and related helpers to V8. Copied as-is modulo compile tweaks from Chromium's base. Copied tests highlighting existing overflow issues with V8's impl... TimeDelta::Max() will initially be used in V8 to flag events that never triggered in a TimedHistogram. Also constexpr'ed a few things while I was in there, it's harmless at worst and helps a little at best. Ideally would constexpr all the Time*::From*() methods like in Chromium but that has inlining implications and I don't know the impact that could have on V8. Bug: chromium:807606 Change-Id: If5aa92759d985be070e12af4dd20f0159169048b Reviewed-on: https://chromium-review.googlesource.com/899342 Reviewed-by: Hannes Payer <hpayer@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#51073} [modify] https://crrev.com/db73d446b91ee8aaa52ddf4d347e29ba9aa0d4f5/src/base/platform/time.cc [modify] https://crrev.com/db73d446b91ee8aaa52ddf4d347e29ba9aa0d4f5/src/base/platform/time.h [modify] https://crrev.com/db73d446b91ee8aaa52ddf4d347e29ba9aa0d4f5/test/unittests/base/platform/time-unittest.cc
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/85b8daed6357e6d0e104912f1c6454b91ce4abef commit 85b8daed6357e6d0e104912f1c6454b91ce4abef Author: Gabriel Charette <gab@chromium.org> Date: Tue Feb 06 21:53:49 2018 Add V8.GC.ParallelTaskLatencyMicroSeconds metric. It will record the time-to-schedule-after-job-start for different task types to try to highlight use cases where contention might be a problem (and show improvements to it later). Also introducing AsyncTimedHistogram to support this use case whose reported timings go beyond a single scope (i.e. the async version of ScopedTimedHistogram). Bug: chromium:807606 Change-Id: Ib4d581fa8b001723dfe8c91102280e9608b4fabb Reviewed-on: https://chromium-review.googlesource.com/899365 Reviewed-by: Hannes Payer <hpayer@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#51131} [modify] https://crrev.com/85b8daed6357e6d0e104912f1c6454b91ce4abef/src/counters.cc [modify] https://crrev.com/85b8daed6357e6d0e104912f1c6454b91ce4abef/src/counters.h [modify] https://crrev.com/85b8daed6357e6d0e104912f1c6454b91ce4abef/src/heap/heap.cc [modify] https://crrev.com/85b8daed6357e6d0e104912f1c6454b91ce4abef/src/heap/item-parallel-job.h [modify] https://crrev.com/85b8daed6357e6d0e104912f1c6454b91ce4abef/src/heap/mark-compact.cc [modify] https://crrev.com/85b8daed6357e6d0e104912f1c6454b91ce4abef/test/unittests/heap/item-parallel-job-unittest.cc
,
Feb 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8beba4645043b30dbf87c1f9301e6acc802d719c commit 8beba4645043b30dbf87c1f9301e6acc802d719c Author: Gabriel Charette <gab@chromium.org> Date: Thu Feb 08 11:40:22 2018 Add V8.GC.ParallelTaskLatencyMicroSeconds to histograms.xml. Matching V8 CL @ https://chromium-review.googlesource.com/c/v8/v8/+/899365 R=isherman@chromium.org Bug: 807606 Change-Id: I9b1b5d9677234ef58a97df1d7606e16582a53b00 Reviewed-on: https://chromium-review.googlesource.com/905602 Reviewed-by: Ilya Sherman <isherman@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#535357} [modify] https://crrev.com/8beba4645043b30dbf87c1f9301e6acc802d719c/tools/metrics/histograms/histograms.xml
,
Feb 21 2018
Implemented the metric, the 99th'ile is quite high @ 9ms. This indicates that tasks in the renderer might tend to steal the worker pool too much with parallel work. Finer grain work items and priorities on each one would help here. Also, going after the long tail of slow interactions isn't really worth it until we've split all of V8's performance stats into foreground/background (we explicitly don't want to improve the long tail of GCs for invisible tabs). https://uma.googleplex.com/p/chrome/histograms/?endDate=latest&dayCount=7&histograms=V8.GC.ParallelTaskLatencyMicroSeconds&fixupData=true&showMax=true&filters=platform%2Ceq%2CA%2Cchannel%2Ceq%2C1%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial |
||
►
Sign in to add a comment |
||
Comment 1 by rmcilroy@chromium.org
, Jan 31 2018