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

Issue 807737 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

webrtc should reuse perf provided script for uploading to perf dashboard

Project Member Reported by nedngu...@google.com, Jan 31 2018

Issue description

Upon changing how we upload to perf dashboard (https://chromium-review.googlesource.com/c/chromium/tools/build/+/894363), I found that webrtc recipe module use a different script for uploading to perf dashboard.

Looking at it closer, I found that webrtc/steps.py uses both "slave/recipe_modules/webrtc/resources/upload_to_perf_dashboard.py" 
(https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/webrtc/steps.py?rcl=eba45707a4e59ec9808647b137d8aab9e5f91d1d&l=450)

and 

"
slave/upload_perf_dashboard_results.py"
(https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/webrtc/steps.py?rcl=eba45707a4e59ec9808647b137d8aab9e5f91d1d&l=398)

Unclear why webrtc recipe uses two different scripts for uploading, but I think it would be better if these two converges to only using perf upload script.
 
Summary: webrtc should reuse perf provided script for uploading to perf dashboard (was: webrtc should reuse perf recipe infra for uploading to perf dashboard)
Cc: mbonadei@chromium.org oprypin@chromium.org
Components: Infra>Client>WebRTC
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 1 2018

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

commit ed7b4ff9e38c81eee8416dac83f340adf7f14d2e
Author: Edward Lemur <ehmaldonado@webrtc.org>
Date: Thu Feb 01 20:35:24 2018

Use isolated-script-test-perf-output on low_bandwidth_audio_test.

Instead of chartjson-result-file, since that's the flag passed by the recipe.

TBR=phoglund@webrtc.org

Bug:  chromium:807737 
Change-Id: I3a679ab7e5c0a446e675d0f4647344cc4194b357
Reviewed-on: https://webrtc-review.googlesource.com/46541
Commit-Queue: Edward Lemur <ehmaldonado@webrtc.org>
Reviewed-by: Oleh Prypin <oprypin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21858}
[modify] https://crrev.com/ed7b4ff9e38c81eee8416dac83f340adf7f14d2e/audio/test/low_bandwidth_audio_test.py

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 2 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/8e2852d506c1cb6a010e4a6361836bd528ea5ad5

commit 8e2852d506c1cb6a010e4a6361836bd528ea5ad5
Author: Edward Lemur <ehmaldonado@webrtc.org>
Date: Fri Feb 02 14:08:26 2018

Add chartjson_result_file argument to isac_fix_test.

So we can report perf results using JSON and not parsing stdout.

I reordered the way the arguments are parsed, so that options go
at the end, and not at the middle, which is an awkward place to put them.

Regular usage specifying [-I], bottleneck_value, infile and outfile
shouldn't be affected.

Bug:  chromium:807737 
Change-Id: Ida863846400326c33e443d723f384971b891b6e5
Reviewed-on: https://webrtc-review.googlesource.com/47161
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Reviewed-by: Oleh Prypin <oprypin@webrtc.org>
Commit-Queue: Edward Lemur <ehmaldonado@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21873}
[modify] https://crrev.com/8e2852d506c1cb6a010e4a6361836bd528ea5ad5/modules/audio_coding/codecs/isac/fix/test/kenny.cc

Components: -Infra>Platform>Recipes
Status: Started (was: Untriaged)
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 5 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/0501e1cd971fd91bf95d8e71c5f2df9db57196f6

commit 0501e1cd971fd91bf95d8e71c5f2df9db57196f6
Author: Edward Lemur <ehmaldonado@webrtc.org>
Date: Mon Feb 05 16:37:01 2018

Pass chartjson_result_file to gtest_parallel tests.

Translate --isolate-script-test-perf-output to --chartjson_result_file
and pass it to the test.
That way we can use Chromium's recipe code to report results to the
Perf dashboard.

TBR=phoglund@webrtc.org

Bug:  chromium:807737 
Change-Id: I2d3479fe29431cc1a8faf9a73b054a5f4ec610a4
Reviewed-on: https://webrtc-review.googlesource.com/47121
Commit-Queue: Edward Lemur <ehmaldonado@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21894}
[modify] https://crrev.com/0501e1cd971fd91bf95d8e71c5f2df9db57196f6/tools_webrtc/gtest-parallel-wrapper.py

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/bd253b4ab6cb443dd67f7204a8347f4c59deaaf2

