RUM-15103: Fixcompose-ui compatibility issue#3303
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59559d79b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return runSafe("getLayoutNodeBoundsInWindow") { | ||
| node.layoutDelegate.outerCoordinator.coordinates.boundsInWindow() | ||
| } ?: runSafe("getLayoutNodeBoundsInWindow[reflection]") { |
There was a problem hiding this comment.
Avoid throwing on every bounds lookup in new Compose runtime
When the app runs with the newer Compose artifact (where internal getters are mangled as $ui), the direct call in getLayoutNodeBoundsInWindow() fails first and only then falls back to reflection. Because hitTest() calls this for every traversed LayoutNode, this means a NoSuchMethodError is thrown and caught repeatedly on the hot path for each tap/scroll, which can cause significant jank/perf regression even though the fallback eventually succeeds. Consider short-circuiting to the reflection path after the first compatibility failure (or caching the successful access strategy) instead of retrying the failing direct path every time.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
That is a valid point. I'm wondering if we can read Compose version used to compile and use version-related strategy for the app lifetime, hitting the same path.
However, I'm not sure at which stage these methods are generated: maybe we can have a mix of naming conventions from different Compose versions (e.g. 3rd-party lib composables + user-owned composables) in the same app?
There was a problem hiding this comment.
I think there isn't an easy way of reading compose version at runtime, at least we can have a simple variable of caching the result of reflection path to avoid to always retry the failing path.
There was a problem hiding this comment.
Caching can be the option. Maybe we can think about then reading version from classpath and generating version property in Gradle Plugin. But we need to make sure that all Compose-related code sites are produced by the same Compose compiler version then.
There was a problem hiding this comment.
I'am not sure that caching here gonna save a lot, because reflection is a fallback here, not the main scenario, but let's add them for any potential case.
There was a problem hiding this comment.
It is a fallback, but it is a persistent fallback, because this is for the static code. This fallback can be hit many times (all the time for the particular element).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3303 +/- ##
===========================================
+ Coverage 71.52% 71.55% +0.03%
===========================================
Files 946 946
Lines 34895 34910 +15
Branches 5909 5916 +7
===========================================
+ Hits 24956 24978 +22
+ Misses 8270 8265 -5
+ Partials 1669 1667 -2
🚀 New features to boost your workflow:
|
59559d7 to
7217f8b
Compare
What does this PR do?
Fix compatibility with the latest
compose-uiwhere Google migrated from android-compose to KMP-compatible Compose. This change caused method mangling differences — the Kotlin compiler now adds$uisuffix instead of$ui_releasefor internal members.Added reflection-based fallback in
getLayoutNodeBoundsInWindow()that searches for methods without$suffix with the newgetMethod()helper.Motivation
Google's migration of Compose to KMP-compatible version (https://android-review.googlesource.com/c/platform/frameworks/support/+/3684165) introduced breaking changes in method mangling for internal members. Our SDK is compiled with Compose BOM 2023.10.01, and upgrading would pull in kotlin-coroutines dependencies that some users don't want.
Screen_recording_20260327_161101.webm
Screen_recording_20260327_161522.webm