New issue
Advanced search Search tips

Issue 843643 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Convert non-telemetry based perf tests to output histogram set

Project Member Reported by eyaich@chromium.org, May 16 2018

Issue description

Currently non-telemetry perf tests output their results to stdout and it is converted to chartjson to be uploaded to the perf dashboard.  We need to change this to histogram set to deprecate chartjson.

gengerate_legacy_perf_dashboard_json.py is the file where we do the conversion currently: https://cs.chromium.org/chromium/src/tools/perf/generate_legacy_perf_dashboard_json.py?q=generate_legacy&sq=package:chromium&dr

And it is called from run_gtest_perf_test.py: https://cs.chromium.org/chromium/src/testing/scripts/run_gtest_perf_test.py
 
Cc: eakuefner@chromium.org
+eakuefner

This came out of today's services meeting, as a way to move histograms forward and chartjson towards non-existence.
Cc: -eakuefner@chromium.org simonhatch@chromium.org
Owner: eakuefner@chromium.org
Status: Assigned (was: Untriaged)
I'm going to start working on this.
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 24

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

commit 689a4af354f5445320e374b4306ba854e7bbd523
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Aug 24 07:58:13 2018

Roll src/third_party/catapult 7a1ed44d248a..a9a4168ab6e9 (4 commits)

https://chromium.googlesource.com/catapult.git/+log/7a1ed44d248a..a9a4168ab6e9


git log 7a1ed44d248a..a9a4168ab6e9 --date=short --no-merges --format='%ad %ae %s'
2018-08-24 agrieve@chromium.org cmd_helper: Don't crash when logging command with None in arg list
2018-08-24 simonhatch@chromium.org Pinpoint - Summarize histograms if needed
2018-08-23 benjhayden@chromium.org Add a Material switch for V2SPA.
2018-08-23 eakuefner@chromium.org [TBMv2] Add legacy JSON converter


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

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:843643
TBR=sullivan@chromium.org

