Issue metadata
Sign in to add a comment
|
Double counting of anonymous GL images in GPU process |
||||||||||||||||||||||||
Issue descriptionThis caused a memory regression ( issue 780510 ) when I turned on service side GMBs (https://chromium-review.googlesource.com/c/chromium/src/+/714654). I've reproduced the regression and it's because the GMBs (the texture images really) are counted in addition to cc resources. In the gpu process, we create these two dumps: 1. gpu/gl/textures/share_group_xx/texture_xx -> (global shared) gl-texture-client-x-process/<share_group_id>/<texture_id> -> (global shared) gl-texture-service-x-process/<share_group_id>/<texture_id> 2. gpu/gl/textures/share_group_xx/texture_xx/face_xx/level_xx -> (global shared) genericsharedmemory-x-process/<client_process_tracing_id>/<client_io_surface_id/client_gmb_id> (for io surfaces) With my change, all resources are textures from the renderer's perspective. Only the service knows that the GLImage for the texture is backed by an IOSurface. The first dump is shared with cc/resource_memory/provider_xx/resource_xx because ResourceProvider::OnMemoryDump adds an edge between that and gl-texture-client-x-process. It seems like the second dump is treated as an independent dump and counts the same memory again. Before my change, the renderer's resources were backed by GMBs so there was no edge between cc/resource_memory and gl-texture-client-x-process. Instead there was a edge form cc to genericsharedmemory-x-process therefore sharing memory with the level dump in 2 above. ericrk@ suggested adding an explicit link between 1 and 2 above (https://chromium-review.googlesource.com/c/chromium/src/+/754441) and that seems to fix the regression. But then the effective size for gpu goes to ~0 as all the memory is charged to renderer/browser cc. See attached traces for before and after my CL, and ericrk's fix.
,
Jul 24
Possible fix for just the IO surface accounting: https://chromium-review.googlesource.com/c/chromium/src/+/1147674 Basically we add a nested node instead a shared edge when the IO surface is anonymous. Pinpoint job to confirm: https://pinpoint-dot-chromeperf.appspot.com/job/105aa2b3a40000 I suspect this will also help with issue 863121 (memory regression after mac views).
,
Jul 24
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/15d5cb5da40000
,
Jul 24
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/15d5cb5da40000
,
Jul 24
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14e58b3ea40000
,
Jul 24
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/14e58b3ea40000
,
Jul 25
Looks like the change worked as expected. all_processes:gpu:effective_size_avg goes down by 48% as anonymous IOSurfaces are no longer double counted, and all the memory is charged to cc's tile or resource memory.
,
Jul 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4da52fed413909a85bb4e52fc7c82e3a62bec3df commit 4da52fed413909a85bb4e52fc7c82e3a62bec3df Author: Sunny Sachanandani <sunnyps@chromium.org> Date: Wed Jul 25 19:48:45 2018 Fix anonymous GLImageIOSurface memory double counting Anonymous GLImageIOSurfaces are textures from the client's perspective so the client adds a shared ownership edge with the GPU process texture instead of considering it as a GpuMemoryBuffer. However, the service also adds another shared ownership edge with the GpuMemoryBuffer using the GMB id at the face_X/level_Y nested level. This adds edges at different nesting levels which causes double counting by memory-infra. This change removes the shared ownership edge when the GLImageIOSurface is anonymous, and replaces that with a nested "anonymous-iosurface" level. This gets rid of the double counting as the memory is charged to cc's tile or resource memory due to its higher importance. R=ericrk,ccameron BUG= 781490 , 780510 ,863121 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Change-Id: Id5b69e38b525d5bfdbb997fec2479b644cfae656 Reviewed-on: https://chromium-review.googlesource.com/1147674 Reviewed-by: ccameron <ccameron@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Reviewed-by: Eric Karl <ericrk@chromium.org> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Cr-Commit-Position: refs/heads/master@{#578021} [modify] https://crrev.com/4da52fed413909a85bb4e52fc7c82e3a62bec3df/gpu/command_buffer/service/texture_manager.cc [modify] https://crrev.com/4da52fed413909a85bb4e52fc7c82e3a62bec3df/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.cc [modify] https://crrev.com/4da52fed413909a85bb4e52fc7c82e3a62bec3df/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.h [modify] https://crrev.com/4da52fed413909a85bb4e52fc7c82e3a62bec3df/ui/gl/gl_image_io_surface.mm
,
Jul 25
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by primiano@chromium.org
, Nov 23 2017