Issue metadata
Sign in to add a comment
|
10.6%-25.1% regression in rendering.desktop/thread_raster_cpu_time_per_frame at 601705:601843 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Oct 25
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14ead9e9e40000
,
Oct 25
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/14ead9e9e40000 cc: Ensure bitmap backed images are tracked as discardable. by khushalsagar@chromium.org https://chromium.googlesource.com/chromium/src/+/bd34e88ade5706c80b60b78ead85af8a251d390a thread_raster_cpu_time_per_frame: 0.3407 → 0.4227 (+0.082) Understanding performance regressions: http://g.co/ChromePerformanceRegressions Benchmark documentation link: https://bit.ly/rendering-benchmarks
,
Oct 25
I see what's going on here. Since we treat bitmaps as discardable now, there are 2 extra tasks for decoding and uploading the image which adds to the raster_cpu_time_per_frame. I think these are cases where these is no reuse of the image across tiles because of which this extra task shows up as pure overhead. This is a trade-off, in the case where the image is shared across tiles, this guarantees that we upload it once and avoids the ref/unref churn for every single use of the image which internally also results in a skia flush for GPU raster. Looking through the overall changes with this patch, there are definitely more improvements than regressions: https://chromeperf.appspot.com/group_report?rev=601717 Still I think we could avoid an extra decode task here, since these are bitmaps so they don't need any decoding.
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/388ab98a1a8c12d2d6bf7d7664b6e04100864b41 commit 388ab98a1a8c12d2d6bf7d7664b6e04100864b41 Author: Khushal <khushalsagar@chromium.org> Date: Thu Oct 25 23:29:31 2018 cc: Avoid a decode task for bitmaps uploaded using GPU image cache. These are bitmaps so they don't need a seperate decode task. Avoid the additional task hop. R=ericrk@chromium.org Bug: 898886 Change-Id: I8b61b99e3f488d8def752b6ea1f3472be3723628 Reviewed-on: https://chromium-review.googlesource.com/c/1299782 Reviewed-by: Eric Karl <ericrk@chromium.org> Commit-Queue: Khushal <khushalsagar@chromium.org> Cr-Commit-Position: refs/heads/master@{#602923} [modify] https://crrev.com/388ab98a1a8c12d2d6bf7d7664b6e04100864b41/cc/tiles/gpu_image_decode_cache.cc [modify] https://crrev.com/388ab98a1a8c12d2d6bf7d7664b6e04100864b41/cc/tiles/gpu_image_decode_cache_unittest.cc
,
Oct 26
,
Oct 26
motionmark_anim_images_50 has a fps and frame times regression on Nexus 5 which is worth looking at. I see that we are now doing some work on the background tile worker, which I'm assuming are image tasks. Other than that, the tasks_per_frame regression was reduced by half with the change in #5 as expected.
,
Oct 29
,
Oct 29
Thanks! I observed that the regression has been addressed from the graphs. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Oct 25