Change-Id: I41299c36589c788d8ad539502009a1c392258a1e
Reviewed-on: https://chromium-review.googlesource.com/1187923
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@{#585748}
[modify] https://crrev.com/689a4af354f5445320e374b4306ba854e7bbd523/DEPS

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 11

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

commit f633aa10498a068e426f3b854f61cfc2621d5b3f
Author: Ethan Kuefner <eakuefner@chromium.org>
Date: Tue Sep 11 17:51:12 2018

[TBMv2] Support Chromium commit positions in legacy_json_converter

C++ tests that run on the perf waterfall, such as angle_perftests, produce
legacy dicts that contain a 'revision' field equal to the 'r_commit_pos'
supplemental column. In this case, rather than using the POINT_ID diagnostic,
we must use CHROMIUM_COMMIT_POSITIONS instead.

Bug: chromium:843643
Change-Id: Ic4ff46eebc38a8ef5676226d4b5a88a525df5934
Reviewed-on: https://chromium-review.googlesource.com/1187570
Commit-Queue: Ethan Kuefner <eakuefner@chromium.org>
Reviewed-by: Simon Hatch <simonhatch@chromium.org>

[modify] https://crrev.com/f633aa10498a068e426f3b854f61cfc2621d5b3f/tracing/tracing/value/legacy_json_converter.py
[modify] https://crrev.com/f633aa10498a068e426f3b854f61cfc2621d5b3f/tracing/tracing/value/legacy_json_converter_unittest.py

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 11

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

commit 6333b6741efe422991bab86ab4e4d184aa2582b1
Author: Ethan Kuefner <eakuefner@chromium.org>
Date: Tue Sep 11 17:58:53 2018

Revert "[TBMv2] Support Chromium commit positions in legacy_json_converter"

This reverts commit f633aa10498a068e426f3b854f61cfc2621d5b3f.

Reason for revert: Final patchset didn't upload correctly.

Original change's description:
> [TBMv2] Support Chromium commit positions in legacy_json_converter
> 
> C++ tests that run on the perf waterfall, such as angle_perftests, produce
> legacy dicts that contain a 'revision' field equal to the 'r_commit_pos'
> supplemental column. In this case, rather than using the POINT_ID diagnostic,
> we must use CHROMIUM_COMMIT_POSITIONS instead.
> 
> Bug: chromium:843643
> Change-Id: Ic4ff46eebc38a8ef5676226d4b5a88a525df5934
> Reviewed-on: https://chromium-review.googlesource.com/1187570
> Commit-Queue: Ethan Kuefner <eakuefner@chromium.org>
> Reviewed-by: Simon Hatch <simonhatch@chromium.org>

TBR=simonhatch@chromium.org,eakuefner@chromium.org

Change-Id: Ib003c2b6bfa04ca1718f0d74d91a9c9e85d9e0cc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:843643
Reviewed-on: https://chromium-review.googlesource.com/1220368
Reviewed-by: Ethan Kuefner <eakuefner@chromium.org>
Commit-Queue: Ethan Kuefner <eakuefner@chromium.org>

[modify] https://crrev.com/6333b6741efe422991bab86ab4e4d184aa2582b1/tracing/tracing/value/legacy_json_converter.py
[modify] https://crrev.com/6333b6741efe422991bab86ab4e4d184aa2582b1/tracing/tracing/value/legacy_json_converter_unittest.py

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 11

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

commit 916e932a98c6c9298931e6e3f5e256269bb392d9
Author: Ethan Kuefner <eakuefner@chromium.org>
Date: Tue Sep 11 19:23:05 2018

Reland "[TBMv2] Support Chromium commit positions in legacy_json_converter"

This is a reland of f633aa10498a068e426f3b854f61cfc2621d5b3f

Original change's description:
> [TBMv2] Support Chromium commit positions in legacy_json_converter
>
> C++ tests that run on the perf waterfall, such as angle_perftests, produce
> legacy dicts that contain a 'revision' field equal to the 'r_commit_pos'
> supplemental column. In this case, rather than using the POINT_ID diagnostic,
> we must use CHROMIUM_COMMIT_POSITIONS instead.
>
> Bug: chromium:843643
> Change-Id: Ic4ff46eebc38a8ef5676226d4b5a88a525df5934
> Reviewed-on: https://chromium-review.googlesource.com/1187570
> Commit-Queue: Ethan Kuefner <eakuefner@chromium.org>
> Reviewed-by: Simon Hatch <simonhatch@chromium.org>

Bug: chromium:843643
Change-Id: I2bbba29cd669b85f54e930ef13bd265c0482e89b
TBR=simonhatch
Reviewed-on: https://chromium-review.googlesource.com/1220369
Reviewed-by: Ethan Kuefner <eakuefner@chromium.org>
Commit-Queue: Ethan Kuefner <eakuefner@chromium.org>

[modify] https://crrev.com/916e932a98c6c9298931e6e3f5e256269bb392d9/tracing/tracing/value/legacy_json_converter.py
[modify] https://crrev.com/916e932a98c6c9298931e6e3f5e256269bb392d9/tracing/tracing/value/legacy_json_converter_unittest.py

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 11

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

commit 726d43f19cb8dad790b00a8181a96fd91589c3ee
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Sep 11 21:49:37 2018

Roll src/third_party/catapult b7f5c6989122..6333b6741efe (2 commits)

https://chromium.googlesource.com/catapult.git/+log/b7f5c6989122..6333b6741efe


git log b7f5c6989122..6333b6741efe --date=short --no-merges --format='%ad %ae %s'
2018-09-11 eakuefner@chromium.org Revert "[TBMv2] Support Chromium commit positions in legacy_json_converter"
2018-09-11 eakuefner@chromium.org [TBMv2] Support Chromium commit positions in legacy_json_converter


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

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

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:843643,chromium:843643
TBR=sullivan@chromium.org

Change-Id: I0d1ec9f49266ef65d643842ee5de8c6e3a2261ea
Reviewed-on: https://chromium-review.googlesource.com/1220232
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@{#590490}
[modify] https://crrev.com/726d43f19cb8dad790b00a8181a96fd91589c3ee/DEPS

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 12

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

commit b64bf3102bab3c7097a44d45eb69e91b9bf53664
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Sep 12 00:07:30 2018

Roll src/third_party/catapult 6333b6741efe..fc2514597ca6 (3 commits)

https://chromium.googlesource.com/catapult.git/+log/6333b6741efe..fc2514597ca6


git log 6333b6741efe..fc2514597ca6 --date=short --no-merges --format='%ad %ae %s'
2018-09-11 bsheedy@chromium.org Add functional test quest args
2018-09-11 benjhayden@chromium.org Add more utility functions to v2spa.
2018-09-11 eakuefner@chromium.org Reland "[TBMv2] Support Chromium commit positions in legacy_json_converter"


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

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

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:881017,chromium:843643
TBR=sullivan@chromium.org

Change-Id: I3d8a43b120d45c76ddbac06cf2627853efb54bbb
Reviewed-on: https://chromium-review.googlesource.com/1220239
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@{#590540}
[modify] https://crrev.com/b64bf3102bab3c7097a44d45eb69e91b9bf53664/DEPS

Owner: ----
Status: Available (was: Started)
This plan is covered by https://docs.google.com/document/d/10gM24Ptfovf6TZOsx2KciL2FZo4uZ_Z0mI3ZKiVQnqc/edit (sorry, internal doc). I've assigned ownership of the doc to Ben.
Are there multiple "legacy" data formats? The current conversion script seems to assume that the data format is completely different from what an actual test is producing. For example, this is sample output from the vr_common_perftests gtest perf tests:

{"TextPerfTest.render_time_avg": {"traces": {"render_lorem_ipsum_700_chars": ["8.67613315582", "0.0"], "render_lorem_ipsum_100_chars": ["1.80183339119", "0.0"]}, "units": "ms", "important": ["render_lorem_ipsum_100_chars", "render_lorem_ipsum_700_chars"]}, "TextPerfTest.number_of_runs": {"traces": {"render_lorem_ipsum_700_chars": ["30.0", "0.0"], "render_lorem_ipsum_100_chars": ["30.0", "0.0"]}, "units": "runs", "important": ["render_lorem_ipsum_100_chars", "render_lorem_ipsum_700_chars"]}}

But the conversion script assumes that there are "master", "bot", "test", and "value" keys, none of which are present in the actual data.
No afaik there are only three formats: 

1) legacy c++ format
This is what I call legacy format and it is what is output from non-telemetry tests. see the format in https://cs.chromium.org/chromium/src/tools/perf/generate_legacy_perf_dashboard_json.py?q=generate_legacy&sq=package:chromium&dr.  It looks like this output you are referencing is this and we attach the master, bot, test, values to it when we put it into chartjson format.

2) chartjson format

3) histogram set

