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

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


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

android-nexus5X

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/12d54770440000

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/12bfabbe440000

Re-kicking bisect.

📍 Found significant differences after each of 2 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/12bfabbe440000

Migrate RWHVAndroid to use new CopyOutputRequest APIs. by miu@chromium.org
https://chromium.googlesource.com/chromium/src/+/419ed0fc82d4f2ddde0a3a54a721e0c623882eff

[test_env.py] Reland: Warm up vpython virtualenv cache on swarming task shards by iannucci@chromium.org
https://chromium.googlesource.com/chromium/src/+/2181305023501418e22e421c3ad7a2ecb0322b4d

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

miu: Sorry the pinpoint chart is a little confusing, it broke at ianucci's CL, but the memory regressed at your CL, "Migrate RWHVAndroid to use new CopyOutputRequest APIs". Can you take a look?

Looking over the code review, I think this is a WontFix. The change did fix a few size calculations, as noted in the change description, any of which may have increased memory usage by the 6%. Also, it's worth noting that we did a lot of memory usage testing on a high-DPI device, with results posted in the code review comments.

At this point, I think it's up to the feature owners how they want to make size vs. quality trade-off a going forward...

Hmm, what size calculation got fixed exactly again?

Looking at before and after traces, I see a single texture going from 2201.5KB to 2389.9KB. What kind of size change could have caused the readback resource to only go up by 8%?

There were errors in the prior code w.r.t. vertical sizing: Legacy code was always subtracting the height of the toolbar from the surface size, but that was erroneous in the new CompositorFrameSink world. That showed up mostly in the DevTools feature as a vertical stretching distortion, but it also affected normal snapshots too. That could very easily account for a ~6-8% increase in bitmap size (toolbar takes up about 6-8% of the screen).

Other factors would be #1 in the change description, or stride changes due to the new image sizes, etc.

It's hard to compare this apples-to-apples since the impl was completely replaced. FWIW, even if there is an unaccounted-for memory increase, IMHO it's worth keeping the new impl because it's a single, shared impl to maintain across all platforms (rather than duplicate copy-and-pasted+tweaked separate code paths). We can, of course, continue to optimize as logistics allow. :)

Ok. I buy that the regression from the toolbar height, so wontfix is correct.