New issue
Advanced search Search tips

Issue 700160 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Feature

Blocked on:
issue 774647

Blocking:
issue 745104
issue 632021
issue 713335



Sign in to add a comment

Port media.tough_video_cases benchmark to TBMv2

Project Member Reported by johnchen@chromium.org, Mar 9 2017

Issue description

The existing media.tough_video_cases benchmark uses a legacy framework, which doesn't support BattOr devices for power measurement. In order to add support for BattOr, this benchmark must be ported to use the new TBMv2 (Timeline Based Measurement v2) framework.

Design doc for this feature: https://docs.google.com/document/d/1T1cmVVLmgjUOtxWM-DtOYUoTdDOiQAAP3ZZO7JbSHTE/edit?usp=sharing
 
Here is a high-level overview of this feature: https://docs.google.com/document/d/1vNdlHEIuLVfhaALaOsQmJYh8ipJ3FA0KTg_uPPPO99g
Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 15 2017

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

commit 0d7277979d62a3b3efc707fb52d3b45b727a04a4
Author: johnchen <johnchen@chromium.org>
Date: Wed Mar 15 02:15:23 2017

Replace battor.tough_video_cases with media.tough_video_cases_tbmv2

This is part of the effort to port media.tough_video_cases to use
the new TBMv2 framework, and to add support for BattOr. Existing
measurements of CPU percentage and power consumption have been
moved to the new benchmark, and battor.tough_video_cases benchmark
has been deleted.

R=nednguyen@google.com
BUG= 700160 

