Many constants in .data that should go in .data.rel.ro |
|||||||
Issue descriptionDiscovered that model_prefixes here: https://cs.chromium.org/chromium/src/media/base/android/media_codec_util.cc?type=cs&q=IsSurfaceViewOutputSupported&sq=package:chromium&l=348 const char* model_prefixes[] = {// Exynos 4 (Mali-400) "GT-I9300", "GT-I9305", "SHV-E210", // Snapdragon S4 (Adreno-225) "SCH-I535", "SCH-J201", "SCH-R530", "SCH-I960", "SCH-S968", "SGH-T999", "SGH-I747", "SGH-N064", 0}; is defined in .data: >>> Print(size_info.symbols.WhereFullNameMatches('model_prefixes')) Showing 1 symbols (1 unique) with total pss: 48 bytes .text=0 bytes .rodata=0 bytes .data*=48 bytes .bss=0 bytes total=48 bytes Number of unique paths: 1 Index | Running Total | Section@Address | PSS | Path ------------------------------------------------------------ 0) 48 (100.0%) d@0x2cd23b0 48 media/base/android/media_codec_util.cc .L_ZZN5media14MediaCodecUtil28IsSurfaceViewOutputSupportedEvE14model_prefixes None of these changes moved it to rodata: "const char* const", "static const char* const", "static constexpr const char* const", d Here's a codesearch showing we have 540 such arrays of strings, and the ones I spot-checked are all in .data rather than .rodata. https://cs.chromium.org/search/?q=const%5C+char%5C*%5C+%5Ba-zA-Z0-9_%5D%2B%5C%5B%5C%5D%5C+%3D%5C+%7B+-file:third_party&sq=package:chromium&type=cs Using supersize on libmonochrome.so, searching for kFoo names show that many "constants" that are not in .rodata: >>> len(size_info.symbols.WhereFullNameMatches(r'\bk[A-Z]').WhereInSection('r')) 10829 >>> len(size_info.symbols.WhereFullNameMatches(r'\bk[A-Z]').WhereInSection('d')) 3086 Figuring out how to move these into .rodata should cause a bit less per-process RAM to be used.
,
Jul 22 2017
Re-opening with tweaked mission (now that I understand things a bit better)
,
Jul 28 2017
Forgot to tag commit: https://chromium-review.googlesource.com/c/581908/13
,
Jul 28 2017
Another thought about this is that we could add style check for kConstant is const https://chromium.googlesource.com/chromium/src/+/master/docs/writing_clang_plugins.md
,
Aug 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/26622d3ff8dcbddd6027a148a4be42d7c0de60e1 commit 26622d3ff8dcbddd6027a148a4be42d7c0de60e1 Author: agrieve <agrieve@chromium.org> Date: Tue Aug 08 17:48:15 2017 Audit of kConstants missing the const qualifier Found via supersize query: size_info.symbols.WhereFullNameMatches(r'\bk[A-Z]').WhereInSection('d') This moves 90 symbols from .data -> .data.rel.ro (5.50kb) BUG=chromium:747064 Review-Url: https://codereview.webrtc.org/2986163002 Cr-Commit-Position: refs/heads/master@{#19274} [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/api/stats/rtcstats_objects.h [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/config.cc [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/config.h [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/media/engine/webrtcmediaengine.cc [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/audio_coding/codecs/isac/fix/source/arith_routines_hist.c [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/audio_coding/codecs/isac/fix/source/arith_routins.h [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/audio_coding/codecs/isac/fix/source/entropy_coding.c [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/audio_coding/codecs/isac/main/source/arith_routines.h [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/audio_coding/codecs/isac/main/source/arith_routines_hist.c [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/audio_coding/codecs/isac/main/source/entropy_coding.c [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/audio_processing/level_controller/noise_spectrum_estimator.cc [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/pacing/alr_detector.cc [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/pacing/alr_detector.h [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/remote_bitrate_estimator/bwe_defines.cc [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/rtp_rtcp/source/fec_private_tables_bursty.h [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/rtp_rtcp/source/fec_private_tables_random.h [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/rtp_rtcp/source/forward_error_correction_internal.cc [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/modules/rtp_rtcp/source/forward_error_correction_internal.h [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/rtc_base/rtccertificategenerator.cc [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/rtc_base/sslroots.h [modify] https://crrev.com/26622d3ff8dcbddd6027a148a4be42d7c0de60e1/webrtc/stats/rtcstats_objects.cc
,
Aug 10 2017
,
Aug 10 2017
Issue 249746 has been merged into this issue.
,
Aug 24 2017
This is now done enough :)
>>> Print(size_info.symbols.WhereFullNameMatches(r'\bk[A-Z]').WhereInSection('d').Sorted())
Showing 15 symbols (15 unique) with total pss: 356 bytes
.data=356 bytes total=356 bytes
Number of unique paths: 10
Section Legend: d=.data
Index | Running Total | Section@Address | PSS | Path
------------------------------------------------------------
0) 168 (47.2%) d@0x2eadc08 168 third_party/brotli/common/dictionary.c
kBrotliDictionary
1) 204 (57.3%) d@0x2ea85a4 36 third_party/skia/src/gpu/ccpr/GrCCPRPathProcessor.cpp
kOctoIndices
2) 220 (61.8%) d@0x2eadce0 16 third_party/libyuv/source/scale_neon.cc
libyuv::kMult38_Div6
3) 236 (66.3%) d@0x2eadd00 16 third_party/libyuv/source/scale_neon.cc
libyuv::kMult38_Div9
4) 252 (70.8%) d@0x2eadcd0 16 third_party/libyuv/source/scale_neon.cc
libyuv::kShuf38
5) 268 (75.3%) d@0x2eadcf0 16 third_party/libyuv/source/scale_neon.cc
libyuv::kShuf38_2
6) 284 (79.8%) d@0x2eadcb0 16 third_party/libyuv/source/rotate_neon.cc
libyuv::kVTbl4x4Transpose
7) 300 (84.3%) d@0x2eadcc0 16 third_party/libyuv/source/rotate_neon.cc
libyuv::kVTbl4x4TransposeDi
8) 312 (87.6%) d@0x2ead1f8 12 third_party/WebKit/Source/platform/json/JSONValues.cpp
blink::kJSONNullString
9) 324 (91.0%) d@0x2eaf538 12 third_party/angle/src/third_party/compiler/ArrayBoundsClamper.cpp
kIntClampBegin
10) 336 (94.4%) d@0x2eaa3f8 12 storage/browser/quota/quota_manager.cc
storage::QuotaManager::kSyncableStorageDefaultHostQuota
11) 344 (96.6%) d@0x2eb0b64 8 third_party/gvr-android-sdk/libgvr_shim_static_arm.a/{shared}/2
kSystemClassPrefixes
12) 348 (97.8%) d@0x2ea85c8 4 third_party/skia/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
GrGLSLFragmentShaderBuilder::kDstColorName
13) 352 (98.9%) d@0x2eb09e0 4 chrome/browser/ssl/mitm_software_blocking_page.cc
MITMSoftwareBlockingPage::kTypeForTesting
14) 356 (100.0%) d@0x2eb0ac0 4 components/omnibox/browser/omnibox_field_trial.cc
OmniboxFieldTrial::kDefaultMinimumTimeBetweenSuggestQueriesMs
,
Jan 19
(4 days ago)
Re-opening, as this has regressed (there are 1007 symbols now). Good news is that once this is fixed, it should not regress again thanks to bug 888863 .
,
Yesterday
(44 hours ago)
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by agrieve@chromium.org
, Jul 20 2017Looks like the answer here is that thanks to -fPIC, all pointers must be in .data rather than .rodata. This applies to arrays of char*s, and also arrays of structs that contain pointers, such as: Print(size_info.symbols.WhereFullNameMatches(r'\bk[A-Z]').WhereInSection('d').Sorted()) 0) 22800 (10.7%) d@0x2c9cff0 22800 third_party/boringssl/src/crypto/obj/obj.c kObjects 1) 45004 (21.2%) d@0x2d0bc10 22204 v8/src/runtime/runtime.cc v8::internal::kIntrinsicFunctions 2) 53452 (25.2%) d@0x2ca5d78 8448 gpu/config/gpu_driver_bug_list_autogen.cc gpu::kGpuDriverBugListEntries 3) 59044 (27.8%) d@0x2d612a8 5592 components/autofill/core/browser/address_rewriter_rules.cc autofill::internal::kRules_US 4) 64612 (30.4%) d@0x2d7fc30 5568 blink/core/HTMLNames.cpp blink::HTMLNames::init::kNames 5) 68884 (32.4%) d@0x2cffca8 4272 v8/src/counters.cc v8::internal::Counters::Counters::kStatsCounters 6) 73088 (34.4%) d@0x2d00d58 4204 v8/src/counters.cc v8::internal::RuntimeCallStats::RuntimeCallStats::kNames 7) 76980 (36.3%) d@0x2daa0ec 3892 third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp blink::InternalCommand::kEditorCommands 8) 80812 (38.1%) d@0x2d20f98 3832 components/domain_reliability/google_configs.cc domain_reliability::kGoogleConfigs 9) 84376 (39.7%) d@0x2d8202c 3564 blink/core/SVGNames.cpp blink::SVGNames::init::kNames 10) 87808 (41.4%) d@0x2d7e4ec 3432 blink/core/EventTypeNames.cpp blink::EventTypeNames::init::kNames It's possible to optimize on a case-by-case by merging all strings and using offsets rather than pointers, but we're talking ~200kb. Won't Fix.