The point of this bug was to convert to histogram set instead of chartjson.  I am wondering if Ethan decided to just convert from the chartjson to histogram set instead of just going from legacy directly to histogram set which saves the conversion to chartjson first.
Project Member

Comment 14 by bugdroid1@chromium.org, Jan 3

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

commit ebf0d23ee62ec1b476cc8a0c8616b6d295271680
Author: bsheedy <bsheedy@chromium.org>
Date: Thu Jan 03 21:47:06 2019

Add gtest perf test conversion script

Adds a conversion script to go from the data format output by gtest
perf tests to histograms.

Bug: chromium:843643
Change-Id: I5cded981f9e7bf88abdef418dd3e82cf77637324
Reviewed-on: https://chromium-review.googlesource.com/c/1377773
Reviewed-by: Ben Hayden <benjhayden@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>

[add] https://crrev.com/ebf0d23ee62ec1b476cc8a0c8616b6d295271680/tracing/tracing/value/gtest_json_converter_unittest.py
[add] https://crrev.com/ebf0d23ee62ec1b476cc8a0c8616b6d295271680/tracing/tracing/value/gtest_json_converter.py

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 4

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

commit 0848784dd5697cddd970e2508213f3f3ce3c6ac7
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Fri Jan 04 00:04:41 2019

Roll src/third_party/catapult 86c48a10110c..ebf0d23ee62e (1 commits)

https://chromium.googlesource.com/catapult.git/+log/86c48a10110c..ebf0d23ee62e


git log 86c48a10110c..ebf0d23ee62e --date=short --no-merges --format='%ad %ae %s'
2019-01-03 bsheedy@chromium.org Add gtest perf test conversion script


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

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

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:843643
TBR=sullivan@chromium.org

Change-Id: Iddb0acdf73020d462a3a04654b01472bc24a4daf
Reviewed-on: https://chromium-review.googlesource.com/c/1395000
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#619823}
[modify] https://crrev.com/0848784dd5697cddd970e2508213f3f3ce3c6ac7/DEPS

Sign in to add a comment