New issue
Advanced search Search tips

Issue 778067 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 760702
issue 779572



Sign in to add a comment

Reduce bloat caused by string literals

Project Member Reported by agrieve@chromium.org, Oct 25 2017

Issue description

Now that supersize has the ability to dump string literals, we should scan through them to look for potential optimizations.

Some strings that popped out to me when I was looking through them:
* Trying to call glActiveTexture() without current GL context
   * Generated by ui/gl/generate_bindings.py
* null replication_state field in Renderer_CreateFrameProxy_Params
   * Generated by .mojom
* invalid directory field in Directory_Clone_Params
   * Generated by .mojom
* StructTraits::RenderPass::Read
   * TRACE_EVENT0
* v8::Uint32Array::New
   * v8/src/api.cc (.rodata=11.2kb) - LOG_API() macro
   * Should test disabling LOG() macro here:
      * https://cs.chromium.org/chromium/src/v8/src/log.h
* not splitting #%d:%s, its common dominator id:%d is perfe
   * FLAG_trace_turbo_scheduler (.rodata=1874)
   * https://cs.chromium.org/chromium/src/v8/src/compiler/scheduler.cc
* V8.Runtime_Runtime_HasFixedFloat64Elements
   * v8/src/runtime/runtime-test.cc (.rodata=3.34kb)
   * TRACE_EVENT0 used by RUNTIME_FUNCTION macro
* kExprF64StoreMem
   * v8/src/wasm/function-body-decoder.cc (.rodata=4.51kb)
   * Used for debugging: --FLAG_trace_wasm_ast_start
* bool OT::ClassDef::sanitize(OT::hb_sanitize_context_t *) const
   * third_party/harfbuzz-ng/src/hb-ot-layout.cc (.rodata=58.6kb)
   * Caused by debug macros not being compiled out!
   * https://cs.chromium.org/chromium/src/third_party/harfbuzz-ng/src/hb-open-type-private.hh?rcl=1c9a9733195266afce1706a22de3bc740bd6fbf8&l=180
* Layout: Bad backtrack offset of %d for backtrack %d in chain context format 3
   * third_party/ots/src/layout.cc (.rodata=11.6kb)
   * all ots/src: .rodata=36.5kb
   * Opentype webfont errors.
   * Most from error messages, eventually shown in devtools console
* ../../third_party/webrtc/api/rtpreceiverinterface.h:133
   * RTC_FROM_HERE macro 
   * 363 Usages * 50 bytes each = 18kb


All v8 string literals:
 * Print(size_info.symbols.Filter(lambda s: s.IsStringLiteral()).WherePathMatches('v8/src'))
    * .rodata=371kb
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 26 2017

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

commit b234c50be29c46abcf8f5c22a4144ddf1989daff
Author: Andrew Grieve <agrieve@chromium.org>
Date: Thu Oct 26 16:28:07 2017

Tweak ui/gl/generate_bindings.py to shrink binary size

1: Use a helper function to log
   "Trying to call $method_name without current GL context"
2: Don't explicitly set fields to 0 (they all start as 0).

1: Shrinks libchrome.so on Android by: .text=-18.3kb .rodata=-25.2kb
2: Shrinks libchrome.so's .text by ~600 bytes

Bug:  778067 
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: I67b5e3c40b60631f3f881ada32fabff988097fd4
Reviewed-on: https://chromium-review.googlesource.com/737250
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511842}
[modify] https://crrev.com/b234c50be29c46abcf8f5c22a4144ddf1989daff/ui/gl/generate_bindings.py
[modify] https://crrev.com/b234c50be29c46abcf8f5c22a4144ddf1989daff/ui/gl/gl_bindings.h
[modify] https://crrev.com/b234c50be29c46abcf8f5c22a4144ddf1989daff/ui/gl/gl_bindings_autogen_egl.cc
[modify] https://crrev.com/b234c50be29c46abcf8f5c22a4144ddf1989daff/ui/gl/gl_bindings_autogen_gl.cc
[modify] https://crrev.com/b234c50be29c46abcf8f5c22a4144ddf1989daff/ui/gl/gl_bindings_autogen_glx.cc
[modify] https://crrev.com/b234c50be29c46abcf8f5c22a4144ddf1989daff/ui/gl/gl_bindings_autogen_osmesa.cc
[modify] https://crrev.com/b234c50be29c46abcf8f5c22a4144ddf1989daff/ui/gl/gl_bindings_autogen_wgl.cc

Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 27 2017

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

commit 9b4ca2e65d1efc2a7f4168dadb3f207c0cfc0513
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Oct 27 18:43:51 2017

Mojo: Show only field index in mojo ValidateNonNull() errors

Used to show object_name and field_name. However, object_name
can be found by looking at stack trace, and field_index is 
enough that you can look up which field it is.

Shrinks android binary by 160kb: .text=-56.1kb .rodata=-106kb

The .text reduction comes from more functions being
identical-code-folded.

Bug: 689690, 778067 
Change-Id: I9090e486935ad391d6c258d67278c352e36ea5c0
Reviewed-on: https://chromium-review.googlesource.com/738398
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512253}
[modify] https://crrev.com/9b4ca2e65d1efc2a7f4168dadb3f207c0cfc0513/mojo/public/cpp/bindings/lib/map_data_internal.h
[modify] https://crrev.com/9b4ca2e65d1efc2a7f4168dadb3f207c0cfc0513/mojo/public/cpp/bindings/lib/message_header_validator.cc
[modify] https://crrev.com/9b4ca2e65d1efc2a7f4168dadb3f207c0cfc0513/mojo/public/cpp/bindings/lib/validation_util.cc
[modify] https://crrev.com/9b4ca2e65d1efc2a7f4168dadb3f207c0cfc0513/mojo/public/cpp/bindings/lib/validation_util.h
[modify] https://crrev.com/9b4ca2e65d1efc2a7f4168dadb3f207c0cfc0513/mojo/public/tools/bindings/generators/cpp_templates/struct_definition.tmpl
[modify] https://crrev.com/9b4ca2e65d1efc2a7f4168dadb3f207c0cfc0513/mojo/public/tools/bindings/generators/cpp_templates/union_definition.tmpl
[modify] https://crrev.com/9b4ca2e65d1efc2a7f4168dadb3f207c0cfc0513/mojo/public/tools/bindings/generators/cpp_templates/validation_macros.tmpl

Blockedon: 779572
Blockedon: 760702
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 30 2017

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

commit e0c60a1bb53b444f54109772ca96b654a7b681f0
Author: Andrew Grieve <agrieve@chromium.org>
Date: Mon Oct 30 20:21:50 2017

Fix field_index in error message for validation of message header

Bug:  778067 
Change-Id: Ib61c658583e4f89ed94af229ded597baa4ee1887
Reviewed-on: https://chromium-review.googlesource.com/744341
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512600}
[modify] https://crrev.com/e0c60a1bb53b444f54109772ca96b654a7b681f0/mojo/public/cpp/bindings/lib/message_header_validator.cc

Tried stubbing out v8's LOG macro, but it saved only 12kb. Not worth pursuing.
Status: Fixed (was: Assigned)
Nothing else actionable here. Closing

Sign in to add a comment