Fix wasm32 build by gating MultiFileSession on non-wasm targets#8612
Conversation
…t panic `VortexSession::default()` eagerly constructs every registered session variable. With the `files` feature enabled (as in vortex-web), that includes `MultiFileSession`, whose `moka` cache builds a `Clock` that calls `std::time::Instant::now()` on construction. `Instant` is unsupported on `wasm32` and panics with "time not implemented on this platform", so vortex-web panicked the first time its session was created — which happens on file import. Multi-file scanning is not available on wasm, so only register `MultiFileSession` on non-wasm targets. Verified by building `vortex-web-wasm` for `wasm32-unknown-unknown` (release): the "time not implemented on this platform" panic path is present before this change and gone afterwards. Signed-off-by: Robert <robert@spiraldb.com> Signed-off-by: Robert Kruszewski <github@robertk.io>
robert3005
force-pushed
the
claude/wasm-time-panic-c9b6s6
branch
from
3356f9a to
ff24840
Compare
Merging this PR will improve performance by 11.1%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | encode_varbin[(1000, 8)] |
156.9 µs | 141.2 µs | +11.1% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/wasm-time-panic-c9b6s6 (87f538a) with develop (88222ac)
Footnotes
-
4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
lwwmanning
left a comment
lwwmanning
left a comment
There was a problem hiding this comment.
approving but I disabled auto-merge, up to you whether you want to address my nit
3 participants
Rationale for this change
The
MultiFileSessionholds amokacache that readsstd::time::Instant::now()during construction. TheInstanttype is unsupported onwasm32targets and panics with "time not implemented on this platform". Since multi-file scanning is not available on wasm anyway, this change gates theMultiFileSessionregistration behind a#[cfg(not(target_arch = "wasm32"))]guard to allow the wasm32 build to succeed.What changes are included in this PR?
Added a
#[cfg(not(target_arch = "wasm32"))]attribute to theMultiFileSessionregistration invortex/src/lib.rsto prevent instantiation of the session on wasm32 targets wherestd::time::Instantis not available.What APIs are changed? Are there any user-facing changes?
No public APIs are changed. This is a build fix that prevents a panic on wasm32 targets. Multi-file scanning functionality remains unavailable on wasm, as it was before.
https://claude.ai/code/session_01P5VUBo8DhakxhhF1Ux2Kte