feat(tv-casting-app): Phase 2 APK size reduction (2.8 MB → 2.6 MB)#71482
Open
chrisdecenzo wants to merge 3 commits intoproject-chip:masterfrom
Open
feat(tv-casting-app): Phase 2 APK size reduction (2.8 MB → 2.6 MB)#71482chrisdecenzo wants to merge 3 commits intoproject-chip:masterfrom
chrisdecenzo wants to merge 3 commits intoproject-chip:masterfrom
Conversation
Remove 10 non-essential infrastructure clusters from the slim casting-cluster-objects.cpp and disable their server-side definitions in tv-casting-app.zap and tv-casting-app.matter. Disable extra data-model logging, host unit-test hooks, and RTTI for size-optimized builds. Make ICD client dependencies conditional on optimize_apk_size. Cluster changes: - Remove .ipp includes for 10 clusters: EthernetNetworkDiagnostics, SoftwareDiagnostics, WiFiNetworkDiagnostics, TimeSynchronization, TimeFormatLocalization, UnitLocalization, FixedLabel, UserLabel, IcdManagement, LocalizationConfiguration - Retain Groupcast (required by InteractionModelEngine.cpp) - Remove orphaned CommandIsFabricScoped entries for TimeSynchronization and IcdManagement - Disable server-side ZAP/matter definitions for removed clusters Build flag changes (optimize_apk_size=true only): - chip_data_model_extra_logging=false via android.py - enable_rtti=false via android.py and args.gni - CONFIG_BUILD_FOR_HOST_UNIT_TEST=0 via BUILD.gn defines - ICD client deps excluded via BUILD.gn conditional - Add src/controller as direct dep (was transitive via ICD) Measured results (arm64-v8a): - Android: libTvCastingApp.so 2.8 MB -> 2.6 MB (-7%) - Darwin: __TEXT 2.8 MB -> 2.7 MB (cluster trimming only)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements Phase 2 size optimizations for the TV casting app, including the removal of ten non-essential infrastructure clusters, disabling RTTI and extra data-model logging for Android, and excluding ICD client dependencies and host unit-test hooks. The documentation in the size analysis files incorrectly lists 11 removed clusters; feedback suggests correcting this count to 10 to align with the actual code changes.
Comment on lines
+349
to
+364
| The following 11 infrastructure clusters were removed from the slim cluster file | ||
| and disabled in the ZAP server-side configuration. A casting client does not | ||
| serve these clusters — they are diagnostic, localization, and management clusters | ||
| irrelevant to the casting use case: | ||
|
|
||
| - EthernetNetworkDiagnostics | ||
| - SoftwareDiagnostics | ||
| - WiFiNetworkDiagnostics | ||
| - TimeSynchronization | ||
| - TimeFormatLocalization | ||
| - UnitLocalization | ||
| - FixedLabel | ||
| - UserLabel | ||
| - Groupcast | ||
| - IcdManagement | ||
| - LocalizationConfiguration |
Contributor
There was a problem hiding this comment.
This section incorrectly states that 11 clusters were removed, including Groupcast. According to the PR description and code changes, Groupcast is intentionally retained, and only 10 clusters are removed. Please update this documentation to be consistent. The count on line 377 should also be corrected from 11 to 10.
References
- When documenting technical terms or component names, ensure consistency in capitalization and spelling throughout the document, especially when referring to names defined in source code.
|
|
||
| | Optimization | Darwin applicability | Notes | | ||
| |---|---|---| | ||
| | Cluster trimming (11 clusters) | ✅ Applies | The slim `casting-cluster-objects.cpp` is shared between Android and Darwin. Removing 11 non-essential infrastructure clusters reduces code in `libTvCastingCommon.a` identically. | |
Contributor
There was a problem hiding this comment.
This should be 'Cluster trimming (10 clusters)' and the notes should refer to 10 removed clusters, not 11, to be consistent with the code changes.
References
- When documenting technical terms or component names, ensure consistency in capitalization and spelling throughout the document, especially when referring to names defined in source code.
…ssors Add explicit build scoping for the TV Casting App size-optimized build: - chip_cluster_server_source_override: compile only 13 cluster server dirs (12 commissioning + content-app-observer) instead of all ~44 discovered from the .matter file - chip_accessors_source_override: compile only 29 casting-relevant cluster accessors instead of the full ~200+ cluster Accessors.cpp - casting-cluster-servers.gni: override list for cluster servers - casting-Accessors.cpp: slim accessors for 29 clusters Both overrides follow the same pattern as the existing chip_cluster_objects_source_override from phase 2. All changes are gated behind optimize_apk_size=true; default builds are unaffected. Measured result: libTvCastingApp.so remains at 2.6 MB (arm64-v8a) because LTO and --gc-sections were already stripping the excluded code. The overrides reduce compile time and intermediate .o sizes, and prevent regressions if linker behavior changes upstream. Also documents jsoncpp/jsontlv removal as not feasible (JNI interaction model layer uses it at runtime) and controller library split as future work (~1.3 MB potential, requires BUILD.gn refactor).
|
PR #71482: Size comparison from f82be95 to 1b78a4e Full report (5 builds for cc32xx, realtek, stm32)
|
|
PR #71482: Size comparison from f82be95 to 53a0e72 Full report (5 builds for cc32xx, realtek, stm32)
|
…el_overrides_dir Replace chip_cluster_objects_source_override, chip_cluster_server_source_override, chip_accessors_source_override, chip_tlv_decoder_attribute_source_override, chip_tlv_decoder_event_source_override, and matter_enable_tlv_decoder_cpp with a single chip_data_model_overrides_dir directory-based argument. When set to a non-empty path, the build system looks for well-known filenames (casting-cluster-objects.cpp, casting-Accessors.cpp, casting-cluster-servers.gni, casting-CHIPAttributeTLVValueDecoder.cpp, casting-CHIPEventTLVValueDecoder.cpp) inside the directory and uses them as source overrides. When empty (default), all consumers fall back to the full zap-generated sources. Also adds generate_slim_overrides.py and casting-cluster-config.yaml for regenerating slim override files from full zap-generated sources. Updates android.py, both args.gni files, all GN consumers, docs, and property tests. 76/76 tests pass.
chrisdecenzo
force-pushed
the
casting-apk-size-reduction-phase2
branch
from
53a0e72 to
8c748c3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remove 10 non-essential infrastructure clusters from the slim casting-cluster-objects.cpp and disable their server-side definitions in tv-casting-app.zap and tv-casting-app.matter. Disable extra data-model logging, host unit-test hooks, and RTTI for size-optimized builds. Make ICD client dependencies conditional on optimize_apk_size.
Cluster changes:
Build flag changes (optimize_apk_size=true only):
Measured results (arm64-v8a):
Testing
iOS and Android regression