Maybe there is a better way to find unused resources |
||||||
Issue descriptionCurrently we rely on the compile warning to find out unused resources, search "enable_resource_whitelist_generation" to find out the detail, During the review of https://codereview.chromium.org/2343083003/diff/60001/android_webview/BUILD.gn , agrieve@ found out the resource defined in android_webview/ui/grit_components_whitelist.txt doesn't match with generated whilelist. We should find out whether there is better way to find unused resources.
,
Sep 20 2017
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 14 2018
,
Feb 14 2018
,
Aug 3
2 wouldn't work because some of the constants appear in integral-constant-expression contexts (e.g. cases in switch statements).
Here's another idea. It relies on this function template definition:
template <int X> __attribute__((used)) constexpr int magic_int() {
return X;
}
Because of the used attribute, this function template causes debug info to be written to the output file whenever it is instantiated. (It also causes a function definition to be written, but they will be removed by --gc-sections -- debug info is immune to --gc-sections.) Because it is constexpr, it can be used in an integral-constant-expression. The definitions of resource ids look like this:
#define IDS_FOO magic_int<123>()
Then we can find all the valid resource ids by scanning the debug info in the unstripped .so file for functions named magic_int<X>.
I implemented the debug info embedding part of this here: https://chromium-review.googlesource.com/c/chromium/src/+/1162699
I used this command to create a whitelist:
$ strings -a o/a/lib.unstripped/libmonochrome.so o/a/android_clang_arm/lib.unstripped/libmonochrome.so | grep '^magic_int<' | grep -o '[0-9]\+' | sort -n | uniq > /tmp/whitelist
and verified that it was almost the same as the one created using the existing pipeline (except for IDS_VERSION_UI_64BIT, which the existing pipeline adds manually).
,
Aug 3
If we wanted this to work well with --gc-sections (i.e. so we could drop resources referred to by gc'd functions), I think we'd need some sort of compiler extension. The extension would allow us to write something conceptually like magic_int, except that as a side effect of instantiating the template the compiler would emit a global variable whose value is the resource id in a section which would be classified as debug info (so that it can be stripped) and associated [1] with the function or global variable that instantiated the template. (If the template instantiation appears in multiple functions or global variables, we would need an associated section for each one.) Then we can discover the whitelist by reading the section in the unstripped binary. [1] https://llvm.org/docs/LangRef.html#associated-metadata
,
Aug 4
That's awesome that you got a prototype to work! We can expand to a clang plugin in the future if needed, but #5 being simple and achieving the same result as now seems like a clear win!
,
Aug 14
https://chromium-review.googlesource.com/c/chromium/src/+/1174012 implements the rest of c#5.
,
Aug 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f26ebd65019e400184adad499c73264140c8fa96 commit f26ebd65019e400184adad499c73264140c8fa96 Author: Peter Collingbourne <pcc@chromium.org> Date: Fri Aug 17 18:24:22 2018 grit: Replace current resource whitelisting implementation with a better one. The new implementation works by embedding debug info in object files instead of parsing compiler warning output. As a result, our official builds are much less noisy. See the new header file ui/base/resource/whitelist.h for details on how it works. It is also simpler because it does not rely on wrapper scripts for collecting the set of whitelisted resource ids. As a result we no longer need, and can remove, the wrapper scripts for the compiler and ar. It is also theoretically more precise because it only whitelists resource ids mentioned in archive members that were chosen by the linker, although I didn't see an improvement in monochrome. It may be possible to gain even more precision by extending the compiler (see comments on bug) but first we'd need to investigate whether that would be worth it. Bug: 648475, 684788 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I12d83a8aaffa42c3afcdcc834f7dd7536b7a9248 Reviewed-on: https://chromium-review.googlesource.com/1174012 Reviewed-by: Eugene But <eugenebut@chromium.org> Reviewed-by: Richard Coles <torne@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Commit-Queue: Peter Collingbourne <pcc@chromium.org> Cr-Commit-Position: refs/heads/master@{#584129} [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/android_webview/BUILD.gn [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/build/android/resource_sizes.py [delete] https://crrev.com/a8ae8cd395ac425d4a521cbbec134c95c3e0e2f9/build/toolchain/gcc_ar_wrapper.py [delete] https://crrev.com/a8ae8cd395ac425d4a521cbbec134c95c3e0e2f9/build/toolchain/gcc_compile_wrapper.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/build/toolchain/gcc_solink_wrapper.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/build/toolchain/gcc_toolchain.gni [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/build/toolchain/nacl/BUILD.gn [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/build/toolchain/wrapper_utils.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/chrome/BUILD.gn [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/chrome/android/BUILD.gn [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/chrome/browser/ui/webui/settings/about_handler.cc [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/chrome/browser/ui/webui/version_ui.cc [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/components/variations/service/generate_ui_string_overrider.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/ios/chrome/browser/ui/webui/version_ui.mm [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/tools/grit/grit/format/gen_predetermined_ids.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/tools/grit/grit/format/gen_predetermined_ids_unittest.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/tools/grit/grit/format/rc_header.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/tools/grit/grit/format/rc_header_unittest.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/tools/grit/grit/node/misc.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/tools/grit/grit/node/misc_unittest.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/tools/grit/grit/tool/build.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/tools/grit/grit_rule.gni [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/tools/mb/mb_config.pyl [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/tools/resources/generate_resource_whitelist.gni [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/tools/resources/generate_resource_whitelist.py [modify] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/ui/base/BUILD.gn [add] https://crrev.com/f26ebd65019e400184adad499c73264140c8fa96/ui/base/resource/whitelist.h
,
Aug 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/069256a2001d1abc491d4a89e594069057351ca4 commit 069256a2001d1abc491d4a89e594069057351ca4 Author: agrieve <agrieve@chromium.org> Date: Fri Aug 24 05:06:28 2018 diagnose_bloat.py: Remove symbol_level=0 now that it's required Bug: 648475 Change-Id: Ibbafd96a020850881c0c515e0e4dd71ee530c099 Reviewed-on: https://chromium-review.googlesource.com/1187922 Reviewed-by: Peter Collingbourne <pcc@chromium.org> Commit-Queue: agrieve <agrieve@chromium.org> Cr-Commit-Position: refs/heads/master@{#585682} [modify] https://crrev.com/069256a2001d1abc491d4a89e594069057351ca4/tools/binary_size/diagnose_bloat.py |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by agrieve@chromium.org
, Sep 20 2016Gradle has a "shrink_resources" compile step that looks through all integer constants that appear in the proguarded classes.dex, as well as in res/xml and AndroidManifest.xml, and uses these as its whitelist. It's possible that we could do something similar on the native side. Ideas: #1 - Look for all constants within .data and .bss and interpret them as whitelist entries. #2 - Have the #define's map to actual constants in a .cc file, and annotate the constants with __attribute__ ((section ("pak_constant_#####"))). Use --print-gc-sections to create a pak file blacklist. #3 - Similar to #2, but have all constants in the same section. Use --relocatable to do a partial link and then read out the section values. #4 - See if there's anything in the generated DWARF debug information that could be used to see which constants were removed.