New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 859524 link

Starred by 1 user
Status: Started
Owner:
█ nednguyen@chromium.org
Last visit > 30 days ago
Cc:
perezju@chromium.org
simonhatch@chromium.org
dtu@chromium.org
sullivan@chromium.org
chanli@chromium.org
seanmccullough@chromium.org
liaoyuke@chromium.org
st...@chromium.org
zhangtiff@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 863861


Sign in to add a comment

Make sure that perf benchmark regression bug & pinpoint surface documentation about how to deal with the benchmark regressions

Project Member Reported by nednguyen@chromium.org, Jul 2 
Benchmarks are complicated (what they run, how to understand their results).

To make it easier for Chrome engineers dealing with the benchmark regressions, we should make suree that perf benchmark regression bug & pinpoint response have documentation about how to deal with the benchmark regressions.

Based on discussion with speed service team, seems like the consensus is we gonna implement this in the way that's similar to specifying perf benchmark owner. 

On benchmark side, I will also add check that makes sure every new benchmark must have documentation (similar to how every of them must have owner)

Comment 1 by nednguyen@chromium.org, Jul 2

Cc: dtu@chromium.org
Components: Speed>Bisection Infra>Flakiness>Dashboard Speed>Benchmarks

Comment 2 by nednguyen@chromium.org, Jul 2

Cc: perezju@chromium.org
I plan to rename the decorator @Owner to @Info, then add another parameter like "document_url".
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 2 

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/a14d6738e1fbed27a858a3e51a17587ff620d155

commit a14d6738e1fbed27a858a3e51a17587ff620d155
Author: nednguyen <nednguyen@google.com>
Date: Mon Jul 02 17:12:34 2018

Rename benchmark @Owner decorator to @Info decorator

This is the first step in adding the ability to specify benchmark documentation
url in @Info decorator.

Bug: chromium:859524
Change-Id: I587d8452bad06a95dcf6479e760b4fe500441322
Reviewed-on: https://chromium-review.googlesource.com/1122957
Reviewed-by: Annie Sullivan <sullivan@chromium.org>
Commit-Queue: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/a14d6738e1fbed27a858a3e51a17587ff620d155/telemetry/telemetry/benchmark.py
[modify] https://crrev.com/a14d6738e1fbed27a858a3e51a17587ff620d155/telemetry/telemetry/decorators_unittest.py
[modify] https://crrev.com/a14d6738e1fbed27a858a3e51a17587ff620d155/telemetry/telemetry/decorators.py
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 2 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ab06bdfb0fa60b1f87a2a94b69dc92d1ca85c24

commit 2ab06bdfb0fa60b1f87a2a94b69dc92d1ca85c24
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Mon Jul 02 19:24:22 2018

Roll src/third_party/catapult 3836c556c4c6..a14d6738e1fb (1 commits)

https://chromium.googlesource.com/catapult.git/+log/3836c556c4c6..a14d6738e1fb


git log 3836c556c4c6..a14d6738e1fb --date=short --no-merges --format='%ad %ae %s'
2018-07-02 nednguyen@google.com Rename benchmark @Owner decorator to @Info decorator


Created with:
  gclient setdep -r src/third_party/catapult@a14d6738e1fb

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG=chromium:859524
TBR=sullivan@chromium.org

Change-Id: Id18e3580f596b177fa3173041ddd596bb3d638de
Reviewed-on: https://chromium-review.googlesource.com/1122996
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#571965}
[modify] https://crrev.com/2ab06bdfb0fa60b1f87a2a94b69dc92d1ca85c24/DEPS
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cd90292c518cfc795886f61d9b1bf991408f3521

commit cd90292c518cfc795886f61d9b1bf991408f3521
Author: Ned Nguyen <nednguyen@google.com>
Date: Tue Jul 03 03:48:32 2018

Update benchmarks code to use @Info decorator and update benchmark ownership documentation

Bug:859524
Change-Id: I7152a82888004508b21ec9d5a2d020019782f33f

This is blocked on catapult roll to roll https://chromium-review.googlesource.com/c/catapult/+/1122957

