New issue
Advanced search Search tips

Issue 789728 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

10.5%-16.7% regression in media.desktop at 518793:518874

Project Member Reported by wolenetz@chromium.org, Nov 29 2017

Issue description

These 3 seem valid. Note, the linux-release bot has no ref trace in the graph; reported to crouleau@ today (cc'ed). Starting a bisect.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Nov 29 2017

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=789728

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=e98abdb4444bab8104ff1b3ff6eca3a261cecfb9dc25ef165de1941a5cc893a6


Bot(s) for this bug's original alert(s):

chromium-rel-mac-retina
chromium-rel-mac11-pro
linux-release
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Nov 29 2017

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/14b10040040000
Lack of linux-release ref trace is tracked by  bug 789698 .
Cc: -crouleau@chromium.org dalecur...@chromium.org
Owner: crouleau@chromium.org
Status: Assigned (was: Untriaged)
The pinpoint job stopped with errors, but it identified https://chromium.googlesource.com/chromium/src/+/6f2d8cbcc8724cdf7941b5f9554bdf8be7f86b03 as the commit causing difference.

=> Caleb per that commit. Also, is any follow-up of the "job stopped with errors" necessary? cc'ing Dale, who's credited in that commit.
Cc: sande...@chromium.org crouleau@chromium.org
Owner: dalecur...@chromium.org
This memory regression was caused by my bug fix for a freezing video issue after seeking. See issue 787626.

It looks like Dale's original change (crrev.com/508895) caused an improvement in media:effective_size_avg memory usage from 20,740,800 Bytes to 17,579,700 Bytes (but that change also caused the stuttering). Then my fix (crrev.com/518845) caused a regression back to 22,123,200 Bytes. So overall we use 1.3 MB more memory at the end of a video playback that involved at least one seek. All these numbers are from a VP9 1280x720 1327 kb/s video (tulip2.vp9.webm). Other content that I checked didn't have this regression...

Note that these improvements and regressions don't seem to have affected Android or Windows based on the graphs that I checked (https://chromeperf.appspot.com/report?sid=153b1de953e7f079fa03c0573b9d6ab0afd1890ae6f90a48f6acd2d2698b36de&start_rev=485856&end_rev=520204) Perhaps Dale knows why this is the case.

In any case, I'm assigning this bug to Dale since it is a byproduct of his change crrev.com/508895.
Components: Internals>Media
Status: WontFix (was: Assigned)
Reposting my comment from the other bug here: "I don't think that's a real memory regression; neither of our changes really affects how much memory is used over the length of a playback. It's possible timing changes such that ffmpeg sometimes needs to allocate a new frame instead of reusing one, but we'll gc unused frames over time."
Agree that this is WontFix issue. It seems isolated to this device and this content. However, I'm not understanding the idea that it isn't a real regression.

A snapshot of Chrome media memory usage during video playback after a seek with Canary will show 1.3 MB more memory than a snapshot with M62 (Stable) on this particular device for this particular content. (See  issue 789728 ; it looks like this regression only affects this device/content combination)

What is memory used over the length of a playback? Is it the total memory allocated over the lifetime of the playback? I'm not sure that metric would matter from a user perspective unless it affects CPU usage or something else. Wouldn't a user just care how much memory is allocated by Chrome at a given time (since that is memory that cannot be used by other processes during that time)?

Just trying to understand your thinking; it's good to have a clear view of what metrics are important.
This memory metric is generally important. It's not useful in this case because we can be pretty confident of the mechanism by which memory is increasing.

Under the hood software decoders always try to reuse blocks of memory, so in a stable state we can assume we'll use something like X +/- a couple frames of memory for N decoded frames. I say +/- because depending on timing we may not be able to reuse a block of memory (it's out for display, not rendered yet, etc).

The code you deleted can be understood as dropping M frames of N total decoded and doing so in a fairly quick manner (before they reach the renderer). Without the code, we're still decoded N frames, but releasing them more slowly, so under the hood when the decoder may temporarily allocate more memory to satisfy this.

This is the same thing that would happen on a system with different timing with or without the code you deleted.
Thanks for the explanation!
(The pinpoint job failure is reported at https://github.com/catapult-project/catapult/issues/4062 -- thank you, Caleb, for reporting it.)
 Issue 789753  has been merged into this issue.

Sign in to add a comment