New issue
Advanced search Search tips

Issue 846385 link

Starred by 2 users
Status: Verified
Owner:
andre.r...@lge.com
Closed: Aug 13
Cc:
piman@chromium.org
zmo@chromium.org
dsinclair@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Task



Sign in to add a comment

Cleanup shader destruction path leading to glDeleteShader call

Project Member Reported by andre.r...@lge.com, May 24 2018 
shader_manager.cc has a deletion path which is not symmetric to program_manager.cc . And functions managing the use count and marking for deletion do more than what they are named for, like calling glDeleteShader indirectly.

It was noticed when debugging a shader leak in upstream mesa and following the code from chromium side.
Project Member

Comment 1 by bugdroid1@chromium.org, May 24 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8f4bd8bc82e2b7d0c577ec1cc545315bdf15b41

commit f8f4bd8bc82e2b7d0c577ec1cc545315bdf15b41
Author: Andre Goddard Rosa <andre.rosa@lge.com>
Date: Thu May 24 18:36:10 2018

Don't hide releasing shader resources in usage count

- Makes implementation symmetric to program manager
- DecUseCount() was not only doing what's named after
- Also decreases binary size:
          text    data     bss     dec     hex filename
before 6883184  193376    2012 7078572  6c02ac out/Release/libgles2.so
after  6882668  193360    2012 7078040  6c0098 out/Release/libgles2.so

R=piman@chromium.org

Bug:  846385 
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: I51fc0b37ab1f59e4fc54fb11b43cc480209a0479
Reviewed-on: https://chromium-review.googlesource.com/1071009
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Andre Goddard Rosa <andre.rosa@lge.com>
Cr-Commit-Position: refs/heads/master@{#561564}
[modify] https://crrev.com/f8f4bd8bc82e2b7d0c577ec1cc545315bdf15b41/gpu/command_buffer/service/shader_manager.cc
[modify] https://crrev.com/f8f4bd8bc82e2b7d0c577ec1cc545315bdf15b41/gpu/command_buffer/service/shader_manager.h

Comment 2 by weiliangc@chromium.org, Aug 10

Cc: zmo@chromium.org piman@chromium.org
Owner: andre.r...@lge.com
Status: Assigned (was: Untriaged)
Is there more work needed or is this done?

Comment 3 by andre.r...@lge.com, Aug 13

Status: Verified (was: Assigned)
This is done, thanks.
►

Sign in to add a comment