Issue metadata
Sign in to add a comment
|
17kb regression in resource_sizes (MonochromePublic.apk) at 621014:621014 |
||||||||||||||||||||
Issue descriptionCaused by “gpu: Check that texture format matches sampler” Commit: 0fce45278dd0c9622984502bd7b427ae39dddd7b Link to size graph: https://chromeperf.appspot.com/report?sid=bb23072657e2d7ca892a1c3fa4643b1ee29b3a0a44d0732adda87168e89c0380&num_points=10&rev=621014 Link to trybot result: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/android-binary-size/130472 Debugging size regressions is documented at: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/apk_size_regressions.md#Debugging-Apk-Size-Increase Looks like increase is entirely from the introduction of: + 0) 13088 (114.5%) R@0x2fca9d0 13088 (0->13088) ui/gl/gl_enums.cc gl::enum_to_string_table Note there is also >1kb worth of relocations that were added due to all the string pointers in the symbol. It's unlikely this increase is warranted. Please have a look to try and reduce the overhead. It typically takes about a week of engineering time to reduce binary size by 50kb so we'd really appreciate you taking some time exploring options to address this regression!
,
Jan 11
Assigning to jdarpinian@chromium.org because this is the only CL in range: gpu: Check that texture format matches sampler This validation is required by the WebGL 2 spec and tested in conformance2/uniforms/incompatible-texture-type-for-sampler.html Bug: 809237 Change-Id: I288566e27ee17e4a38208f6ae8369dd40d31d8ef Reviewed-on: https://chromium-review.googlesource.com/c/1368832 Commit-Queue: James Darpinian <jdarpinian@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Cr-Commit-Position: refs/heads/master@{#621014}
,
Jan 11
I really think that this string table should have already been included in the binary. All I did was introduce a new call to GetStringEnum, but it is already called all over the place [1]. So why would one new call increase the binary size? Are we getting multiple copies of the string table? [1] https://cs.chromium.org/chromium/src/gpu/command_buffer/common/gles2_cmd_utils.cc?l=1572&gs=kythe%253A%252F%252Fchromium%253Flang%253Dc%25252B%25252B%253Fpath%253Dsrc%252Fgpu%252Fcommand_buffer%252Fcommon%252Fgles2_cmd_utils.cc%25235NYAuCTfy7EZnjf1YHUdRy0cjR8Icw4F%25252Bp8Z%25252FFmH7LQ%25253D&gsn=GetStringEnum&ct=xref_usages
,
Jan 15
Maybe your call is the first one that gets included on Android? The source path for they symbol is "ui/gl/gl_enums.cc", so I don't think it's likely that it's being included multiple times.
,
Jan 15
It looks like we have two versions of GetStringEnum, and I called the wrong one. I'm not sure if they're actually different, but they may be.
,
Jan 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9eb19884ede27f7080c79b9ea596d48bcdd96bf3 commit 9eb19884ede27f7080c79b9ea596d48bcdd96bf3 Author: James Darpinian <jdarpinian@chromium.org> Date: Wed Jan 16 02:06:55 2019 gpu: Fix calls to wrong version of GetStringEnum There are two versions of GetStringEnum, and calling the wrong one inflates binary size by embedding two versions of the string table. Bug: 921064 Change-Id: Ie96256904a23f7c16176188145a4482e01d519d5 Reviewed-on: https://chromium-review.googlesource.com/c/1409887 Commit-Queue: James Darpinian <jdarpinian@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Cr-Commit-Position: refs/heads/master@{#623029} [modify] https://crrev.com/9eb19884ede27f7080c79b9ea596d48bcdd96bf3/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/9eb19884ede27f7080c79b9ea596d48bcdd96bf3/gpu/command_buffer/service/texture_manager.cc
,
Jan 16
Regression is gone. Thanks for the fix! |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Jan 11