Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2836063003:
Set ganesh cache limit based on system physical RAM (Closed)
|
Created:
3 years, 8 months ago by Justin Novosad Modified:
3 years, 8 months ago CC:
chromium-reviews, piman+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionSet Ganesh cache limit based on system physical RAM
This is a temporary mitigation to alleviate web developer complaints
that canvas rendering slows down with large working sets of images. It also reduces the cache size for very low end devices.
BUG=683994
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Review-Url: https://codereview.chromium.org/2836063003
Cr-Commit-Position: refs/heads/master@{#467338}
Committed: https://chromium.googlesource.com/chromium/src/+/c78b8bf07f4819342ee6d99796100cc62880718b
Patch Set 1 #Patch Set 2 : memory tiers #
Total comments: 1
Patch Set 3 : adjust low-end threshold #Messages
Total messages: 21 (13 generated)
Description was changed from ========== Double skia GPU cache limit on non-low-end devices. This is a temporary mitigation to alleviate web developer complaints that canvas rendering slows down with large working sets of images. Ultimately this problem will rest when we get globally coordinated GPU resource management. BUG=683994 ========== to ========== Double skia GPU cache limit on non-low-end devices. This is a temporary mitigation to alleviate web developer complaints that canvas rendering slows down with large working sets of images. Ultimately this problem will rest when we get globally coordinated GPU resource management. BUG=683994 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
junov@chromium.org changed reviewers: + vmiura@chromium.org
PTAL
The CQ bit was checked by junov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Double skia GPU cache limit on non-low-end devices. This is a temporary mitigation to alleviate web developer complaints that canvas rendering slows down with large working sets of images. Ultimately this problem will rest when we get globally coordinated GPU resource management. BUG=683994 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Set Ganesh cache limit based on system physical RAM This is a temporary mitigation to alleviate web developer complaints that canvas rendering slows down with large working sets of images. It also reduces the cache size for very low end devices. BUG=683994 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
On 2017/04/24 18:56:40, junov - OoO - back April 24 wrote: > PTAL New patch: We now have three tiers: Low-end (<=512MB) Regular High-end (>=4GB)
junov@chromium.org changed reviewers: + vmpstr@chromium.org
vmiura@chromium.org changed reviewers: + ericrk@chromium.org
Generaly LGTM. +ericrk as I guess this will also affect GPU Raster.
I'm OK with this, but we should watch the bots pretty carefully. If we see many regressions in memory, we may want to split this out to not apply to the worker context used for GPU raster. Please CC me if any telemetry memory regressions show up. In general, I don't think this is needed for GPU raster (especially now that images are handled externally) - but it shouldn't really hurt things, as we purge pretty aggressively. I fully support reducing the limit to 48MB on low-end. LGTM https://codereview.chromium.org/2836063003/diff/20001/gpu/skia_bindings/grcon... File gpu/skia_bindings/grcontext_for_gles2_interface.cc (right): https://codereview.chromium.org/2836063003/diff/20001/gpu/skia_bindings/grcon... gpu/skia_bindings/grcontext_for_gles2_interface.cc:42: if (base::SysInfo::IsLowEndDevice()) { Note that on Android O IsLowEndDevice applies to 1GB devices (not just 512M) - I don't know that we want to lower our current 96MB limits for these, WDYT? Instead, we may want to specifically use a kLowEndMemoryThreshold of 512M. I can go either way, just wanted to make sure we had considered this.
On 2017/04/26 00:49:32, ericrk wrote: > > Instead, we may want to specifically use a kLowEndMemoryThreshold of 512M. > Good idea. done.
The CQ bit was checked by junov@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from vmiura@chromium.org, ericrk@chromium.org Link to the patchset: https://codereview.chromium.org/2836063003/#ps40001 (title: "adjust low-end threshold")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493219518103100,
"parent_rev": "ef2a188ba732dd2a75b26909182cea063846c50e", "commit_rev":
"c78b8bf07f4819342ee6d99796100cc62880718b"}
Message was sent while issue was closed.
Description was changed from ========== Set Ganesh cache limit based on system physical RAM This is a temporary mitigation to alleviate web developer complaints that canvas rendering slows down with large working sets of images. It also reduces the cache size for very low end devices. BUG=683994 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Set Ganesh cache limit based on system physical RAM This is a temporary mitigation to alleviate web developer complaints that canvas rendering slows down with large working sets of images. It also reduces the cache size for very low end devices. BUG=683994 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2836063003 Cr-Commit-Position: refs/heads/master@{#467338} Committed: https://chromium.googlesource.com/chromium/src/+/c78b8bf07f4819342ee6d9979610... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/c78b8bf07f4819342ee6d9979610... |