Review-Url: https://codereview.chromium.org/2743773002
Cr-Commit-Position: refs/heads/master@{#456953}

[modify] https://crrev.com/0d7277979d62a3b3efc707fb52d3b45b727a04a4/testing/buildbot/chromium.perf.fyi.json
[modify] https://crrev.com/0d7277979d62a3b3efc707fb52d3b45b727a04a4/testing/buildbot/chromium.perf.json
[modify] https://crrev.com/0d7277979d62a3b3efc707fb52d3b45b727a04a4/tools/perf/benchmarks/battor.py
[modify] https://crrev.com/0d7277979d62a3b3efc707fb52d3b45b727a04a4/tools/perf/benchmarks/media.py

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 30 2017

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

commit 7cda4ca0bfbdb63159254a5b274833f5b091a9a0
Author: johnchen <johnchen@chromium.org>
Date: Thu Mar 30 16:06:02 2017

Port TBMv2-based media benchmark to Android

This is a continuation of porting media benchmarks to use TBMv2.
It allows using TBMv2 to collect power (using BattOr) and CPU time
metrics on Android devices, similar to what's already available
on desktop.

BUG= 700160 

Review-Url: https://codereview.chromium.org/2775883003
Cr-Commit-Position: refs/heads/master@{#460781}

[modify] https://crrev.com/7cda4ca0bfbdb63159254a5b274833f5b091a9a0/testing/buildbot/chromium.perf.fyi.json
[modify] https://crrev.com/7cda4ca0bfbdb63159254a5b274833f5b091a9a0/testing/buildbot/chromium.perf.json
[modify] https://crrev.com/7cda4ca0bfbdb63159254a5b274833f5b091a9a0/tools/perf/benchmark.csv
[modify] https://crrev.com/7cda4ca0bfbdb63159254a5b274833f5b091a9a0/tools/perf/benchmarks/media.py
[modify] https://crrev.com/7cda4ca0bfbdb63159254a5b274833f5b091a9a0/tools/perf/page_sets/tough_video_cases.py

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 30 2017

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

commit 7cda4ca0bfbdb63159254a5b274833f5b091a9a0
Author: johnchen <johnchen@chromium.org>
Date: Thu Mar 30 16:06:02 2017

Port TBMv2-based media benchmark to Android

This is a continuation of porting media benchmarks to use TBMv2.
It allows using TBMv2 to collect power (using BattOr) and CPU time
metrics on Android devices, similar to what's already available
on desktop.

BUG= 700160 

Review-Url: https://codereview.chromium.org/2775883003
Cr-Commit-Position: refs/heads/master@{#460781}

[modify] https://crrev.com/7cda4ca0bfbdb63159254a5b274833f5b091a9a0/testing/buildbot/chromium.perf.fyi.json
[modify] https://crrev.com/7cda4ca0bfbdb63159254a5b274833f5b091a9a0/testing/buildbot/chromium.perf.json
[modify] https://crrev.com/7cda4ca0bfbdb63159254a5b274833f5b091a9a0/tools/perf/benchmark.csv
[modify] https://crrev.com/7cda4ca0bfbdb63159254a5b274833f5b091a9a0/tools/perf/benchmarks/media.py
[modify] https://crrev.com/7cda4ca0bfbdb63159254a5b274833f5b091a9a0/tools/perf/page_sets/tough_video_cases.py

Blocking: 713335
Cc: nednguyen@chromium.org charliea@chromium.org perezju@chromium.org johnchen@chromium.org
 Issue 743086  has been merged into this issue.
Blocking: 632021
Labels: -Pri-3 Pri-1
Status: Started (was: Untriaged)
John plans to finish this work this quarter.
Blocking: 745104
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 9 2017

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

commit dfbde222c87f52f3fe87ed0f53962922376267a1
Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org>
Date: Sat Sep 09 00:13:14 2017

Roll src/third_party/catapult/ cd2d1cccb..7a80ad340 (5 commits)

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/cd2d1cccba76..7a80ad3406d1

$ git log cd2d1cccb..7a80ad340 --date=short --no-merges --format='%ad %ae %s'
2017-09-08 benjhayden Dashboard charts: display sparklines of related timeseries in a tab strip.
2017-09-08 etienneb Add support for opening uncompressed traces
2017-09-08 dtu [pinpoint] Show status for every Execution.
2017-09-08 johnchen Port a media metric to TBMv2
2017-09-08 eakuefner Revert of Plumb trace canonicalUrl through TelemetryInfo. (patchset #6 id:160001 of https://chromiumcodereview.appspot.com/3007063002/ )

Created with:
  roll-dep src/third_party/catapult
BUG= 733056 , 700160 


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

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=sullivan@chromium.org

Change-Id: I2ccf4499cb84911b324307d1263cf82aa6b082d4
Reviewed-on: https://chromium-review.googlesource.com/657243
Reviewed-by: <catapult-deps-roller@chromium.org>
Commit-Queue: <catapult-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500751}
[modify] https://crrev.com/dfbde222c87f52f3fe87ed0f53962922376267a1/DEPS

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 11 2017

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

commit 0b035bb8808f1eb744527f2a478448e8b440caa9
Author: John Chen <johnchen@chromium.org>
Date: Mon Sep 11 04:03:42 2017

Port a media metric to TBMv2

Add mediaMetric to TBMv2 media benchmarks, and add necessary trace
events in Chrome.

This CL depends on https://crrev.com/2996233004 in catapult, which
creates the new mediaMetric. The catapult change must land first.

Bug:  700160 
Change-Id: I71445f1e3c816155a09bda7ceecb229bc2db378b
Reviewed-on: https://chromium-review.googlesource.com/622007
Commit-Queue: John Chen <johnchen@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#500836}
[modify] https://crrev.com/0b035bb8808f1eb744527f2a478448e8b440caa9/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/0b035bb8808f1eb744527f2a478448e8b440caa9/media/renderers/audio_renderer_impl.cc
[modify] https://crrev.com/0b035bb8808f1eb744527f2a478448e8b440caa9/media/renderers/video_renderer_impl.cc
[modify] https://crrev.com/0b035bb8808f1eb744527f2a478448e8b440caa9/tools/perf/benchmarks/media.py

Is this bug considered done? If not, what are other blockers?
There are a few more media metrics that I'm migrating. I'll send a code review within a day or two, and after that goes through, we can consider this bug done.
Sounds awesome. Thanks for pushing this through, John!
Project Member

Comment 17 by bugdroid1@chromium.org, Sep 28 2017

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

commit 591738408846bcf010452d5fbbc2c229efaa1dcf
Author: John Chen <johnchen@chromium.org>
Date: Thu Sep 28 04:16:40 2017

Finish migrating media metrics to TBMv2

Update media metrics to add buffering time, dropped frame count,
and seek time.

An example trace that can use these metrics is available at
http://www/~zhanliang/3020433002.trace.html, with corresponding
histograms at http://www/~zhanliang/3020433002.html.

Bug:  chromium:700160 
Change-Id: Iaae2260db02f1b89599ab641bbe8f9503eb23d0b
Reviewed-on: https://chromium-review.googlesource.com/687831
Reviewed-by: Ben Hayden <benjhayden@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>

[modify] https://crrev.com/591738408846bcf010452d5fbbc2c229efaa1dcf/tracing/tracing/metrics/media_metric.html
[modify] https://crrev.com/591738408846bcf010452d5fbbc2c229efaa1dcf/tracing/tracing/metrics/media_metric_test.html

Project Member

Comment 18 by bugdroid1@chromium.org, Sep 28 2017

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

commit 6e481570349b72366ba4083b92eebbb0a06757ba
Author: John Chen <johnchen@chromium.org>
Date: Thu Sep 28 16:16:51 2017

Add media trace events for perf benchmark

Add media trace events needed to calculate buffering time, dropped
frame count, and seek time in media perf benchmarks.

Bug:  700160 
Change-Id: I340fe3763a73fe3a77e6a04d0d8761811ae6838e
Reviewed-on: https://chromium-review.googlesource.com/674166
Commit-Queue: John Chen <johnchen@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505045}
[modify] https://crrev.com/6e481570349b72366ba4083b92eebbb0a06757ba/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/6e481570349b72366ba4083b92eebbb0a06757ba/media/renderers/video_renderer_impl.cc

John is going to document differences between the metrics from the legacy tests and the metrics from the tbmv2 tests and make sure they are accounted for and approved by domain experts. After that, we will see if we need to change anything about the current tbmv2 metrics, and then we delete the legacy benchmark and metrics. After that, we can rename media.tough_video_cases_tbmv2 to media.desktop and media.android.tough_video_cases_tbmv2 to media.mobile.
Awesome, sounds great! Thanks folks!
Project Member

Comment 21 by bugdroid1@chromium.org, Oct 9 2017

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

commit b451847e6fc207b7f39c986b74712bfb33132874
Author: John Chen <johnchen@chromium.org>
Date: Mon Oct 09 19:59:40 2017

Rename TBMv2 media benchmarks

Rename media.tough_video_cases_tbmv2 to media.dekstop, and
media.android.tough_video_cases.tmbv2 to media.mobile.

Bug:  700160 
Change-Id: Iff99db562f3804a8417df6de0ae5dcc2790cf2ee
Reviewed-on: https://chromium-review.googlesource.com/706057
Commit-Queue: John Chen <johnchen@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#507460}
[modify] https://crrev.com/b451847e6fc207b7f39c986b74712bfb33132874/testing/buildbot/chromium.perf.fyi.json
[modify] https://crrev.com/b451847e6fc207b7f39c986b74712bfb33132874/testing/buildbot/chromium.perf.json
[modify] https://crrev.com/b451847e6fc207b7f39c986b74712bfb33132874/tools/perf/benchmark.csv
[modify] https://crrev.com/b451847e6fc207b7f39c986b74712bfb33132874/tools/perf/benchmarks/media.py
[modify] https://crrev.com/b451847e6fc207b7f39c986b74712bfb33132874/tools/perf/core/benchmark_sharding_map.json

Project Member

Comment 22 by bugdroid1@chromium.org, Oct 12 2017

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

commit 52d632bd80b420048553b623fc8e5fbb2bc09849
Author: John Chen <johnchen@chromium.org>
Date: Thu Oct 12 06:37:07 2017

Add event to mark buffering done after seek

When WebMediaPlayerImpl::OnBufferingStateChange is called with
state of BUFFERING_HAVE_ENOUGH, emit a trace event to indicate
the end of seek.

Bug:  700160 
Change-Id: If291a4f4573d05ab1a8dab39995f277ff3fcb15c
Reviewed-on: https://chromium-review.googlesource.com/709454
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508269}
[modify] https://crrev.com/52d632bd80b420048553b623fc8e5fbb2bc09849/media/blink/webmediaplayer_impl.cc

Blockedon: 774647
Project Member

Comment 24 by bugdroid1@chromium.org, Oct 15 2017

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

commit 9e22dab3da5f0dc6dc95dac73ff27d457b358bb2
Author: John Chen <johnchen@chromium.org>
Date: Fri Oct 13 21:14:07 2017

Improve seek time histogram in media metrics

The seek time histogram in previous implementation reported very
different value from the non-TBMv2 benchmark, and is not indicative of
the seek time perceived by the user. This change adds a new seek time
histogram that more closely matches the delay perceived by the user,
and renamed the previous seek time to pipeline seek time.

Bug:  chromium:700160 
Change-Id: I96bfadb38405417fa599e7b4170da17428b8b02e
Reviewed-on: https://chromium-review.googlesource.com/713753
Commit-Queue: John Chen <johnchen@chromium.org>
Reviewed-by: Ben Hayden <benjhayden@chromium.org>

[modify] https://crrev.com/9e22dab3da5f0dc6dc95dac73ff27d457b358bb2/tracing/tracing/metrics/media_metric.html
[modify] https://crrev.com/9e22dab3da5f0dc6dc95dac73ff27d457b358bb2/tracing/tracing/metrics/media_metric_test.html

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 27 2017

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

commit eed882f93394bd92ac159a97a39e99c90f7321c3
Author: John Chen <johnchen@chromium.org>
Date: Fri Oct 27 17:19:04 2017

Remove legacy media benchmarks

Remove media.tough_video_cases, which has been replaced by
media.dekstop, and media.android.tough_video_cases, which has been
replaced by media.mobile.

Bug:  700160 
Change-Id: I47bc7cf1e7798c2f0b30a6f6c2237015c4db0b7b
Reviewed-on: https://chromium-review.googlesource.com/739022
Commit-Queue: John Chen <johnchen@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512222}
[modify] https://crrev.com/eed882f93394bd92ac159a97a39e99c90f7321c3/testing/buildbot/chromium.perf.fyi.json
[modify] https://crrev.com/eed882f93394bd92ac159a97a39e99c90f7321c3/testing/buildbot/chromium.perf.json
[modify] https://crrev.com/eed882f93394bd92ac159a97a39e99c90f7321c3/tools/perf/benchmark.csv
[modify] https://crrev.com/eed882f93394bd92ac159a97a39e99c90f7321c3/tools/perf/benchmarks/media.py
[modify] https://crrev.com/eed882f93394bd92ac159a97a39e99c90f7321c3/tools/perf/fetch_benchmark_deps_unittest.py

Status: Fixed (was: Started)
The port from media.tough_video_cases to media.desktop, and from media.android.tough_video_cases to media.mobile, is now complete.
Cc: -nednguyen@chromium.org nedngu...@google.com
Thanks! This is great!

I've seen though, that a few pages from media.mobile and media.desktop (those based on BeginningToEndPlayPage [1]) still rely on the old SystemMemoryMetric [2]. Is it planned to also remove that dependency?

Also, there is a media.media_cns_cases benchmark with pages based on BasicPlayPage [3] still using the old metric. Should we file a separate bug for that?

The desired end state is being able to remove SystemMemoryMetric completely.

[1]: https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.py?q=add_browser_metrics
[2]: https://cs.chromium.org/chromium/src/tools/perf/measurements/media.py?q=SystemMemoryMetric
[3]: https://cs.chromium.org/chromium/src/tools/perf/page_sets/media_cns_cases.py?q=add_browser_metrics
The removal of media.media_cns_cases benchmark is tracked by  issue #676345 , and is expected to happen soon. This benchmark holds the last reference to perf/measurements/media.py. After media.media_cns_cases is removed, perf/measurements/media.py can also be removed, together with its reference to SystemMemoryMetric.
Awesome, thanks for letting us know.

Sign in to add a comment