Let Mob.RenderStats see the native half of an Android frame (MOB-146) - #136
Merged
Conversation
native_summary/1 returned {:error, :unsupported} on Android because there was
no native implementation, which looks identical to "you forgot to enable it".
Adds native_stats/0 and native_stats_enable/1 there. The Erlang stubs and all
of Mob.RenderStats were already platform-agnostic, so only the native half was
missing.
The buffer lives in the app's generated MobBridge.kt rather than here: the
measurement can only be taken on the main thread, so keeping it beside the
writer avoids a JNI hop per sample. These NIFs fetch and toggle it the way
nif_element_frames fetches frames, and both Kotlin methods are cacheOptional so
an app generated before they existed still loads.
First Android navigation baseline, physical moto g power, 1600-node screen:
none 221ms p50, push 818ms, pop 788ms. Navigation costs 3.7x a re-render of
the same tree — the dispose-and-recreate gap this ticket exists to close, now
a number rather than an inference. Corroborated against the platform's own
accounting before it was believed: "Davey! duration=1414ms" and "Choreographer:
Skipped 67 frames" in the same window.
Android's apply_us is NOT iOS's apply_us, and the decision record says so
plainly because a differential test would otherwise compare them and conclude
something false. iOS starts its clock on the main thread after the dispatch
hop; Android starts on the NIF thread, so its interval includes a thread
handoff and up to one vsync of queue latency. It is a before-and-after tool
for one platform, not a cross-platform comparison.
Review fixes:
- native_call/3 now converts a RETURNED {:error, :not_loaded} to :unsupported.
Android signals a missing bridge method by returning rather than raising, so
it took the one path that was not converted, leaking a fourth shape past a
@SPEC promising two — and every app generated before this release hits it.
- Exact atom compare in native_stats_enable rather than a four-byte prefix,
which accepted :truthy and :true_x as "on" — the typo-enables-it case the
comment beside it claimed to refuse. Anything that is neither true nor false
is now badarg, matching iOS.
- native_stats added to iOS's caps[] and to Mob.Test.probe_keys/0. An existing
test enforces three-way agreement between the two platforms and Elixir, and
caught this: a key advertised by one native half and not the other.
- Moved a comment that had landed above sample_region's value instead of
render_stats', inside the exact positional array it was describing.
The capabilities test now checks the arrays pairwise rather than by length.
Counting cannot catch the failure it exists to catch: inserting a key
mid-array while appending its value at the end keeps the counts equal and
produces exactly the misordering, where each capability reports a different
NIF's availability. Mutation-checked by swapping two values.
Also pins the Android JSON shape against Mob.RenderStats: a wrapped ring
(recorded 250, retained 240, dropped 10), an escaped quote in a transition,
and an unescaped one — the last asserting that it costs the whole window
rather than one sample, so the escaping in MobBridge.kt reads as load-bearing.
Pairs with mob_new. Suite 1545 passed, credo clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Adding native_stats on its own line changed the column alignment of the whole block, so clang-format wanted to reflow it to one entry per line. CI runs `clang-format --dry-run -Werror` over ios/mob_nif.m; the repo's pre-commit checklist names `mix format` and credo but not the native formatters, which is how this reached CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Item 5 said `mix format && mix credo --strict`, and mix format says nothing about ObjC. CI runs clang-format over ios/mob_nif.m with -Werror, so adding one entry to an aligned C initialiser fails the build — clang-format re-flows the whole block around it. That is exactly what happened on this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two gaps found in pre-merge review.
The headline fix of this PR was untested. `native_call/3` converting a
RETURNED `{:error, :not_loaded}` to `:unsupported` is the path every
pre-existing Android app takes — the NIF is present so nothing raises, but the
Kotlin half is absent because MobBridge.kt is generated once and never
re-rendered. Deleting that clause left all 1545 tests green: every stub in the
file either raises or returns valid JSON, so nothing exercised a NIF that
returns the tuple. Mutation-checked now — removing the clause fails the new
test.
guides/agentic_coding.md claimed "every one of the seventeen keys is shown,
because guessing at the rest is exactly what goes wrong", which this PR made
false by adding an eighteenth. The example map is now complete again, labelled
as the pre-0.4.32 template it actually describes, and says what regenerating
flips — including that clear_text stays false on purpose. The guide arguing
against guessing at capabilities should not itself be a stale table.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Both edit the Android nif_funcs table: master flags the input NIFs IO_BOUND, this branch adds native_stats/native_stats_enable. Kept both — master's flags on the shared entries, plus the two new registrations. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
native_summary/1returned{:error, :unsupported}on Android because there was no native implementation — which looks identical to "you forgot to enable it." Addsnative_stats/0andnative_stats_enable/1there. The Erlang stubs and all ofMob.RenderStatswere already platform-agnostic, so only the native half was missing.The buffer lives in the app's generated
MobBridge.ktrather than here: the measurement can only be taken on the main thread, so keeping it beside the writer avoids a JNI hop per sample. These NIFs fetch and toggle it the waynif_element_framesfetches frames, and both Kotlin methods arecacheOptionalso an app generated before they existed still loads.First Android navigation baseline
Physical moto g power, 1600-node screen:
none221ms p50,push818ms,pop788ms. Navigation costs 3.7x a re-render of the same tree — the dispose-and-recreate gap MOB-146 exists to close, now a number rather than an inference.Corroborated against the platform's own accounting before it was believed:
Davey! duration=1414msandChoreographer: Skipped 67 framesin the same window.Android's
apply_usis not iOS'sapply_usStated plainly in the decision record, because a differential test (MOB-157) would otherwise compare them and conclude something false. iOS starts its clock on the main thread after the dispatch hop; Android starts on the NIF thread, so its interval includes a thread handoff and up to one vsync of queue latency. It is a before-and-after tool for one platform, not a cross-platform comparison.
Review fixes
native_call/3now converts a returned{:error, :not_loaded}to:unsupported. Android signals a missing bridge method by returning rather than raising, so it took the one path that wasn't converted — leaking a fourth shape past a@specpromising two, and every app generated before this release hits it.native_stats_enablerather than a four-byte prefix, which accepted:truthyand:true_xas "on" — the typo-enables-it case the comment beside it claimed to refuse. Anything that is neithertruenorfalseis nowbadarg, matching iOS.native_statsadded to iOS'scaps[]andMob.Test.probe_keys/0. An existing test enforces three-way agreement between the two platforms and Elixir, and caught this — a key advertised by one native half and not the other. Good test.sample_region's value instead ofrender_stats', inside the exact positional array it describes.Tests
The capabilities test now checks the arrays pairwise rather than by length. Counting cannot catch the failure it exists to catch: inserting a key mid-array while appending its value at the end keeps the counts equal and produces exactly the misordering, where each capability reports a different NIF's availability. Mutation-checked by swapping two values.
Also pins the Android JSON shape against
Mob.RenderStats: a wrapped ring (recorded 250, retained 240, dropped 10), an escaped quote in a transition, and an unescaped one — the last asserting it costs the whole window rather than one sample, so the escaping inMobBridge.ktreads as load-bearing rather than defensive tidiness.Suite 1545 passed, credo clean.
Pairs with GenericJam/mob_new#57.
🤖 Generated with Claude Code