ADFA-3604: Re-enable R8 shrinking with keep rules for kotlin-analysis-api - #1596
Conversation
…-api Removes the blanket -dontshrink added in ADFA-996, which neutered R8 shrinking entirely after it broke release-build project init. Adds targeted keep rules for every class the bundled kotlin-analysis-api jar loads reflectively via IntelliJ plugin XML descriptors and ServiceLoader entries, since those are the paths R8 can't trace from bytecode reachability alone. Cuts release dex from 119.4 MB / 104,033 classes to 28.8 MB / 24,853 classes in a trial build. Not yet verified on a physical device. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![claude[bot]](https://cdn.github.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
📝 WalkthroughWalkthroughR8 shrinking is re-enabled for the application, while consumer rules preserve Kotlin analysis and IntelliJ classes instantiated through plugin descriptors and ChangesR8 preservation configuration
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
subprojects/kotlin-analysis-api/consumer-rules.pro (1)
9-360: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the maintenance/refresh process for this keep-rule list.
This file replaces a blanket
-dontshrinkwith ~350 hand-enumerated FQCN keep rules. That's the right tradeoff for size, but it's brittle against future upgrades of the bundled Kotlin Analysis API / IntelliJ platform jars: if a class referenced here is renamed or removed upstream, R8 will typically just drop the now-unresolvable keep rule (or warn) rather than fail the build, so breakage would only surface at runtime viaServiceConfigurationError/ClassNotFoundException— exactly the kind of regression the PR flags as still needing physical-device validation.Consider adding a short note (here or in the ticket) on how this list was derived (e.g., scanning
META-INF/services/*and plugin XML descriptors) and how to regenerate/diff it on future dependency bumps, so staleness is caught deliberately rather than discovered in the field.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subprojects/kotlin-analysis-api/consumer-rules.pro` around lines 9 - 360, Document how the hand-maintained keep-rule list was generated, including scanning META-INF/services entries and plugin XML descriptors, and describe the steps to regenerate and diff it after Kotlin Analysis API or IntelliJ platform dependency upgrades. Add this maintenance note near the keep rules or in the associated tracking documentation, referencing the enumerated FQCN rules as the maintained artifact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@subprojects/kotlin-analysis-api/consumer-rules.pro`:
- Around line 9-360: Document how the hand-maintained keep-rule list was
generated, including scanning META-INF/services entries and plugin XML
descriptors, and describe the steps to regenerate and diff it after Kotlin
Analysis API or IntelliJ platform dependency upgrades. Add this maintenance note
near the keep rules or in the associated tracking documentation, referencing the
enumerated FQCN rules as the maintained artifact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b2ba7a8-0354-4e65-9b01-6a35a8ec8d9c
📒 Files selected for processing (2)
app/proguard-rules.prosubprojects/kotlin-analysis-api/consumer-rules.pro
💤 Files with no reviewable changes (1)
- app/proguard-rules.pro
Review feedback on #1596: the 352-entry keep-rule list had no documented provenance. Adds the exact extraction/parsing steps used to generate it and how to regenerate and diff it after a Kotlin Analysis API or IntelliJ platform dependency bump. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
On-device verification (debug build,
This was a debug build rather than release, so it doesn't fully replace the release-build device test already noted in the PR description, but it's a second independent confirmation that the kotlin-analysis-api keep rules hold up for project init, diagnostics, and completion. Go-to-definition check still to follow. |
|
Follow-up on the go-to-definition item: reopened the same test project (debug build) and long-pressed a resolved symbol ( Checked why: So this is a pre-existing gap in the product (Kotlin go-to-definition was never hooked up to the editor UI), not something this PR's R8/keep-rule changes affect either way — there's no existing behavior here for the keep rules to have broken. Updated the test-plan checkbox to reflect that autocomplete and diagnostics were verified for Kotlin, and noted go-to-definition separately rather than leave it looking untested or falsely checked. |
|
Filed ADFA-4923 to track the missing Kotlin go-to-definition action. |
2 participants
Summary
classes*.dexexceeds 108 MB. The single biggest contributor turned out to be the bundled Kotlin Analysis API jar (used for in-editor Kotlin code intelligence): 43,951 classes / ~18.4 MB, ~42% of all dex bytecode.:apprelease (isMinifyEnabled = isAppModule), butapp/proguard-rules.prohad a blanket-dontshrinkadded in ADFA-996 after shrinking broke release-build project init — neutering shrinking entirely ever since.-dontshrinkand adds targeted-keeprules tosubprojects/kotlin-analysis-api/consumer-rules.profor every class the jar loads reflectively via IntelliJ plugin XML descriptors (Core.xml,CoreImpl.xml,JavaPsiPlugin.xml,analysis-api/*.xml) andMETA-INF/servicesServiceLoaderentries — the paths R8 can't trace from bytecode reachability alone.Trial results
Local release build (
:app:assembleV8Release), before vs after:org.jetbrains.kotlin.*(analysis API)Spot-checked R8's
usage.txtand the output dex directly:MainActivity,OnboardingActivity,CrashHandlerActivity, and the Kotlin LSP classes are all still present. Thecom.itsaky.androidide.*entries R8 removed are unused syntheticR$/BuildConfigclasses and dead private members, not core app code.Risk
ADFA-996's prior shrink attempt failed at runtime (release-build project init), not at build time — static analysis and a build-succeeds check don't rule that out here. No ARM device was available to install and exercise the shrunk APK before opening this PR, particularly Kotlin autocomplete/diagnostics (the exact feature the new keep rules protect).
Test plan
🤖 Generated with Claude Code