DRAFT: TV Casting App - Add size-optimized Android build variant#43592
DRAFT: TV Casting App - Add size-optimized Android build variant#43592chrisdecenzo wants to merge 4 commits intoproject-chip:masterfrom
Conversation
Add a 'size-optimized' build modifier for the Android tv-casting-app
that reduces APK size through multiple layers of optimization:
- GN: exclude legacy chip-tool sources and heavy deps (tracing, jsoncpp)
when optimize_apk_size=true via new tv-casting-common.gni flag
- GN: enable -Os, -ffunction-sections, -fdata-sections, -flto=thin for
aggressive dead-code elimination by the linker
- GN: add tracing_noop.cpp stub to satisfy link deps without pulling in
the full tracing framework
- android.py: pass optimize_apk_size, is_debug=false, static libc++,
and disable tracing as GN build args; skip libc++_shared.so copy
- Gradle: enable R8 shrinking in debug builds when optimizeApkSize flag
is set; handle TvCastingApp.jar as compileOnly to avoid duplicate
class errors with R8
- ProGuard: add rules to keep JNI-referenced and Matter SDK classes
Build with:
./scripts/build/build_examples.py \
--target android-x86-tv-casting-app-size-optimized build
There was a problem hiding this comment.
Code Review
This pull request introduces a 'size-optimized' build variant for the Android tv-casting-app, which is a significant and well-executed improvement. The optimizations are applied at multiple levels: conditionally compiling out legacy C++ sources and heavy dependencies via GN build flags, enabling aggressive compiler and linker optimizations like LTO and dead code stripping, and enabling R8 shrinking in Gradle for Java/Kotlin code. The changes are logical, well-commented, and effectively reduce the APK size. I have a couple of suggestions for further improvement and code cleanup.
Replace the monolithic generated cluster-objects.cpp (~144 clusters) with a casting-specific version that only includes the ~36 clusters a casting client actually needs. This significantly reduces libClusterObjects size on Linux and Darwin builds. Changes: - Add casting-cluster-objects.cpp with casting-specific and infrastructure clusters only (excludes DoorLock, HVAC, appliance clusters, etc.) - Add chip_cluster_objects_source_override build arg to common_flags.gni to allow applications to substitute their own cluster-objects source - Update src/app/common/BUILD.gn to conditionally compile the override - Set the override in linux/args.gni and darwin/args.gni - Android is excluded because its Java controller TLV decoders reference Decode() methods for all clusters; --gc-sections + LTO handle stripping
- Remove tracing_noop.cpp stub per andy31415 feedback; guard call sites in AndroidChipPlatform-JNI.cpp with #if MATTER_TRACING_ENABLED instead of maintaining empty placeholder functions - Remove duplicate -ffunction-sections/-fdata-sections/-flto=thin cflags from android/BUILD.gn jni target; these are already inherited from tv-casting-common's public config - Add -fvisibility=hidden to size-optimized cflags to prevent internal C++ symbols from being exported in the .so symbol table; JNIEXPORT already marks JNI functions with visibility(default) - Add JNIEXPORT to JNI_OnLoad/JNI_OnUnload in CastingApp-JNI.cpp to ensure they remain visible with -fvisibility=hidden
|
PR #43592: Size comparison from 0d30044 to 09e495a Full report (12 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, realtek, stm32)
|
| # Optional path to a replacement cluster-objects.cpp source file. | ||
| # When set, the cluster-objects static library compiles this file instead | ||
| # of the full generated cluster-objects.cpp, allowing applications to | ||
| # include only the cluster implementations they actually need. |
There was a problem hiding this comment.
Would LTO not optimize things out?
As a concept we should indeed only include things that we need/want, however this seems a bit more hacky on the build system.
The way I would understand this is that devices / full apps would do LTO and cluster-objects is fine. However if we compile libchip in a shared library like android, then we cannot LTO and get all the clusters, which if this is an android app as opposed to a full controller supporting everything, it is undesireable. Is this what is going on?
I wonder if instead we should have a bool include-cluster-serialization or something like that instead of overrides. Is that feasable and apps would compile the things they need themselves? Or does that cause other problems?
|
|
||
| deps = [ "${chip_root}/src/lib/support" ] | ||
|
|
||
| if (matter_enable_tracing_support) { |
There was a problem hiding this comment.
It seems odd that we have a tracing target without tracing enabled, just exposing the header.
Could we have whenever this is depended on to have the if there? I imagine it would be something like:
if (matter_enable_tracing_support) {
deps += [ "${chip_root}/src/platform/androdi:tracing" ]
}
That would seem clearer rather than one being able to depend on tracing and not actually have tracing enabled/compiled in.
| use_static_libcxx = false | ||
| } | ||
|
|
||
| # The slim cluster-objects override is set per-platform in each args.gni. |
There was a problem hiding this comment.
This comment feels a bit misplaced
Summary
Add a 'size-optimized' build modifier for the Android tv-casting-app that reduces APK size through multiple layers of optimization:
Build with:
./scripts/build/build_examples.py \ --target android-x86-tv-casting-app-size-optimized build
Testing
regression testing via phone app