commit bd253b4ab6cb443dd67f7204a8347f4c59deaaf2
Author: Edward Lemur <ehmaldonado@chromium.org>
Date: Tue Feb 06 10:52:21 2018

WebRTC: Remove our copy of upload_to_perf_dashboard.

We are now using Chromium's and don't need it anymore.

TBR=phoglund@chromium.org

Bug:  807737 
Change-Id: I4dfa492d5f7b7b8c16fc0c4eb34432596c6fb4d3
Reviewed-on: https://chromium-review.googlesource.com/904042
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

[delete] https://crrev.com/84d3ccb44fe85fb1d3f2dfbf854bf807b1477225/scripts/slave/recipe_modules/webrtc/resources/upload_to_perf_dashboard.py

Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/2160d0a30470da405218e9e1f6b129449b306a1c

commit 2160d0a30470da405218e9e1f6b129449b306a1c
Author: Edward Lemur <ehmaldonado@chromium.org>
Date: Tue Feb 06 14:28:13 2018

Revert "WebRTC: Use perf recipe infra for uploading perf results."

This reverts commit ff806b080d1ac5cfc02da2e9c0fb250ead49c94b.

Reason for revert:
Doesn't upload all results to the dashboard.

Original change's description:
> WebRTC: Use perf recipe infra for uploading perf results.
> 
> Stop using our own script to upload results to the perf dashboard.
> 
> TBR=phoglund@chromium.org
> 
> Bug:  chromium:807737 
> Change-Id: Ide88c25d51b13b49b9ffc93a31308ff66b57fe77
> Reviewed-on: https://chromium-review.googlesource.com/897786
> Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

TBR=ehmaldonado@chromium.org,mbonadei@chromium.org,oprypin@chromium.org

Change-Id: I1fa205ee898bb5bbe371c4579ae91632032ccd83
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:807737 
Reviewed-on: https://chromium-review.googlesource.com/904483
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

[add] https://crrev.com/2160d0a30470da405218e9e1f6b129449b306a1c/scripts/slave/recipe_modules/webrtc/resources/upload_to_perf_dashboard.py
[modify] https://crrev.com/2160d0a30470da405218e9e1f6b129449b306a1c/scripts/slave/recipe_modules/webrtc/steps.py
[modify] https://crrev.com/2160d0a30470da405218e9e1f6b129449b306a1c/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_mac_10_11.json
[modify] https://crrev.com/2160d0a30470da405218e9e1f6b129449b306a1c/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_linux_trusty.json
[modify] https://crrev.com/2160d0a30470da405218e9e1f6b129449b306a1c/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_win7.json

Status: Started (was: Fixed)
Project Member

Comment 12 by bugdroid1@chromium.org, Feb 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/abfe88484f830b420b7da84b71b6197fcb952311

commit abfe88484f830b420b7da84b71b6197fcb952311
Author: Edward Lesmes <ehmaldonado@chromium.org>
Date: Mon Feb 26 22:25:19 2018

Reland "WebRTC: Use perf recipe infra for uploading perf results."

This reverts commit 2160d0a30470da405218e9e1f6b129449b306a1c.

Reason for revert:
webrtc_perf_tests is not uploading perf results, since it's not using gtest-parallel.
This should fix that.

Original change's description:
> Revert "WebRTC: Use perf recipe infra for uploading perf results."
>
> This reverts commit ff806b080d1ac5cfc02da2e9c0fb250ead49c94b.
>
> Reason for revert:
> Doesn't upload all results to the dashboard.
>
> Original change's description:
> > WebRTC: Use perf recipe infra for uploading perf results.
> >
> > Stop using our own script to upload results to the perf dashboard.
> >
> > TBR=phoglund@chromium.org
> >
> > Bug:  chromium:807737 
> > Change-Id: Ide88c25d51b13b49b9ffc93a31308ff66b57fe77
> > Reviewed-on: https://chromium-review.googlesource.com/897786
> > Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
> > Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
>
> TBR=ehmaldonado@chromium.org,mbonadei@chromium.org,oprypin@chromium.org
>
> Change-Id: I1fa205ee898bb5bbe371c4579ae91632032ccd83
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  chromium:807737 
> Reviewed-on: https://chromium-review.googlesource.com/904483
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

TBR=phoglund@chromium.org

Bug:  chromium:807737 
Change-Id: Ibdbedf6568cba675ed8ef1bb6028bef0a1a6e059
Reviewed-on: https://chromium-review.googlesource.com/938441
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

