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

Issue 648992 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Enable components perf tests on CQ and perf waterfall

Reported by dyaros...@yandex-team.ru, Sep 21 2016

Issue description

In the task https://bugs.chromium.org/p/chromium/issues/detail?id=643668 we've run into a few preparations that are required for creating a perf test on HistoryQuickProvider.

1) - We need to extract common components from run_all_unittests.cc
2) - Decide which of perf frame_works ought to be used base/test/perf* or testing/perf/*
3) - Find out how to include a perf test in regular runs.
 
Cc: ashej...@chromium.org
Components: Infra>Client>Perf
Status: Untriaged (was: Unconfirmed)
Marking the above issue as Untriaged as CL is already up for review.

Thank you!
Committed patch does not enable components perftests in regular runs. Do we create a separate task for this, or it should be done here?

Btw, I'm not the guy to do it, I don't know how it's done and no one who does has come to help out.

Comment 5 by benhenry@google.com, Nov 22 2016

Cc: sullivan@chromium.org
Annie - can you respond to comment #4 and assign?
Owner: sullivan@chromium.org
ping
Owner: martiniss@chromium.org
Adding it to the bot should be very similar to https://codereview.chromium.org/2617133004 

martiniss@ probably can help with reviewing your patch
Status: Assigned (was: Untriaged)
Cc: nedngu...@google.com
Summary: Enable components perf tests on perf waterfall (was: Figuring out components perf tests)
 dyaroshev@, can you work on the CL or do you want Stephen to help you with the CL?
nedgnu...@
I would prefer if somebody else did this task. I can't even test it.
Stephen: can you help  dyaroshev@ with adding component perf tests to the waterfall?

dyaroshev@: to be sure, the build target is //components:components_perftests & what is the .exe script to run?
I can help. Will wait for dyaroshev@ to answer #11
Cc: jochen@chromium.org sdefresne@chromium.org caitkp@chromium.org blundell@chromium.org
Also who from @chromium team should we add as the owner for this benchmark?

CC'ed component owners
#11
This is the target: https://cs.chromium.org/chromium/src/components/BUILD.gn?q=components/BUIL+package:%5Echromium$&l=485

There are actually two sets of tests: mine and some old ones, maybe you want both:
To run all: out/gn/components_perftests
To run just mine: out/gn/components_perftests --gtest_filter=*HQP*

Unfortunately, tests added by me miss some dependency, so they fail if you do not build chrome first. This probably should also be fixed here.
I'm sorry - it requires components_unittests to be built, not chromium.
We can help you with adding the tests to the bot, but not fixing the test' build dependency since we're not experts in the area.

Can you make sure that all required dependencies are in components_perftests then ping this bug again so we can enable it on bot?
#16 Oh, OK. I'm not much of an expert myself but I'll try to do it in the near future. Thanks.
Cc: martiniss@chromium.org
Owner: ----
Status: Available (was: Assigned)
un-assigning myself from this. Let me know if you need more help.
Cc: -ashej...@chromium.org
Owner: csharrison@chromium.org
Status: Started (was: Available)
Summary: Enable components perf tests on CQ and perf waterfall (was: Enable components perf tests on perf waterfall)
I have a component that wants this, so might as well address this bug. I couldn't repro the lack of dependency in #14. We'll see what the bots think though :)
Blocking: 810203
Project Member

Comment 22 by bugdroid1@chromium.org, Mar 15 2018

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

commit b73107cb0226c61a1afae2457f2a3988c1b18c0b
Author: Charles Harrison <csharrison@chromium.org>
Date: Thu Mar 15 12:12:30 2018

Add components_perftests to CQ

This patch also fixes up some missing dependencies to get
components_perftests building + passing on all targets.

Bug:  648992 
Change-Id: I911bdea5ba946e58037f40a4bfa0d534486be761
Reviewed-on: https://chromium-review.googlesource.com/957224
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543348}
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/components/BUILD.gn
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/testing/buildbot/chromium.android.fyi.json
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/testing/buildbot/chromium.android.json
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/testing/buildbot/chromium.chromiumos.json
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/testing/buildbot/chromium.clang.json
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/testing/buildbot/chromium.linux.json
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/testing/buildbot/chromium.mac.json
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/testing/buildbot/chromium.memory.json
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/testing/buildbot/chromium.win.json
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/testing/buildbot/gn_isolate_map.pyl
[modify] https://crrev.com/b73107cb0226c61a1afae2457f2a3988c1b18c0b/testing/buildbot/test_suites.pyl

Where can we see results of this tests? Is where a dashboard with results?
r543348 just enables the tests on the CQ, they are not enabled on the perf waterfall. I can look into enabling the HQP ones in addition to some I'm planning on landing soon. It would be best if we found an @chromium omnibox owner to agree to own the benchmark though.
a-v-y: Dashboards can be found at https://chromeperf.appspot.com/
nednguyen: Can non-chromium members be owners of benchmarks? I see some opera folks in the benchmark owners spreadsheet.
#25: good point. I think all benchmarks owner should be chromium member
Blocking: -810203
Project Member

Comment 28 by bugdroid1@chromium.org, Mar 23 2018

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

commit 3c4489169d2ccd333bec9925afcd4b4c0aad42b7
Author: Charlie Harrison <csharrison@chromium.org>
Date: Fri Mar 23 16:54:17 2018

Prepare components_perftests in gn_isolate_map for perf waterfall

This change prepares us to run components_perftests on the perf waterfall.
It does a few things:
 - Moves the tests to run as an isolated script
 - Moves the target out of chromium_gtests and into suites run by
   isolated_scripts
 - Updates BUILD files to support the perf test runner

Bug:  648992 
Change-Id: I898203dacc2b89754d2681952819b3104d9b95e9
Reviewed-on: https://chromium-review.googlesource.com/971163
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Emily Hanley <eyaich@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545488}
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/BUILD.gn
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/components/BUILD.gn
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/testing/buildbot/chromium.android.fyi.json
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/testing/buildbot/chromium.android.json
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/testing/buildbot/chromium.chromiumos.json
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/testing/buildbot/chromium.clang.json
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/testing/buildbot/chromium.linux.json
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/testing/buildbot/chromium.mac.json
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/testing/buildbot/chromium.memory.json
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/testing/buildbot/chromium.win.json
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/testing/buildbot/gn_isolate_map.pyl
[modify] https://crrev.com/3c4489169d2ccd333bec9925afcd4b4c0aad42b7/testing/buildbot/test_suites.pyl

Project Member

Comment 29 by bugdroid1@chromium.org, Mar 27 2018

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

commit a33ba78b7502b9bfad38cfd48fa5f8ff698bac79
Author: Charlie Harrison <csharrison@chromium.org>
Date: Tue Mar 27 03:17:15 2018

Add components_perftests to chromium.perf.json

Bug:  648992 
Change-Id: Ida40b9d927d851af5d8ba700bfd6a6aeb41281d6
Reviewed-on: https://chromium-review.googlesource.com/977952
Reviewed-by: Emily Hanley <eyaich@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545948}
[modify] https://crrev.com/a33ba78b7502b9bfad38cfd48fa5f8ff698bac79/testing/buildbot/chromium.perf.json
[modify] https://crrev.com/a33ba78b7502b9bfad38cfd48fa5f8ff698bac79/tools/perf/benchmark.csv
[modify] https://crrev.com/a33ba78b7502b9bfad38cfd48fa5f8ff698bac79/tools/perf/core/perf_data_generator.py

Status: Fixed (was: Started)
Fixed, chromeperf.appspot.com is generating graphs for these tests. There's probably some cleanup left to do in the tests themselves though.
Thanks a lot for finally adding this tests to perf run. 
What cleanup needs to be done?
Some of the tests do not have specific enough names when viewed in the dashboard, and need a prefix of what component they are testing (GetMatchingHashPrefix -> SafeBrowsing_GetMatchingHashPrefix for example).

There's also some issue with the dashboard not understanding microsecond measurements. ms ones say "lower is better", but microsecond ones say "? is better".

There may be more small things I notice later, I haven't looked through all the measurements and they have only logged one data point each :)

Sign in to add a comment