Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2493953002:
Initial implementation of V8 Top 25 Runtime Stats benchmark (Closed)
DescriptionInitial implementation of V8 Top 25 Runtime Stats benchmark
This cl creates a story set of top 25+ V8 website consisting of mostly
top websites + some use cases the V8 team is interested to track.
This is a migration from an internal benchmark by the V8 team to
telemetry. It uses the newly created runtimeStatsMetric.
The runtime stats metrics aims at building a breakdown of the time spent
inside of V8 into the different V8 components. The time and count of
key functions in V8 is tracked individually, and then grouped by the
metric into top level V8 groups.
BUG=664318
Removing the perf CQ try bots.
if this benchmark fails, please contact fmeawad@ first.
Committed: https://crrev.com/014643829588899aed0b7608feea34d8826df8b3
Cr-Commit-Position: refs/heads/master@{#432575}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Address comments #
Total comments: 6
Patch Set 3 : Update the timelinebasedmeasurments options to remove unused categories #Patch Set 4 : Rebase #
Messages
Total messages: 47 (26 generated)
Description was changed from ========== Initial implementation of V8 Top 25 Runtime Stats benchmark This cl creates a story set of top 25+ V8 website consisting of mostly top websites + some use cases the V8 team is interested to track. This is a migration from an internal benchmark by the V8 team to telemetry. It uses the newly created runtimeStatsMetric. The runtime stats metrics aims at building a breakdown of the time spent inside of V8 into the different V8 components. The time and count of key functions in V8 is tracked individually, and then grouped by the metric into top level V8 groups. BUG=664318 ========== to ========== Initial implementation of V8 Top 25 Runtime Stats benchmark This cl creates a story set of top 25+ V8 website consisting of mostly top websites + some use cases the V8 team is interested to track. This is a migration from an internal benchmark by the V8 team to telemetry. It uses the newly created runtimeStatsMetric. The runtime stats metrics aims at building a breakdown of the time spent inside of V8 into the different V8 components. The time and count of key functions in V8 is tracked individually, and then grouped by the metric into top level V8 groups. BUG=664318 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ==========
The CQ bit was checked by fmeawad@chromium.org to run a CQ dry run
fmeawad@chromium.org changed reviewers: + nednguyen@google.com
PTAL
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
mostly lg2me https://codereview.chromium.org/2493953002/diff/1/tools/perf/benchmarks/v8_to... File tools/perf/benchmarks/v8_top_25.py (right): https://codereview.chromium.org/2493953002/diff/1/tools/perf/benchmarks/v8_to... tools/perf/benchmarks/v8_top_25.py:4: nits: let merge this to v8.py file https://codereview.chromium.org/2493953002/diff/1/tools/perf/benchmarks/v8_to... tools/perf/benchmarks/v8_top_25.py:14: tag = 'cold' what is this tag for? https://codereview.chromium.org/2493953002/diff/1/tools/perf/page_sets/v8_top... File tools/perf/page_sets/v8_top_25.py (right): https://codereview.chromium.org/2493953002/diff/1/tools/perf/page_sets/v8_top... tools/perf/page_sets/v8_top_25.py:22: "https://www.google.de/search?q=v8", nits: string are always single quoted unless you can't
Comments addressed. Thank you for the quick review. https://codereview.chromium.org/2493953002/diff/1/tools/perf/benchmarks/v8_to... File tools/perf/benchmarks/v8_top_25.py (right): https://codereview.chromium.org/2493953002/diff/1/tools/perf/benchmarks/v8_to... tools/perf/benchmarks/v8_top_25.py:4: On 2016/11/10 23:04:11, nednguyen wrote: > nits: let merge this to v8.py file Done. https://codereview.chromium.org/2493953002/diff/1/tools/perf/benchmarks/v8_to... tools/perf/benchmarks/v8_top_25.py:14: tag = 'cold' On 2016/11/10 23:04:11, nednguyen wrote: > what is this tag for? Not sure, probably copied from another benchmark, removed. https://codereview.chromium.org/2493953002/diff/1/tools/perf/page_sets/v8_top... File tools/perf/page_sets/v8_top_25.py (right): https://codereview.chromium.org/2493953002/diff/1/tools/perf/page_sets/v8_top... tools/perf/page_sets/v8_top_25.py:22: "https://www.google.de/search?q=v8", On 2016/11/10 23:04:11, nednguyen wrote: > nits: string are always single quoted unless you can't Done.
lgtm https://codereview.chromium.org/2493953002/diff/20001/tools/perf/benchmarks/v... File tools/perf/benchmarks/v8.py (right): https://codereview.chromium.org/2493953002/diff/20001/tools/perf/benchmarks/v... tools/perf/benchmarks/v8.py:240: # Below categories are needed for first-meaningful-paint computation. You no longer need these two for FMP. Maybe just reuse page_cycler_v2. TimelineBasedMeasurementOptionsForLoadingMetric in https://cs.chromium.org/chromium/src/tools/perf/benchmarks/page_cycler_v2.py?... https://codereview.chromium.org/2493953002/diff/20001/tools/perf/benchmarks/v... tools/perf/benchmarks/v8.py:263: def ShouldTearDownStateAfterEachStoryRun(cls): You don't need to override this since it's True by default https://codereview.chromium.org/2493953002/diff/20001/tools/perf/page_sets/v8... File tools/perf/page_sets/v8_top_25.py (right): https://codereview.chromium.org/2493953002/diff/20001/tools/perf/page_sets/v8... tools/perf/page_sets/v8_top_25.py:19: nits: add another blank line.
The CQ bit was checked by fmeawad@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Done. https://codereview.chromium.org/2493953002/diff/20001/tools/perf/benchmarks/v... File tools/perf/benchmarks/v8.py (right): https://codereview.chromium.org/2493953002/diff/20001/tools/perf/benchmarks/v... tools/perf/benchmarks/v8.py:240: # Below categories are needed for first-meaningful-paint computation. On 2016/11/11 00:06:09, nednguyen wrote: > You no longer need these two for FMP. Maybe just reuse page_cycler_v2. > TimelineBasedMeasurementOptionsForLoadingMetric in > https://cs.chromium.org/chromium/src/tools/perf/benchmarks/page_cycler_v2.py?... Copied the loadingMetric version, and added a TODO https://codereview.chromium.org/2493953002/diff/20001/tools/perf/benchmarks/v... tools/perf/benchmarks/v8.py:263: def ShouldTearDownStateAfterEachStoryRun(cls): On 2016/11/11 00:06:09, nednguyen wrote: > You don't need to override this since it's True by default Done. https://codereview.chromium.org/2493953002/diff/20001/tools/perf/page_sets/v8... File tools/perf/page_sets/v8_top_25.py (right): https://codereview.chromium.org/2493953002/diff/20001/tools/perf/page_sets/v8... tools/perf/page_sets/v8_top_25.py:19: On 2016/11/11 00:06:09, nednguyen wrote: > nits: add another blank line. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_s5_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by fmeawad@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nednguyen@google.com Link to the patchset: https://codereview.chromium.org/2493953002/#ps10005 (title: "Update the timelinebasedmeasurments options to remove unused categories")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: winx64_10_perf_cq on master.tryserver.chromium.perf (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64_10_perf_c...)
The CQ bit was checked by fmeawad@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by fmeawad@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nednguyen@google.com Link to the patchset: https://codereview.chromium.org/2493953002/#ps50001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_s5_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL) linux_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL) mac_retina_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL) winx64_10_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by fmeawad@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_s5_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by fmeawad@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_s5_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)
Description was changed from ========== Initial implementation of V8 Top 25 Runtime Stats benchmark This cl creates a story set of top 25+ V8 website consisting of mostly top websites + some use cases the V8 team is interested to track. This is a migration from an internal benchmark by the V8 team to telemetry. It uses the newly created runtimeStatsMetric. The runtime stats metrics aims at building a breakdown of the time spent inside of V8 into the different V8 components. The time and count of key functions in V8 is tracked individually, and then grouped by the metric into top level V8 groups. BUG=664318 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ========== to ========== Initial implementation of V8 Top 25 Runtime Stats benchmark This cl creates a story set of top 25+ V8 website consisting of mostly top websites + some use cases the V8 team is interested to track. This is a migration from an internal benchmark by the V8 team to telemetry. It uses the newly created runtimeStatsMetric. The runtime stats metrics aims at building a breakdown of the time spent inside of V8 into the different V8 components. The time and count of key functions in V8 is tracked individually, and then grouped by the metric into top level V8 groups. BUG=664318 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq Removing the android_s5_cq, feel free to revert this CL is v8 benchmarks starts failing with this CL in range on android. ==========
On 2016/11/16 00:14:19, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > android_s5_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build > URL) Removing the android_s5_cq bot.
The CQ bit was checked by fmeawad@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)
Description was changed from ========== Initial implementation of V8 Top 25 Runtime Stats benchmark This cl creates a story set of top 25+ V8 website consisting of mostly top websites + some use cases the V8 team is interested to track. This is a migration from an internal benchmark by the V8 team to telemetry. It uses the newly created runtimeStatsMetric. The runtime stats metrics aims at building a breakdown of the time spent inside of V8 into the different V8 components. The time and count of key functions in V8 is tracked individually, and then grouped by the metric into top level V8 groups. BUG=664318 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq Removing the android_s5_cq, feel free to revert this CL is v8 benchmarks starts failing with this CL in range on android. ========== to ========== Initial implementation of V8 Top 25 Runtime Stats benchmark This cl creates a story set of top 25+ V8 website consisting of mostly top websites + some use cases the V8 team is interested to track. This is a migration from an internal benchmark by the V8 team to telemetry. It uses the newly created runtimeStatsMetric. The runtime stats metrics aims at building a breakdown of the time spent inside of V8 into the different V8 components. The time and count of key functions in V8 is tracked individually, and then grouped by the metric into top level V8 groups. BUG=664318 Removing the perf CQ try bots. if this benchmark fails, please contact fmeawad@ first. ==========
The CQ bit was checked by fmeawad@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Initial implementation of V8 Top 25 Runtime Stats benchmark This cl creates a story set of top 25+ V8 website consisting of mostly top websites + some use cases the V8 team is interested to track. This is a migration from an internal benchmark by the V8 team to telemetry. It uses the newly created runtimeStatsMetric. The runtime stats metrics aims at building a breakdown of the time spent inside of V8 into the different V8 components. The time and count of key functions in V8 is tracked individually, and then grouped by the metric into top level V8 groups. BUG=664318 Removing the perf CQ try bots. if this benchmark fails, please contact fmeawad@ first. ========== to ========== Initial implementation of V8 Top 25 Runtime Stats benchmark This cl creates a story set of top 25+ V8 website consisting of mostly top websites + some use cases the V8 team is interested to track. This is a migration from an internal benchmark by the V8 team to telemetry. It uses the newly created runtimeStatsMetric. The runtime stats metrics aims at building a breakdown of the time spent inside of V8 into the different V8 components. The time and count of key functions in V8 is tracked individually, and then grouped by the metric into top level V8 groups. BUG=664318 Removing the perf CQ try bots. if this benchmark fails, please contact fmeawad@ first. ==========
Message was sent while issue was closed.
Committed patchset #4 (id:50001)
Message was sent while issue was closed.
Description was changed from ========== Initial implementation of V8 Top 25 Runtime Stats benchmark This cl creates a story set of top 25+ V8 website consisting of mostly top websites + some use cases the V8 team is interested to track. This is a migration from an internal benchmark by the V8 team to telemetry. It uses the newly created runtimeStatsMetric. The runtime stats metrics aims at building a breakdown of the time spent inside of V8 into the different V8 components. The time and count of key functions in V8 is tracked individually, and then grouped by the metric into top level V8 groups. BUG=664318 Removing the perf CQ try bots. if this benchmark fails, please contact fmeawad@ first. ========== to ========== Initial implementation of V8 Top 25 Runtime Stats benchmark This cl creates a story set of top 25+ V8 website consisting of mostly top websites + some use cases the V8 team is interested to track. This is a migration from an internal benchmark by the V8 team to telemetry. It uses the newly created runtimeStatsMetric. The runtime stats metrics aims at building a breakdown of the time spent inside of V8 into the different V8 components. The time and count of key functions in V8 is tracked individually, and then grouped by the metric into top level V8 groups. BUG=664318 Removing the perf CQ try bots. if this benchmark fails, please contact fmeawad@ first. Committed: https://crrev.com/014643829588899aed0b7608feea34d8826df8b3 Cr-Commit-Position: refs/heads/master@{#432575} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/014643829588899aed0b7608feea34d8826df8b3 Cr-Commit-Position: refs/heads/master@{#432575}
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:50001) has been created in https://codereview.chromium.org/2505283002/ by dbeam@chromium.org. The reason for reverting is: Broke telemetry_perf_unittests https://build.chromium.org/p/chromium.win/builders/Win%207%20Tests%20x64%20(1) https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20(1) https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