[modify] https://crrev.com/abfe88484f830b420b7da84b71b6197fcb952311/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_mac_10_11.json
[modify] https://crrev.com/abfe88484f830b420b7da84b71b6197fcb952311/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_linux_trusty.json
[modify] https://crrev.com/abfe88484f830b420b7da84b71b6197fcb952311/scripts/slave/README.recipes.md
[modify] https://crrev.com/abfe88484f830b420b7da84b71b6197fcb952311/scripts/slave/recipe_modules/webrtc/steps.py
[delete] https://crrev.com/251ef108e66b6487fc825109af56a4f96f036236/scripts/slave/recipe_modules/webrtc/resources/upload_to_perf_dashboard.py
[modify] https://crrev.com/abfe88484f830b420b7da84b71b6197fcb952311/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_win7.json

Project Member

Comment 13 by bugdroid1@chromium.org, Feb 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/184e67a4e59767be064fd6f6e89dd44910e377e8

commit 184e67a4e59767be064fd6f6e89dd44910e377e8
Author: Edward Lesmes <ehmaldonado@chromium.org>
Date: Tue Feb 27 01:55:07 2018

Revert "Reland "WebRTC: Use perf recipe infra for uploading perf results.""

This reverts commit abfe88484f830b420b7da84b71b6197fcb952311.

Reason for revert: isac_fix_test is not storing perf results in the json file

Original change's description:
> Reland "WebRTC: Use perf recipe infra for uploading perf results."
> 
> This reverts commit 2160d0a30470da405218e9e1f6b129449b306a1c.
> 
> Reason for revert:
> webrtc_perf_tests is not uploading perf results, since it's not using gtest-parallel.
> This should fix that.
> 
> Original change's description:
> > Revert "WebRTC: Use perf recipe infra for uploading perf results."
> >
> > This reverts commit ff806b080d1ac5cfc02da2e9c0fb250ead49c94b.
> >
> > Reason for revert:
> > Doesn't upload all results to the dashboard.
> >
> > Original change's description:
> > > WebRTC: Use perf recipe infra for uploading perf results.
> > >
> > > Stop using our own script to upload results to the perf dashboard.
> > >
> > > TBR=phoglund@chromium.org
> > >
> > > Bug:  chromium:807737 
> > > Change-Id: Ide88c25d51b13b49b9ffc93a31308ff66b57fe77
> > > Reviewed-on: https://chromium-review.googlesource.com/897786
> > > Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
> > > Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> >
> > TBR=ehmaldonado@chromium.org,mbonadei@chromium.org,oprypin@chromium.org
> >
> > Change-Id: I1fa205ee898bb5bbe371c4579ae91632032ccd83
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug:  chromium:807737 
> > Reviewed-on: https://chromium-review.googlesource.com/904483
> > Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> > Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
> 
> TBR=phoglund@chromium.org
> 
> Bug:  chromium:807737 
> Change-Id: Ibdbedf6568cba675ed8ef1bb6028bef0a1a6e059
> Reviewed-on: https://chromium-review.googlesource.com/938441
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

TBR=phoglund@chromium.org,ehmaldonado@chromium.org

Change-Id: I4b2a8f8643f776b58fab04018565fdf62c81456d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:807737 
Reviewed-on: https://chromium-review.googlesource.com/938951
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

[modify] https://crrev.com/184e67a4e59767be064fd6f6e89dd44910e377e8/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_mac_10_11.json
[modify] https://crrev.com/184e67a4e59767be064fd6f6e89dd44910e377e8/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_linux_trusty.json
[modify] https://crrev.com/184e67a4e59767be064fd6f6e89dd44910e377e8/scripts/slave/README.recipes.md
[modify] https://crrev.com/184e67a4e59767be064fd6f6e89dd44910e377e8/scripts/slave/recipe_modules/webrtc/steps.py
[add] https://crrev.com/184e67a4e59767be064fd6f6e89dd44910e377e8/scripts/slave/recipe_modules/webrtc/resources/upload_to_perf_dashboard.py
[modify] https://crrev.com/184e67a4e59767be064fd6f6e89dd44910e377e8/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_win7.json

Project Member

Comment 14 by bugdroid1@chromium.org, Feb 27 2018

Status: Fixed (was: Started)

Sign in to add a comment