Add benchmark to measure V8 runtimeCallStats and GC metrics and monitor the impact on GC metrics. |
||||||||||
Issue descriptionAdd a new benchmark to measure v8 performance on interactive web pages like system health's browse story set. GC metrics are already tracked by v8.browse* benchmarks on this story set. Since RuntimeCallStats is a bit expensive (14% increase in v8 execution time), adding this metric to the existing benchmark could potentially impact the GC metrics. To avoid this add a new benchmark that monitors both runtimeCallStats and GC metrics. If the performance alerts for GC metrics look similar from both of these benchmarks we could merge them into a singe one. So This involves: 1. Adding a new benchmark to measure runtimeCallStats + GC metrics. 2. Monitor the existing v8.browse and the newly added benchmarks over a couple of quarters.
,
Jun 26 2017
mythria@: what is the status of this bug?
,
Jun 26 2017
,
Jun 28 2017
I am looking into it. I looked at the alerts on the linux perf bots from both v8.browsing and v8.runtimestats.browsing and the alerts seem to be different. I have to still check if the benchmarks have been enabled through out. I will also take a look at other bots and update this bug in a couple of days.
,
Jun 28 2017
I discussed with ulan@. Atleast for the linux bot the alerts look OK. There are overlapping alerts and in one case the alert just happened at a little different revision range. So, it looks good. I will also take a quick look at others. Though for windows / mac runtimestats benchmark was disabled for some time. I will now enable the memory metrics and also port the jank metrics from v8.browsing benchmark to v8.runtimestats.benchmark and then wait few weeks for these metrics to stabilize. If everything goes fine, we can deprecate the v8.browsing benchmark.
,
Jul 11 2017
,
Jul 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/51e582272983e7b49f395ccce1f3c56e1dc2d722 commit 51e582272983e7b49f395ccce1f3c56e1dc2d722 Author: Mythri Alle <mythria@chromium.org> Date: Mon Jul 17 14:39:35 2017 Enable memory metric, v8.compile category on v8.runtimestats.browse* We plan to merge v8.browse and v8.runtimestats benchmarks into a single benchmark. So, enable the memory metric on the v8.runtimestats benchmark. Though the expectedQueueingTimeMetric is alredy enabled on v8.runtimestats* it requires v8.compile trace events to work correctly. Also enable v8.compile category for the expectedQueueingTimeMetric. The only other metric tracked by v8.browsing and not by v8.runtimestats.browsing is the v8 executionMetric. This is not required since the runtimeStatsMetric is a super set of the executionMetric. Bug: chromium:690921 Change-Id: I574c8c27515d3a62dc6d94c759be0eed32543f20 Reviewed-on: https://chromium-review.googlesource.com/574171 Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Ned Nguyen <nednguyen@google.com> Commit-Queue: Mythri Alle <mythria@chromium.org> Cr-Commit-Position: refs/heads/master@{#487075} [modify] https://crrev.com/51e582272983e7b49f395ccce1f3c56e1dc2d722/tools/perf/benchmarks/v8_browsing.py
,
Jul 24 2017
,
Jul 24 2017
Looks like this is too much data for the perf dashboard to handle; total data points uploaded per build went from ~3000 to ~8000. See bug 747974 for details. I'm going to revert until we've got it sorted out.
,
Jul 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bc93fdaf7aa3a0ba84e0c1f3802386484f86fc0c commit bc93fdaf7aa3a0ba84e0c1f3802386484f86fc0c Author: Ned Nguyen <nednguyen@google.com> Date: Mon Jul 24 17:26:45 2017 Revert "Enable memory metric, v8.compile category on v8.runtimestats.browse*" This reverts commit 51e582272983e7b49f395ccce1f3c56e1dc2d722. Reason for revert: this turns out produce too many metrics (8k metrics) which make the perf dashboard timed out. Revert this so the benchmark still upload data to the perf dashboard while we think of a way to deal with this. Original change's description: > Enable memory metric, v8.compile category on v8.runtimestats.browse* > > We plan to merge v8.browse and v8.runtimestats benchmarks into a single > benchmark. So, enable the memory metric on the v8.runtimestats benchmark. > Though the expectedQueueingTimeMetric is alredy enabled on v8.runtimestats* > it requires v8.compile trace events to work correctly. Also enable > v8.compile category for the expectedQueueingTimeMetric. > > The only other metric tracked by v8.browsing and not by > v8.runtimestats.browsing is the v8 executionMetric. This is not required > since the runtimeStatsMetric is a super set of the executionMetric. > > > Bug: chromium:690921 > Change-Id: I574c8c27515d3a62dc6d94c759be0eed32543f20 > Reviewed-on: https://chromium-review.googlesource.com/574171 > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Ned Nguyen <nednguyen@google.com> > Commit-Queue: Mythri Alle <mythria@chromium.org> > Cr-Commit-Position: refs/heads/master@{#487075} TBR=ulan@chromium.org,nednguyen@google.com,cbruni@chromium.org,mythria@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: chromium:690921 Change-Id: I9bbb317ce6684c7aafc81ceb10cabe902f41aec6 Reviewed-on: https://chromium-review.googlesource.com/583128 Reviewed-by: Ned Nguyen <nednguyen@google.com> Commit-Queue: Ned Nguyen <nednguyen@google.com> Cr-Commit-Position: refs/heads/master@{#489009} [modify] https://crrev.com/bc93fdaf7aa3a0ba84e0c1f3802386484f86fc0c/tools/perf/benchmarks/v8_browsing.py
,
Aug 29 2017
I am currently investigating what is the best way to merge both these benchmarks. The V8 gc benchmarks also track the jank metrics (eqt* metrics). The compile, parse and optimize contributions to the jank is computed based on the v8.execute trace events. When I enable v8.execute trace events in the v8.runtimestats benchmarks the compile and parse metrics regress by about ~20% on several pages. I tried the other option of trying to migrate the jank metrics to use the RCS events instead of v8.execute trace events. However, the eqt metrics are reported both for the wallclock time and the cpu time. RCS metrics do not track the cpu time and it is very expensive to track the cpu time in RCS. (https://mythria.users.x20web.corp.google.com/www/results_rcs_cpu_time_comparison.html). Not having cpu time for v8 contributions towards the eqt metric is a possibility but that adds kind of inconsistency among the different eqt metrics. So, I am trying why we have a 20% overhead due to the v8.execute trace events. Once I have more data on this we can decide on what is the best way to integrate both these benchmarks.
,
Aug 29 2017
oops. in the comment 11 it is v8.compile instead of v8.execute. So enabling v8.compile trace events regresses parse and compile metrics in RCS by 20%. I am currently investigating it.
,
Sep 1 2017
I took a look at v8.compile trace events again. From what I measured, recording trace event takes around 4-6 micro seconds (including both the time spent initializing at the start of the scope and recording at the end of the scope). Lot of our parse and compile tasks are quite small and hence we see close to 20% overhead. I am not sure why we have such a high overhead. Using cpu time (CLOCK_THREAD_CPUTIME_ID on linux) is expensive but on average it should take less than half a micro second. So even considering that it is still a high overhead.
,
Oct 17 2017
,
Oct 18 2017
,
Oct 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/40d0e648f8925445cd3b4d55e14ba7ed46b05ad5 commit 40d0e648f8925445cd3b4d55e14ba7ed46b05ad5 Author: Mythri Alle <mythria@chromium.org> Date: Wed Oct 25 13:41:35 2017 Use runtimeCallStats to compute renderer.eqt related to v8.compile events Jank related to V8 compile/parse/optimize events was computed using v8.compile trace events. The same events are also measured using runtimeCallStats. Enabling v8.compile trace events adds approximately 20% overhead in compile and parse events. To avoid this overhead and also to avoid measuring the same events using two different trace events this cl migrates the computation of V8 compile/parse event contribution to the renderer.eqt to use runtimeCallStats. Since runtimeCallStats do not measure the cpu time, renderer.eqt_cpu will not have the contribution to these events. Bug: chromium:690921 Change-Id: I57d4b1b5598e68f376dd1fcbcf374b9672853a52 Reviewed-on: https://chromium-review.googlesource.com/725728 Commit-Queue: Mythri Alle <mythria@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Ben Hayden <benjhayden@chromium.org> [modify] https://crrev.com/40d0e648f8925445cd3b4d55e14ba7ed46b05ad5/tracing/tracing/metrics/system_health/expected_queueing_time_metric.html [modify] https://crrev.com/40d0e648f8925445cd3b4d55e14ba7ed46b05ad5/tracing/tracing/metrics/v8/utils.html [modify] https://crrev.com/40d0e648f8925445cd3b4d55e14ba7ed46b05ad5/tracing/tracing/metrics/system_health/expected_queueing_time_metric_test.html
,
Oct 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6fd4e9a35b3ae7031015b700078cd11974778132 commit 6fd4e9a35b3ae7031015b700078cd11974778132 Author: Mythri Alle <mythria@chromium.org> Date: Fri Oct 27 10:18:58 2017 Add memory metric on v8.runtimestats.browse* benchmark We plan to merge v8.browse* and v8.runtimestats.browse* benchmarks. So enable memory metric that is tracked by v8.browse on v8.runtimestats.browse benchmarks. Bug: chromium:690921 Change-Id: I8562bb17c747073835062cb3dfc9f7fa0ff21597 Reviewed-on: https://chromium-review.googlesource.com/738096 Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Ned Nguyen <nednguyen@google.com> Commit-Queue: Mythri Alle <mythria@chromium.org> Cr-Commit-Position: refs/heads/master@{#512141} [modify] https://crrev.com/6fd4e9a35b3ae7031015b700078cd11974778132/tools/perf/benchmarks/v8_browsing.py
,
Oct 30 2017
The rcs based eqt metrics for compile are working fine. The eqt computed by the RCS for compile categories (from v8.runtimestats.browsing_desktop) matches that of the eqt from v8.compile trace events (from v8.browsing_desktop). Some examples here: https://chromeperf.appspot.com/report?sid=d696f938d7739a38ed06875fdd8ab2372c7751ffbe40f107606f864a336fab00
,
Oct 30 2017
Memory metrics also look fine: https://chromeperf.appspot.com/report?sid=0ff105dc025eca3d9511370d53ee583fd8b4097181fb2c044147761f5e5bbd0b
,
Oct 30 2017
All the required work is done now. v8.runtimestats* benchmarks track all the events that are tracked by v8.browsing* benchmarks. I propose to wait for 2-3 weeks and if everything is working fine we can remove v8.browsing benchmarks. Let me know if I have missed something or if there are any other concerns.
,
Oct 30 2017
Thanks a lot, Mythri! The plan sgtm. Are there alerts for the new metrics?
,
Nov 6 2017
,
Nov 6 2017
Thanks Ulan. I totally forgot about alerts. I filed a monitoring request for both memory and eqt metrics. The other metrics RuntimeStats and gcMetric should be monitored already. For the GC metric: here is the list of monitored metrics: https://bugs.chromium.org/p/chromium/issues/detail?id=699516. For RuntimeStats: here is the list: https://bugs.chromium.org/p/chromium/issues/detail?id=699520
,
Nov 10 2017
The alerts for the new metrics have been added. So we are all good to go here. I am planning to send out a cl to delete v8.browsing_desktop and v8.browsing_mobile benchmarks in a week.
,
Nov 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2d88095699f02011d0d0c7d21bf4f348ae453b20 commit 2d88095699f02011d0d0c7d21bf4f348ae453b20 Author: Mythri Alle <mythria@chromium.org> Date: Wed Nov 22 11:48:27 2017 Remove v8.browsing_desktop and v8.browsing_mobile benchmarks. Remove: v8.browsing_desktop v8.browsing_mobile and rename: v8.runtimestats.browsing_desktop to v8.browsing_desktop v8.runtimestats.browsing_mobile to v8.browsing_mobile Bug: chromium:690921 Change-Id: I563e6a59b67d322dea0a41c501e70d12b15e8f01 Reviewed-on: https://chromium-review.googlesource.com/782279 Reviewed-by: Ned Nguyen <nednguyen@google.com> Reviewed-by: Michael Hablich <hablich@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Commit-Queue: Ned Nguyen <nednguyen@google.com> Cr-Commit-Position: refs/heads/master@{#518609} [modify] https://crrev.com/2d88095699f02011d0d0c7d21bf4f348ae453b20/testing/buildbot/chromium.perf.fyi.json [modify] https://crrev.com/2d88095699f02011d0d0c7d21bf4f348ae453b20/testing/buildbot/chromium.perf.json [modify] https://crrev.com/2d88095699f02011d0d0c7d21bf4f348ae453b20/tools/perf/benchmark.csv [modify] https://crrev.com/2d88095699f02011d0d0c7d21bf4f348ae453b20/tools/perf/benchmarks/v8_browsing.py
,
Nov 28 2017
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/e0dc203974cdbdd9eb69a3afb213cfd0dcf400e7 commit e0dc203974cdbdd9eb69a3afb213cfd0dcf400e7 Author: Mythri Alle <mythria@chromium.org> Date: Tue Dec 05 10:35:05 2017 Remove v8.compile trace event based contribution to eqt metrics We now use runtimeCallStats to compute compile time contribution to renderer.eqt metric. All V8 benchmarks have runtimeCallStats enabled now, so the trace event based metrics for v8.compile are no longer required. The other events like GC, execute still use the trace events to compute the contribution to eqt metrics. Bug: chromium:690921 Change-Id: Ia647e5cc8a0594ed0d2aa115d60360ad723fb23c Reviewed-on: https://chromium-review.googlesource.com/787952 Reviewed-by: Fadi Meawad <fmeawad@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Commit-Queue: Mythri Alle <mythria@chromium.org> [modify] https://crrev.com/e0dc203974cdbdd9eb69a3afb213cfd0dcf400e7/tracing/tracing/metrics/system_health/expected_queueing_time_metric.html [modify] https://crrev.com/e0dc203974cdbdd9eb69a3afb213cfd0dcf400e7/tracing/tracing/metrics/v8/utils.html [modify] https://crrev.com/e0dc203974cdbdd9eb69a3afb213cfd0dcf400e7/tracing/tracing/metrics/system_health/expected_queueing_time_metric_test.html |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by bugdroid1@chromium.org
, Feb 17 2017