Issue metadata
Sign in to add a comment
|
10.5%-16.7% regression in media.desktop at 518793:518874 |
||||||||||||||||||||||
Issue descriptionThese 3 seem valid. Note, the linux-release bot has no ref trace in the graph; reported to crouleau@ today (cc'ed). Starting a bisect.
,
Nov 29 2017
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14b10040040000
,
Nov 29 2017
😿 Pinpoint job stopped with an error. https://pinpoint-dot-chromeperf.appspot.com/job/14b10040040000
,
Nov 29 2017
Lack of linux-release ref trace is tracked by bug 789698 .
,
Nov 30 2017
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.
,
Nov 30 2017
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.
,
Nov 30 2017
,
Nov 30 2017
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."
,
Nov 30 2017
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.
,
Nov 30 2017
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.
,
Nov 30 2017
Thanks for the explanation!
,
Nov 30 2017
(The pinpoint job failure is reported at https://github.com/catapult-project/catapult/issues/4062 -- thank you, Caleb, for reporting it.)
,
Nov 30 2017
Issue 789753 has been merged into this issue. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Nov 29 2017