Conversation
The generated `updateViewProps` resolved the view's C++ HybridObject (`javaView->getJHybrid<T>Spec()`) on every props update, only to call setters that forward to Kotlin through its `_javaPart`. Since the `CxxPart` weak_ptr split (margelo#1238), nothing keeps that object alive between updates, so each update created it (with a JNI global ref and the `synchronized` lookup) and destroyed it again. Generate each changed prop's call directly on `javaView`, with the same method name, JNI signature and conversion as the C++ setter (the forward body is now shared through `getFbjniMethodCallBody`), include the prop types' JNI conversion headers in the updater, and resolve the C++ HybridObject only when `hybridRef` changed and is handed to JS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Author
|
Take with a grain of salt, just something claude found which I think might be interesting. |
This branch was previously deployed
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
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.
On Android, every props update of every HybridView creates the view's C++ HybridObject and destroys it again. The generated
updateViewPropsresolves it withjavaView->getJHybrid<T>Spec()only to call setters, and those forward to Kotlin through its_javaPart, whichupdateViewPropsalready has asjavaView.This used to be free: the updater called through
javaView->cthis(). #1238 split out theCxxPartwith aweak_ptrto fix the global-ref leak, and the updater switched togetJHybrid<T>Spec(). Since then nothing holds the object between updates, so each update:synchronizedlookup and thegetCxxPart()JNI call;JHybrid<T>Specand a JNI global ref;The weak cache stays as it is. This PR only stops the updater from needing the object:
javaView, with the same method name, JNI signature and conversion as the generated C++ setter. The forward-call body is shared throughgetFbjniMethodCallBody, so both code paths generate from one place..cppincludes the prop types' JNI conversion headers (enums, callbacks, optionals…).hybridRef: the C++ HybridObject is resolved only whenhybridRefchanged and is handed to JS, as before fix: Fix Kotlin HybridObjectjni::global_refmemory leak by separatingCxxPartwithweak_ptr#1238.Regenerating
react-native-nitro-testchanges only the two*StateUpdater.cppfiles; everyJHybrid*Spec.cppis byte-identical.Measured
All numbers are from a Galaxy A22 (Android 13, arm64) running Release builds of
apps/example, built frommainwith and without this change.The workload is a local screen, not included in this PR: 1000
RecyclableTestViews without ahybridRef, withisBlueflipped on all of them 200 times. Main-thread CPU comes from/proc, three alternating runs per build:mainsimpleperf on the same run:
updateViewPropsfell from 33.6% of main-thread samples to 14.6%.main, 61% ofupdateViewPropsis resolving, creating and destroying the HybridObject; the setter JNI calls themselves are 5.8%.getPropsFromStateWrapper.The performance CI benchmarks JS↔native calls, not view prop updates, so it won't show this. I can add a view benchmark if that's wanted.
Validation
bun run build,bun specs,bun typecheck,bun lint,git diff --check: pass. I couldn't runlint-cpp/lint-swift/lint-kotlinlocally, but no hand-written C++, Swift or Kotlin changes.nitro.views.harness.tsx+nitro.harness.ts):mainand this change;hybridRef, a callback and an enum):someMethod()works;main.hybridRefno longer creates itsCxxPartat all;dispose()already handles a missing one. ThehybridRefpath, and so what JS holds, is unchanged. iOS is not affected.🤖 Generated with Claude Code