Change-Id: I7152a82888004508b21ec9d5a2d020019782f33f
Reviewed-on: https://chromium-review.googlesource.com/1122960
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Annie Sullivan <sullivan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572099}
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/docs/speed/benchmark/benchmark_ownership.md
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/blink_perf.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/dromaeo.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/dummy_benchmark.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/jetstream.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/kraken.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/loading.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/media.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/memory.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/octane.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/oortonline.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/power.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/rasterize_and_record_micro.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/rendering.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/smoothness.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/speedometer.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/speedometer2.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/start_with_url.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/startup_mobile.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/system_health.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/tab_switching.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/tracing.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/v8.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/v8_browsing.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/wasm.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/benchmarks/webrtc.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/contrib/cluster_telemetry/skpicture_printer.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/contrib/cros_benchmarks/page_cycler_v2.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/contrib/cros_benchmarks/tab_switching_bench.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/contrib/cros_benchmarks/ui_smoothness_bench.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/contrib/leak_detection/leak_detection.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/contrib/memory_extras/memory_extras.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/contrib/network_service/loading.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/contrib/oilpan/oilpan_benchmarks.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/contrib/oop_raster/oop_raster.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/contrib/oopd/oopd.py
[modify] https://crrev.com/cd90292c518cfc795886f61d9b1bf991408f3521/tools/perf/contrib/vr_benchmarks/vr_benchmarks.py

Comment 6 by perezju@chromium.org, Jul 3 

Oh, maybe I'm too late and it's only a nit, but would it be a tiny bit better if the API was:

  @benchmark.Info(owners=['nednguyen@chromium.org'], ...)

Instead of

  @benchmark.Info(emails=['nednguyen@chromium.org'], ...)

?

Comment 7 by nednguyen@chromium.org, Jul 3 

Juan: I can also make that change
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/9f46241c6c03d9ffb2422e85a362f0bab5d4215f

commit 9f46241c6c03d9ffb2422e85a362f0bab5d4215f
Author: nednguyen <nednguyen@google.com>
Date: Tue Jul 03 15:11:46 2018

Add documentation_link diagnostic

This allows services like perf dashboard or pinpoint to surface the documentation
for the benchmark regression/results.

Bug: chromium:859524
Change-Id: I63c2313719a8bcb163cbebc48db98554aab3a64c
Reviewed-on: https://chromium-review.googlesource.com/1123096
Reviewed-by: Ethan Kuefner <eakuefner@chromium.org>
Commit-Queue: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/9f46241c6c03d9ffb2422e85a362f0bab5d4215f/telemetry/telemetry/benchmark.py
[modify] https://crrev.com/9f46241c6c03d9ffb2422e85a362f0bab5d4215f/telemetry/telemetry/decorators_unittest.py
[modify] https://crrev.com/9f46241c6c03d9ffb2422e85a362f0bab5d4215f/telemetry/telemetry/decorators.py
[modify] https://crrev.com/9f46241c6c03d9ffb2422e85a362f0bab5d4215f/tracing/tracing/value/diagnostics/reserved_infos.py
[modify] https://crrev.com/9f46241c6c03d9ffb2422e85a362f0bab5d4215f/telemetry/telemetry/internal/story_runner.py
[modify] https://crrev.com/9f46241c6c03d9ffb2422e85a362f0bab5d4215f/telemetry/telemetry/internal/story_runner_unittest.py
[modify] https://crrev.com/9f46241c6c03d9ffb2422e85a362f0bab5d4215f/tracing/tracing/value/diagnostics/reserved_names.html
[modify] https://crrev.com/9f46241c6c03d9ffb2422e85a362f0bab5d4215f/docs/how-to-write-metrics.md
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d8354ab023d4aa368c7b979a395522ac8c9d4b4b

commit d8354ab023d4aa368c7b979a395522ac8c9d4b4b
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Jul 03 16:36:21 2018

Roll src/third_party/catapult c61a0380486a..9f46241c6c03 (1 commits)

