Repository navigation
test(wasm): choose UniFFI-generated moq-ffi as the browser binding - #5838
Conversation
|
Outcome: the harness and report are complete. The binding pick is left open for the maintainer (decision 1, recommending UniFFI moq-ffi via ubrn, with the size, byte-copy, and ubrn 0.32 work as conditions). Every number in the report comes from a loaded shared host, with candidates interleaved. (Written by Claude Opus 5.5) |
fdb4f78 to
9f557ac
Compare
kixelated
changed the title
|
Iteration: BoltFFI now runs end to end. The slice gained a browser client ( (Written by Claude Opus 5.5) |
…feature #5776 put uniffi-bindgen's binary behind a `cli` feature, so run.sh now runs it through `cargo run --features cli` like rs/moq-ffi/build.sh. `just bench-ffi-gen` takes several steps, and `size` builds the moq-ffi and moq-wasm references only it needs. The nightly runs `build bench` so #5478 and #5838 can reuse a slice that still builds. Also fixes clippy on the standalone crate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9f557ac to
d15b697
Compare
kixelated
changed the title
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 23 minutes. View limit details
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d15b697f65
ℹ️ 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".
| import { chromium } from "playwright"; | ||
| import type { Relay, Report } from "./cases.ts"; | ||
|
|
||
| const { values } = parseArgs({ options: { relays: { type: "string" }, timeout: { type: "string", default: "120" } } }); |
There was a problem hiding this comment.
When initialization or a benchmarked binding call never settles, page.evaluate() can remain pending until the 60-minute workflow limit because the parsed timeout value is never applied; run.sh also launches this driver without forwarding its configured --timeout. This turns the failure mode the harness is meant to detect into a long, opaque nightly hang, so bound the complete page run using the requested timeout and pass $TIMEOUT to this driver.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in bb55b79. --timeout now bounds each page (init, cases, and bench together), with a default of 300 s. A run with --timeout 1 fails with page did not finish. I didn't forward run.sh's --timeout: that value is a per-case budget (default 30 s), and each UniFFI case already carries its own 15 s bound inside the page, so the driver's page budget is a separate, larger number.
(Written by Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
browser.patch gives bench/ffi-gen's slice a WebTransport client, so both generators bind identical Rust and join the baseline sweep with parity cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llow-ups The browser binds moq-ffi through UniFFI over ubrn. Drop the BoltFFI slice harness (its numbers stay in the report, pointing at the commit that ran them), drop pin.patch's clock workaround now that moq-mux reads moq-clock's browser wall clock, and replace the generator quest with its follow-ups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bb55b79 to
d66f237
Compare
|
Merging with the maintainer's approval (2026-10-17).
(Written by Claude Opus 5.5) |
Completes the browser binding generator quest. The report is
test/wasm/generator.md.Decision
The maintainer chose moq-ffi generated by UniFFI over ubrn's
wasm2player (2026-10-17) over the alternatives:What lands
test/wasm/uniffi/, the harness Complete WASM bindings builds on:build.shbuilds moq-ffi through ubrn 0.31.0-6.pin.patchpins UniFFI 0.31 in a scratch worktree and never in the tree.size.tsattributes a.wasmto sections and crates.uniffibackend in the baseline sweep, interleaved with JS and moq-wasm.just test wasm-baseline --uniffiruns in nightly.What was dropped
test/wasm/slice/, the BoltFFI harness. It ran bench(ffi-gen): compare BoltFFI with UniFFI on a moq-ffi slice #5736's slice under both generators with a browser client overlay. It only served the choice, and the net-only size it showed is better measured on moq-ffi itself (net-only). The report keeps its numbers and points to the commit that produced them,9f557ac2b.--sliceflag and the two slice backends in the sweep.pin.patch's moq-mux clock workaround. Migrate network timing onto moq-clock #5681 moved moq-mux onto moq-clock's browser wall clock, and the cases pass without it.Results
All numbers come from a shared, loaded host (load average 12 to 32). Candidates were interleaved and cells are medians.
UniFFI's costs are the size of moq-ffi's surface, which the media layer drives, and per-byte payload copies at about 7 to 8 ns per byte. Both have follow-ups.
Follow-up quests (in this PR)
.wasmnear moq-wasm's size. Adoption requires it.Folded into existing quests rather than new ones:
Decisions confirmed by the maintainer (2026-10-17)
Since the last review
bench/ffi-gen/README.md's browser notes, which named a stale PR, at this decision.just check,quest check, andjust test wasm --uniffipass again on the rebased head.Public API / wire
None. Test harness, report, quests, and one nightly flag.
Checks
just checkpasses, and so doesquest check.just test wasm --uniffipasses (the moq-wasm suite 15/15, plus every UniFFI case in both CSP modes), and the--baseline --uniffismoke passes.9f557ac2b.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code