Issue metadata
Sign in to add a comment
|
40kb regression in resource_sizes (MonochromePublic.apk) at 497934:497934 |
||||||||||||||||||||
Issue descriptionCaused by “Add out of process gpu raster behind a flag” Commit: 436a77572ce1088aff858c11afe43c9198655c46 Link to size graph: https://chromeperf.appspot.com/report?sid=a097e74b1aa288511afb4cb616efe0f95ba4d347ad61d5e835072f23450938ba&num_points=10&rev=497934 Debugging size regressions is documented at: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/apk_size_regressions.md#Debugging-Apk-Size-Increase Based on the graph, increase is entirely from native code.
,
Aug 29 2017
Note: 40kb is somewhat sizeable. Please put in a bit of effort to verify that:
1) This code is required on Android
2) There is no easy way to make it smaller.
Attached the diff from diagnose_bloat.py below. From it, I can see:
* The top 10 symbols account for 35kb out of 40kb
* The first symbol is likely not needed on Android (which supports only GLES afaik)
Section Sizes (Total=39.6kb (40580 bytes)):
.bss: 0 bytes (0 bytes) (not included in totals)
.data: 0 bytes (0 bytes) (0.0%)
.data.rel.ro: 288 bytes (288 bytes) (0.7%)
.rel.dyn: -3.45kb (-3536 bytes) (-8.7%)
.rodata: 4.50kb (4608 bytes) (11.4%)
.text: 38.3kb (39216 bytes) (96.6%)
415 symbols added (+), 23 changed (~), 117 removed (-), 521111 unchanged (not shown)
Number of unique symbols 402054 -> 402180 (+126)
3 paths added, 0 removed, 17 changed
Showing 555 symbols (aliases not grouped for diffs) with total pss: 44112 bytes
.text=38.3kb .rodata=4.50kb .data.rel.ro=288 bytes .data=0 bytes .bss=0 bytes total=43.1kb
Number of unique paths: 31
Section Legend: t=.text, r=.rodata, R=.data.rel.ro, d=.data, b=.bss
Index | Running Total | Section@Address | Δ PSS (Δ size_without_padding) | Path
------------------------------------------------------------
+ 0) 14128 (31.9%) t@0x686480 +14128 (0->14128) third_party/skia/src/gpu/gl/GrGLAssembleInterface.cpp
GrGLAssembleGLInterface
+ 1) 24184 (54.6%) t@0x683d38 +10056 (0->10056) third_party/skia/src/gpu/gl/GrGLAssembleInterface.cpp
GrGLAssembleGLESInterface
~ 2) 28911 (65.3%) r@Group +4727 (2979763->2984490) {{no path}}
** merge strings (count=7)
+ 3) 30545 (69.0%) t@0x8800c2 +1634 (0->1634) cc/paint/paint_op_reader.cc
cc::PaintOpReader::Read
+ 4) 31797 (71.8%) t@0x880c52 +1252 (0->1252) cc/paint/paint_op_writer.cc
cc::PaintOpWriter::Write
+ 5) 32753 (74.0%) t@0x1891884 +956 (0->956) gpu/command_buffer/service/gles2_cmd_decoder.cc
gpu::gles2::GLES2DecoderImpl::DoBeginRasterCHROMIUM
~ 6) 33225 (75.0%) t@0x9fa954 +472 (528->1000) cc/raster/gpu_raster_buffer_provider.cc
cc::GpuRasterBufferProvider::PlaybackOnWorkerThread
+ 7) 33685 (76.1%) t@0x87fba4 +460 (0->460) cc/paint/paint_op_reader.cc
cc::PaintOpReader::Read
+ 8) 34121 (77.1%) t@0x880a9e +436 (0->436) cc/paint/paint_op_writer.cc
cc::PaintOpWriter::Write
+ 9) 34539 (78.0%) t@0x87f2ce +418 (0->418) cc/paint/paint_op_buffer.cc
cc::PaintOpBuffer::FlatteningIterator::FlattenCurrentOpIfNeeded
+ 10) 34895 (78.8%) t@0xa811e4 +356 (0->356) gpu/command_buffer/client/gles2_implementation.cc
gpu::gles2::GLES2Implementation::RasterCHROMIUM
+ 11) 35231 (79.6%) t@0x5332c8 +336 (0->336) third_party/skia/src/core/SkPathRef.cpp
SkPathRef::isValid const
+ 12) 35539 (80.3%) t@0x880724 +308 (0->308) cc/paint/paint_op_reader.cc
cc::PaintOpReader::Read
+ 13) 35837 (80.9%) t@0x1874700 +298 (0->296) gpu/command_buffer/service/gles2_cmd_decoder.cc
gpu::gles2::GLES2DecoderImpl::HandleRasterCHROMIUM
+ 14) 36093 (81.5%) t@0x87d5d4 +256 (0->256) cc/paint/paint_op_buffer.cc
cc::DrawImageRectOp::Deserialize
+ 15) 36341 (82.1%) t@0x881136 +248 (0->248) cc/paint/paint_op_writer.cc
cc::PaintOpWriter::Write
+ 16) 36571 (82.6%) t@0x2117788 +230 (0->228) content/renderer/render_thread_impl.cc
content::CreateOffscreenContext
- 17) 36349 (82.1%) t@0x0 -222 (220->0) content/renderer/render_thread_impl.cc
content::CreateOffscreenContext
~ 18) 36129 (81.6%) t@Group -220 (0->0) {{no path}}
** symbol gaps (count=9)
~ 19) 36337 (82.1%) t@0x1876c20 +208 (5600->5808) gpu/command_buffer/service/gles2_cmd_decoder.cc
gpu::gles2::GLES2DecoderImpl::Initialize
+ 20) 36525 (82.5%) t@0x87d230 +188 (0->188) cc/paint/paint_op_buffer.cc
cc::AnnotateOp::Deserialize
+ 21) 36709 (82.9%) t@0x87d2ec +184 (0->184) cc/paint/paint_op_buffer.cc
cc::ClipPathOp::Deserialize
+ 22) 36879 (83.3%) t@0x87fd70 +170 (0->170) cc/paint/paint_op_reader.cc
cc::PaintOpReader::ReadFlattenable
+ 23) 37049 (83.7%) t@0x87fe1a +170 (0->170) cc/paint/paint_op_reader.cc
cc::PaintOpReader::ReadFlattenable
+ 24) 37219 (84.0%) t@0x87fec4 +170 (0->170) cc/paint/paint_op_reader.cc
cc::PaintOpReader::ReadFlattenable
+ 25) 37389 (84.4%) t@0x87ff6e +170 (0->170) cc/paint/paint_op_reader.cc
cc::PaintOpReader::ReadFlattenable
+ 26) 37559 (84.8%) t@0x880018 +170 (0->170) cc/paint/paint_op_reader.cc
cc::PaintOpReader::ReadFlattenable
+ 27) 37729 (85.2%) t@0x87dad8 +170 (0->168) cc/paint/paint_op_buffer.cc
cc::SaveLayerOp::Deserialize
+ 28) 37897 (85.6%) t@0x9fb072 +168 (0->168) cc/raster/gpu_raster_buffer_provider.cc
cc::DisplayItemList::push (num_aliases=2)
+ 29) 38063 (86.0%) t@0x87d848 +166 (0->166) cc/paint/paint_op_buffer.cc
cc::DrawPathOp::Deserialize
+ 30) 38225 (86.3%) t@0x880858 +162 (0->162) cc/paint/paint_op_writer.cc
cc::PaintOpWriter::WriteFlattenable
+ 31) 38385 (86.7%) t@0x87d9ee +160 (0->160) cc/paint/paint_op_buffer.cc
cc::DrawTextBlobOp::Deserialize
+ 32) 38535 (87.0%) t@0x87d7b4 +150 (0->148) cc/paint/paint_op_buffer.cc
,
Aug 29 2017
vmiura: it looks like 60% of this is coming from GrGLAssembleInterface. Is there some alternative on Android that is less binary size, do you think? I don't really want to #ifdef this patch out on Android, since it would really only be a short term thing. Android seems one of the more important platforms for this work, especially with enabling Vulkan.
,
Aug 29 2017
Can we avoid linking in GrGLAssembleGLInterface on Android? Keep only GrGLAssembleGLESInterface? On the client side, Brian implemented CreateGLES2InterfaceBindings which was more efficient than going via GrGLAssembleGLESInterface, but now I guess we have both getting linked. Roughly an option is to refactor CreateGLES2InterfaceBindings to use GrGLAssembleGLESInterface, which means making a GetProc equivalent for GLES2Interface. A table based approach could take less space.
,
Aug 29 2017
We already look up most (all?) these functions in ui/gl/gl_bindings*, so there may be a cheaper way to do this, by accessing gl::g_current_gl_driver (which is a gl::DriverGL*, which has fields for all these functions). It would avoid the string lookups and calls to the GetProcAddress function, and even the extension/version tests (since we also do those in the bindings) - it "should" be possible to make it as small as copying N function pointers.
,
Aug 29 2017
CreateGLES2InterfaceBindings is only 4KB; adding a GetProc isn't likely to save anything so please ignore that suggestion. Implementing GrGLAssembleGLESInterface in terms of g_current_gl_driver should in turn get us to ~4KB.
,
Aug 29 2017
Let me take a look.
,
Aug 30 2017
Tentatively a quick hack shows that we should be able to save ~23kb with the approach from #5. I say tentatively because it doesn't fully work yet.
,
Sep 1 2017
https://chromium-review.googlesource.com/c/chromium/src/+/646957 is a somewhat principled attempts, with some caveats. It probably won't land before the branch, we can decide whether it's better to merge that or a trivial patch that disables the feature.
,
Sep 1 2017
,
Sep 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/148d0597cb0d72a73c3cc2e481396cb6b24cc535 commit 148d0597cb0d72a73c3cc2e481396cb6b24cc535 Author: Antoine Labour <piman@chromium.org> Date: Fri Sep 01 21:00:20 2017 Create GrGLInterface based on ui/gl's bindings instead of GrGLAssembleGLInterface GrGLAssembleGLInterface is a lot of code to lookup GL function pointers, which is somewhat duplicated in ui/gl's bindings. This saves about 21kb on Android. Bug: 760250 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 Change-Id: I906997ab8cc03db06a437d3428ca4263b13d2025 Reviewed-on: https://chromium-review.googlesource.com/646957 Reviewed-by: enne <enne@chromium.org> Reviewed-by: Zhenyao Mo <zmo@chromium.org> Commit-Queue: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#499293} [modify] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/gpu/command_buffer/service/BUILD.gn [add] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/gpu/command_buffer/service/create_gr_gl_interface.cc [add] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/gpu/command_buffer/service/create_gr_gl_interface.h [modify] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/ui/gl/generate_bindings.py [modify] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/ui/gl/gl_bindings_api_autogen_gl.h [modify] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/ui/gl/gl_bindings_autogen_gl.cc [modify] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/ui/gl/gl_bindings_autogen_gl.h [modify] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/ui/gl/gl_bindings_autogen_mock.cc [modify] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/ui/gl/gl_bindings_autogen_mock.h [modify] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/ui/gl/gl_mock_autogen_gl.h [modify] https://crrev.com/148d0597cb0d72a73c3cc2e481396cb6b24cc535/ui/gl/gl_stub_autogen_gl.h
,
Sep 2 2017
I have another indirectly related change that helps saving another 15k: https://chromium-review.googlesource.com/c/chromium/src/+/648244 It should also help us scale better as we add support for more extensions.
,
Sep 4 2017
This issue is marked as a release blocker with no OS labels associated. Please add an appropriate OS label. All release blocking issues should have OS labels associated to it, so that the issue can tracked and promptly verified, once it gets fixed. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/20d1620b86ae574a79cad73116d7c1f339ddabd7 commit 20d1620b86ae574a79cad73116d7c1f339ddabd7 Author: Antoine Labour <piman@chromium.org> Date: Tue Sep 05 23:05:29 2017 Introduce gl::ExtensionSet and simplify extension handling in GL bindings - ExtensionSet is just a flat_map of StringPiece's. After construction, we only look up into ExtensionSet, so a flat map is ideal. Using StringPiece avoids copies. - Remove GLContext::GetExtensions platform overrides. They used to add WGL/GLX/EGL extensions to the GL extension string, but we never test for them in that path (i.e. that's pure overhead). If we need to expose them in the future, a separate function would be preferable. - Have GLContext::GetExtensions compute, cache, and return the ExtensionSet. This avoids extraneous string manipulations (split/join) and copies. As a side effect, bindings now lookup the actual extension string as opposed to adding a space after it. This allows sharing the string with other places we test for those extensions. It also makes the bindings initialization code much more efficient. About 15Kb saved on Android. Bug: 760250 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 Change-Id: If830612ca7e23f705808c7b0908322671553122c Reviewed-on: https://chromium-review.googlesource.com/648244 Reviewed-by: Zhenyao Mo <zmo@chromium.org> Commit-Queue: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#499783} [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/gpu/command_buffer/service/feature_info.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/gpu/command_buffer/service/feature_info.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/gpu/command_buffer/service/gl_context_virtual.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/gpu/command_buffer/service/gl_context_virtual.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/gpu/command_buffer/service/shader_translator_unittest.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/gpu/command_buffer/service/test_helper.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/gpu/command_buffer/service/test_helper.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/gpu/command_buffer/service/texture_manager_unittest.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/gpu/config/gpu_info_collector.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/BUILD.gn [add] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/extension_set.cc [add] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/extension_set.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/generate_bindings.py [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_api_unittest.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_bindings.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_bindings_autogen_egl.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_bindings_autogen_gl.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_bindings_autogen_glx.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_bindings_autogen_osmesa.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_bindings_autogen_wgl.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_context.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_context.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_context_egl.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_context_egl.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_context_glx.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_context_glx.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_context_stub.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_context_stub.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_context_wgl.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_context_wgl.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_implementation.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_implementation.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_version_info.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_version_info.h [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/gl_version_info_unittest.cc [modify] https://crrev.com/20d1620b86ae574a79cad73116d7c1f339ddabd7/ui/gl/init/gl_factory_android.cc
,
Sep 8 2017
This issue is marked as a release blocker with no OS labels associated. Please add an appropriate OS label. All release blocking issues should have OS labels associated to it, so that the issue can tracked and promptly verified, once it gets fixed. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 11 2017
Requesting merge for https://chromium-review.googlesource.com/646957. It's mostly harmless because it essentially only touches a path that is disabled by default. We could consider https://chromium-review.googlesource.com/648244 too for extra savings, depending on appetite. It is more risky in that it touches production code. It landed safely though (AFAICT at this point).
,
Sep 11 2017
Please add appropriate OSs.
,
Sep 11 2017
,
Sep 12 2017
Your change meets the bar and is auto-approved for M62. Please go ahead and merge the CL to branch 3202 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd commit e1d02623bdb26e3ddc14b32879bf471bffcbe3cd Author: Antoine Labour <piman@chromium.org> Date: Tue Sep 12 18:46:36 2017 Create GrGLInterface based on ui/gl's bindings instead of GrGLAssembleGLInterface GrGLAssembleGLInterface is a lot of code to lookup GL function pointers, which is somewhat duplicated in ui/gl's bindings. This saves about 21kb on Android. Bug: 760250 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 Change-Id: I906997ab8cc03db06a437d3428ca4263b13d2025 Reviewed-on: https://chromium-review.googlesource.com/646957 Reviewed-by: enne <enne@chromium.org> Reviewed-by: Zhenyao Mo <zmo@chromium.org> Commit-Queue: Antoine Labour <piman@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#499293}(cherry picked from commit 148d0597cb0d72a73c3cc2e481396cb6b24cc535) Reviewed-on: https://chromium-review.googlesource.com/663669 Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#176} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/gpu/command_buffer/service/BUILD.gn [add] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/gpu/command_buffer/service/create_gr_gl_interface.cc [add] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/gpu/command_buffer/service/create_gr_gl_interface.h [modify] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/ui/gl/generate_bindings.py [modify] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/ui/gl/gl_bindings_api_autogen_gl.h [modify] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/ui/gl/gl_bindings_autogen_gl.cc [modify] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/ui/gl/gl_bindings_autogen_gl.h [modify] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/ui/gl/gl_bindings_autogen_mock.cc [modify] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/ui/gl/gl_bindings_autogen_mock.h [modify] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/ui/gl/gl_mock_autogen_gl.h [modify] https://crrev.com/e1d02623bdb26e3ddc14b32879bf471bffcbe3cd/ui/gl/gl_stub_autogen_gl.h
,
Sep 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4076bacedad5df5611ce7d7f26f164f46d3cbb8c commit 4076bacedad5df5611ce7d7f26f164f46d3cbb8c Author: Antoine Labour <piman@chromium.org> Date: Tue Sep 12 18:48:22 2017 Introduce gl::ExtensionSet and simplify extension handling in GL bindings - ExtensionSet is just a flat_map of StringPiece's. After construction, we only look up into ExtensionSet, so a flat map is ideal. Using StringPiece avoids copies. - Remove GLContext::GetExtensions platform overrides. They used to add WGL/GLX/EGL extensions to the GL extension string, but we never test for them in that path (i.e. that's pure overhead). If we need to expose them in the future, a separate function would be preferable. - Have GLContext::GetExtensions compute, cache, and return the ExtensionSet. This avoids extraneous string manipulations (split/join) and copies. As a side effect, bindings now lookup the actual extension string as opposed to adding a space after it. This allows sharing the string with other places we test for those extensions. It also makes the bindings initialization code much more efficient. About 15Kb saved on Android. Bug: 760250 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 Change-Id: If830612ca7e23f705808c7b0908322671553122c Reviewed-on: https://chromium-review.googlesource.com/648244 Reviewed-by: Zhenyao Mo <zmo@chromium.org> Commit-Queue: Antoine Labour <piman@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#499783}(cherry picked from commit 20d1620b86ae574a79cad73116d7c1f339ddabd7) Reviewed-on: https://chromium-review.googlesource.com/663883 Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#177} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/gpu/command_buffer/service/feature_info.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/gpu/command_buffer/service/feature_info.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/gpu/command_buffer/service/gl_context_virtual.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/gpu/command_buffer/service/gl_context_virtual.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/gpu/command_buffer/service/shader_translator_unittest.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/gpu/command_buffer/service/test_helper.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/gpu/command_buffer/service/test_helper.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/gpu/command_buffer/service/texture_manager_unittest.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/gpu/config/gpu_info_collector.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/BUILD.gn [add] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/extension_set.cc [add] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/extension_set.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/generate_bindings.py [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_api_unittest.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_bindings.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_bindings_autogen_egl.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_bindings_autogen_gl.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_bindings_autogen_glx.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_bindings_autogen_osmesa.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_bindings_autogen_wgl.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_context.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_context.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_context_egl.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_context_egl.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_context_glx.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_context_glx.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_context_stub.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_context_stub.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_context_wgl.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_context_wgl.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_implementation.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_implementation.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_version_info.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_version_info.h [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/gl_version_info_unittest.cc [modify] https://crrev.com/4076bacedad5df5611ce7d7f26f164f46d3cbb8c/ui/gl/init/gl_factory_android.cc
,
Sep 12 2017
Merged both patches per offline discussion with aluo@ |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Aug 29 2017