https://chromium.googlesource.com/catapult.git/+log/c61a0380486a..9f46241c6c03


git log c61a0380486a..9f46241c6c03 --date=short --no-merges --format='%ad %ae %s'
2018-07-03 nednguyen@google.com Add documentation_link diagnostic


Created with:
  gclient setdep -r src/third_party/catapult@9f46241c6c03

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG=chromium:859524
TBR=sullivan@chromium.org

Change-Id: I9f01f6305f67c46c9bb9723ef4b43eb1cd30a67b
Reviewed-on: https://chromium-review.googlesource.com/1124644
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#572251}
[modify] https://crrev.com/d8354ab023d4aa368c7b979a395522ac8c9d4b4b/DEPS

Comment 10 by nednguyen@chromium.org, Jul 11

Owner: nednguyen@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 11 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7a061560c4123d22d230d14a83c9d6d37593a36

commit f7a061560c4123d22d230d14a83c9d6d37593a36
Author: Ned Nguyen <nednguyen@google.com>
Date: Wed Jul 11 19:02:33 2018

Add documentation link for few benchmarks

This CL also updates tools/perf/core_data_generator so that it generates the documentation links in the benchmark.csv file

Bug: chromium:859524
Cq-Include-Trybots: master.tryserver.chromium.perf:obbs_fyi
Change-Id: I99c77a9c2bd60ffdab1f8a628a208b2c81aa4ead
Reviewed-on: https://chromium-review.googlesource.com/1133199
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Annie Sullivan <sullivan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574273}
[modify] https://crrev.com/f7a061560c4123d22d230d14a83c9d6d37593a36/tools/perf/benchmark.csv
[modify] https://crrev.com/f7a061560c4123d22d230d14a83c9d6d37593a36/tools/perf/benchmarks/blink_perf.py
[modify] https://crrev.com/f7a061560c4123d22d230d14a83c9d6d37593a36/tools/perf/benchmarks/loading.py
[modify] https://crrev.com/f7a061560c4123d22d230d14a83c9d6d37593a36/tools/perf/benchmarks/power.py
[modify] https://crrev.com/f7a061560c4123d22d230d14a83c9d6d37593a36/tools/perf/benchmarks/rendering.py
[modify] https://crrev.com/f7a061560c4123d22d230d14a83c9d6d37593a36/tools/perf/benchmarks/system_health.py
[modify] https://crrev.com/f7a061560c4123d22d230d14a83c9d6d37593a36/tools/perf/benchmarks/webrtc.py
[modify] https://crrev.com/f7a061560c4123d22d230d14a83c9d6d37593a36/tools/perf/core/perf_data_generator.py
[modify] https://crrev.com/f7a061560c4123d22d230d14a83c9d6d37593a36/tools/perf/core/perf_data_generator_unittest.py

Comment 12 by nednguyen@chromium.org, Jul 16

Blockedon: 863861
Now that documentations are plumbed through, filed  issue 863861  for surfacing the documentation on perf dashboard/bisection
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 16 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/42f67e24dec4cf73a9c4c3ebc81d674f9d378da6

commit 42f67e24dec4cf73a9c4c3ebc81d674f9d378da6
Author: nednguyen <nednguyen@google.com>
Date: Mon Jul 16 15:29:26 2018

Add a known list of undocumented benchmarks & add check to ensure new benchmarks must be documented

Bug: 859524
Cq-Include-Trybots: master.tryserver.chromium.perf:obbs_fyi
Change-Id: I98dd67a56c1da6f083e8853b6c08b5c87a7bd453
Reviewed-on: https://chromium-review.googlesource.com/1138181
Reviewed-by: Annie Sullivan <sullivan@chromium.org>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#575277}
[modify] https://crrev.com/42f67e24dec4cf73a9c4c3ebc81d674f9d378da6/tools/perf/core/perf_data_generator.py
[add] https://crrev.com/42f67e24dec4cf73a9c4c3ebc81d674f9d378da6/tools/perf/core/undocumented_benchmarks.py
►

Sign in to add a comment