Repository navigation
perf(bindings): the iOS and Android libraries are built for size, and still unwind - #580
Merged
Merged
Conversation
… profile A `mobile` profile, `release` at opt-level "s" and unwinding, builds the iOS and Android libraries: the release workflow's Android job, both React Native build scripts (the release's iOS job runs the iOS one) and CI's iOS simulator build. The desktop libraries and the wheels stay on `release`. `minisize` is deleted: no build ever used it. "s" rather than "z", from the Criterion benches on macOS arm64 against opt-level 3: "s" costs MLS sealing and opening about 40% and other paths at most a third, "z" doubles MLS and triples small-message serialization for a further quarter off the shared library. Keeping 30 crypto and codec crates at 3 under "z" recovered a third of the MLS cost and nothing else. Measured on Android arm64 (NDK 27.1, no C binding): 11,843,344 bytes today, 11,032,352 with the link-time optimisation fix, 8,540,440 with both. The 16 KB alignment gate passes on it, and the iOS minimum-OS gate on the simulator archive built at the new path. Two guards hold the profile (opt-level "s", no `panic = "abort"`, no `minisize`) and the build sites (the four mobile builds on `mobile`, the desktop builds on `--release`), each negative-controlled eight ways.
ADR 0018, ADR 0019, threat model R11, the C guide's K4, the telemetry sink contract, CLAUDE.md and six code comments said the mobile library shipped under `minisize` with `panic = "abort"`. No build ever did. Each now says what ships: every library unwinds, so a panic reaches Swift and Kotlin as an exception, and a document whose import panics is marked unusable for the process instead of ending it. R11's impact line follows. The containment argument is unchanged: the AEAD tag was always the first line. ADR 0029 records the profile decision with the size and speed tables. ADR 0024's `minisize` row is annotated as a measurement of a profile no build used. The integration guides, the Swift package recipe and the quick start build with `--profile mobile`. CHANGELOG and UPGRADING section 28 describe the change.
scripts/tests/test-ios-min-os.sh finds the iOS script's cargo build by its text to check the deployment target is exported first; it still looked for `--release`. All 69 assertions pass.
…orts The point of this branch is that the documents finally say what ships. Three places still said the phone library aborts on a panic, two of them in files the branch had already edited: the remote import's doc comment, a data test's comment, and the firmware footprint README, which called its `panic = "abort"` "the shape a shipping artifact has". It isn't. Not any more, and not ever, in fact. The rewrite of R11 and ADRs 0018, 0019 and 0029 had a subtler problem. It said the import's `catch_unwind` turns an engine panic into an error and marks the document unusable. It turns out that catch wraps the engine's apply step and nothing else. The decode that judges a peer's blob first, in `inspect` and in the import's own header check, runs outside it, and the code's comments already say a crafted blob can panic in that decoder. So under the unwinding build there are two outcomes, not one. A decoder panic unwinds out of the frame's handling, leaves the document alone, and leaves the in-flight marker on disk, which the next record load for that space promotes into the quarantine. An apply-step panic is caught and poisons the document. Say both, in the threat model, the three ADRs and the changelog entry. While at it, the UPGRADING note now admits the data layer has no bench, and two comment lines that ran to 119 columns are rewrapped.
The profile guard checked for `panic = "abort"` in the `mobile` table only. `mobile` names no `panic` of its own, so it inherits `release`'s, and an abort written there sailed past the guard while reaching every phone, and every desktop library the C binding needs unwinding in. The guard already parsed `release` for `lto`. Check `panic` there too. The build commands in the quickstart, the two integration guides, the Swift README and CLAUDE.md are what people copy, and nothing builds from them, so nothing failed when one drifted back to `--release`. Hold them: every documented iOS or Android build uses `--profile mobile`, and no iOS or Android path points into `release/`. Negative-controlled four ways: an abort on `release`, an iOS build and an iOS path back on `release`, and an Android build back on `--release`. Each fails; the tree as committed passes.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Merge after #579. This branch does not contain it, and the mobile profile inherits its link-time optimisation from
release, which only reaches the library once #579 drops thelibcrate type.What was wrong
ADR 0018, ADR 0019, threat model R11, the C guide's K4, the telemetry sink contract, CLAUDE.md and six code comments said the mobile library shipped under
minisize, withpanic = "abort". No build ever usedminisize: every iOS, Android, desktop and wheel build ran--release. The mobile libraries shipped at opt-level 3 and unwound.The change
mobileprofile:releaseat opt-level "s", unwinding. The release workflow's Android job, both React Native build scripts (the release's iOS job runs the iOS one) and CI's iOS simulator build use--profile mobile, and read their output fromtarget/<triple>/mobile/.release.minisizeis deleted.--release.Why unwinding
Every panic seam in the SDK is a
catch_unwind: event handler isolation, the telemetry sink dispatch, the document import, and UniFFI's own conversion of a panic into a Swift or Kotlin exception.panic = "abort"would turn each into a SIGABRT attributed to the app, for a further 19% off the shared library.Why "s" and not "z"
The plan proposed "z". The benches overruled it. Criterion, macOS arm64, change against opt-level 3:
mls_seal_dmmls_open_dmsend_messageprocess_loopmessage_serialization/1000ble_fragmentation/1000ble_reassembly/1000Keeping 30 crypto and codec crates at opt-level 3 under "z" recovered only a third of the MLS cost, and the engine and serialization paths not at all. The data layer has no bench, so its cost at "z" would ship unmeasured. The static library that iOS and npm carry is no larger at "s" than at "z". Moving to "z" later is one word in
Cargo.tomland one in the guard.Size
Every row with #579's link-time optimisation except the first.
.so.dylib--release, no LTO)mobile, "s")Verified locally
cargo clippy --workspace -- -D warnings,cargo test --workspace --lib, rustdoc under-D warnings.panic = "abort"added,minisizeback, each of the four mobile sites back on--release, a desktop job onmobile, the profile at "z".Not verified
workflow_dispatchdry run with an-rc.Nversion after both PRs merge. This is the first release run through the Android and iOS collection steps since they moved.