Integration: TS from rust
CodSpeed Performance Report
Merging #3835 integration/ts-from-rust (eff7140) will improve performances by 75.32%.
Summary
π₯ 3 improvements
β 0 regressions
β
3 untouched benchmarks
π 0 new benchmarks
βοΈ 0 dropped benchmarks
Benchmarks breakdown
| Benchmark | main |
integration/ts-from-rust |
Change | |
|---|---|---|---|---|
| π₯ | build (large) |
7.4 s | 4.2 s | +75.32% |
| π₯ | build (medium) |
347.6 ms | 268.4 ms | +29.5% |
| π₯ | build (small) |
42.3 ms | 34.6 ms | +22.14% |
Also sharing for early feedback and to start a discussion: I still have doubts about tsify. Is IntoWasmAbi meant to be implemented like that? I'm not familiar enough with wasm-bindgen internals to say, I'd need to get more familiar with the implementation first to do a good review. We'd be taking on something more complex (always counting the dependencies), we have to be sure the gains on the typescript side are justifying the tradeoffs.
One last factor that is making me hesitant is that the wasm β host boundary is not well tested or measured in other ways (e.g. basic benchmarks), so we're steering in the dark. Not an argument for status quo either, just a concern.
I also had another look serde_wasm_bindgen and it makes sense now, though. It's still very green but it makes sense. That might be a way around adding wasm-bindgen dependencies to crates other than prisma-fmt-wasm.
Also sharing for early feedback and to start a discussion: I still have doubts about tsify. Is
IntoWasmAbimeant to be implemented like that? I'm not familiar enough with wasm-bindgen internals to say, I'd need to get more familiar with the implementation first to do a good review. We'd be taking on something more complex (always counting the dependencies), we have to be sure the gains on the typescript side are justifying the tradeoffs.One last factor that is making me hesitant is that the wasm β host boundary is not well tested or measured in other ways (e.g. basic benchmarks), so we're steering in the dark. Not an argument for status quo either, just a concern.
Structurally, this PR would remove the need for two expensive operations in JS land: JSON.stringify(inputArguments) and JSON.parse(returnedValue). I know this is faster because I've done it elsewhere, and we can use benchmarks to test the difference of the old getDmmf.test.ts (using Wasm + JSON (de)serialization in Rust + JS) vs the new getDmmf.test.ts (using Wasm + JSON (de)serialization in Rust only)
I also had another look serde_wasm_bindgen and it makes sense now, though. It's still very green but it makes sense. That might be a way around adding wasm-bindgen dependencies to crates other than prisma-fmt-wasm.
Note: tsifyΒ is essentially a wrapper of serde_wasm_bindgen (currently using the jsonΒ feature flag) that's also responsible of creating type definitions that usually work out of the box, with escape hatches when needed (via the #[tsify(type = "...")] struct field macro).
Structurally, this PR would remove the need for two expensive operations in JS land: JSON.stringify(inputArguments) and JSON.parse(returnedValue). I know this is faster because I've done it elsewhere, and we can use benchmarks to test the difference of the old getDmmf.test.ts (using Wasm + JSON (de)serialization in Rust + JS) vs the new getDmmf.test.ts (using Wasm + JSON (de)serialization in Rust only)
For my understanding: as it is, the PR uses the serde-json variant of tsify (because the serde-wasm-bindgen variant had problems you raised on slack), β I understand the potential improvements in performance from using serde-wasm-bindgen, but in this case it's not clear to me, would it make any difference with what we do now?