"ParamTraits deserialization: num runs in 2 seconds" is a "more is better" metric but perf bots think it's a "less is better" metric |
|||||
Issue descriptionIf I read https://codereview.chromium.org/2657873003/diff/20001/cc/ipc/cc_serialization_perftest.cc right, it's good when "ParamTraits deserialization: num runs in 2 seconds" gets higher since it counts how often something happens per time unit. (weiliangc, is that accurate?) Yet on https://chromeperf.appspot.com/group_report?rev=478517 a pretty big increase of that number shows up as red, when I think it really should be green. If that's true: simonhatch, qyearsley: Do you know how to tell the perf system that this metric should be tracked as "more is better"?
,
Jun 12 2017
Looks like this comes from https://cs.chromium.org/chromium/src/third_party/catapult/dashboard/dashboard/elements/alerts-table.html?type=cs&q=%5C(unformatted%5C)+package:%5Echromium$&l=571 ; benjhayden, do you happen to know how to if passing "" as unit in https://codereview.chromium.org/2657873003/diff/20001/cc/ipc/cc_serialization_perftest.cc is correct? If not, what do we have to pass to get a "bigger is better" count thing? Are the perf unit strings documented anywhere?
,
Jun 12 2017
I'm not totally clear where those units come from, I think the dashboard has some UnitsToDirection entities that store that info, not clear how or where that gets set. Adding Ethan/Annie, if they don't know I can dig in further.
,
Jun 12 2017
Try setting unit to "runs/s"? https://github.com/catapult-project/catapult/blob/ff40d4569c8bcf2eed27ed49682e7b8198d24dc1/telemetry/telemetry/value/unit-info.json https://github.com/catapult-project/catapult/blob/ff40d4569c8bcf2eed27ed49682e7b8198d24dc1/dashboard/dashboard/units_to_direction.py#L29
,
Jun 12 2017
I confirmed there is a UnitsToDirection entity with name=runs/s in the chromeperf database with bigger_is_better=true, and units_to_direction.GetImprovementDirection() is called when results are added in add_point_queue._GetOrCreateTest to populate TestMetadata.improvement_direction. So I think setting unit="runs/s" should work.
,
Jun 12 2017
,
Jun 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3b9abe0882ab16d79cf14160512e3b150febf63e commit 3b9abe0882ab16d79cf14160512e3b150febf63e Author: thakis <thakis@chromium.org> Date: Tue Jun 13 03:02:07 2017 cc_perftests: Annotate some metrics as runs/s. This way, the perf bots know that bigger is better for these. BUG= 732489 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2936723003 Cr-Commit-Position: refs/heads/master@{#478874} [modify] https://crrev.com/3b9abe0882ab16d79cf14160512e3b150febf63e/cc/ipc/cc_serialization_perftest.cc
,
Jun 13 2017
https://chromeperf.appspot.com/report?sid=17fb7d70d0a08ada06b93e124e7ada4f760250fb5f56c26ed2052706bc2f46a4&rev=478521 now says "higher is better" and the new "StructTraits serialization/num_runs" metrics on https://chromeperf.appspot.com/group_report?rev=478517 show up as green. The old, existing ones still show up as red, but there's probably nothing we can do about it. (And it's still a progression there, it's just displayed incorrectly.) |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by thakis@chromium.org
, Jun 12 2017