Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qe-wasm: Fix Bytes and scalar lists #4649

Merged
merged 1 commit into from
Jan 18, 2024
Merged

qe-wasm: Fix Bytes and scalar lists #4649

merged 1 commit into from
Jan 18, 2024

Conversation

SevInf
Copy link
Contributor

@SevInf SevInf commented Jan 16, 2024

Unlike NAPI engine, we did not have a proper Quaint <-> JS conversion
for lists and byte buffers. This PR adds it through introduction of
ToJsValue trait.

Previously, we relied on serde implementation to support this. However,
to do proper conversion we need to diverge from what serde is doing and
writing Serialize implementation by hand is not that easy. Esentially,
resons for ToJsValue trait introduction are the same as the resons for
FromJsValue were back in the day: allow simple conversions between
rust and JS without low-level serde code.

Fix prisma/team-orm#655
Fix prisma/team-orm#644
Fix prisma/team-orm#711

@SevInf SevInf added this to the 5.9.0 milestone Jan 16, 2024
Copy link
Contributor

github-actions bot commented Jan 16, 2024

WASM Size

Engine This PR Base branch Diff
WASM 2.719MiB 2.719MiB 480.000B
WASM (gzip) 1.002MiB 1.002MiB 380.000B

Copy link

codspeed-hq bot commented Jan 16, 2024

CodSpeed Performance Report

Merging #4649 will not alter performance

Comparing fix/bytes-lists-wasm (87d7b5d) with main (476ee04)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

github-actions bot commented Jan 16, 2024

🚨 WASM query-engine: 2 benchmark(s) have regressed at least 2%

Full benchmark report
DATABASE_URL="postgresql://postgres:postgres@localhost:5432/bench?schema=imdb_bench&sslmode=disable" \
node --experimental-wasm-modules query-engine/driver-adapters/executor/dist/bench.mjs
cpu: AMD EPYC 7763 64-Core Processor
runtime: node v18.19.0 (x64-linux)

benchmark                   time (avg)             (min … max)       p75       p99      p995
-------------------------------------------------------------- -----------------------------
• movies.findMany() (all - 25000)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  309.63 ms/iter (304.39 ms … 335.62 ms)  308.6 ms 335.62 ms 335.62 ms
Web Assembly: Latest    312.49 ms/iter (310.04 ms … 316.83 ms) 313.38 ms 316.83 ms 316.83 ms
Web Assembly: Current   315.57 ms/iter (310.99 ms … 329.22 ms) 316.71 ms 329.22 ms 329.22 ms
Node API: Current       230.95 ms/iter (225.66 ms … 242.19 ms) 232.02 ms 242.19 ms 242.19 ms

summary for movies.findMany() (all - 25000)
  Web Assembly: Current
   1.37x slower than Node API: Current
   1.02x slower than Web Assembly: Baseline
   1.01x slower than Web Assembly: Latest

• movies.findMany({ take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.43 ms/iter    (11.86 ms … 17.2 ms)   12.4 ms   17.2 ms   17.2 ms
Web Assembly: Latest     13.07 ms/iter   (12.25 ms … 21.72 ms)  12.61 ms  21.72 ms  21.72 ms
Web Assembly: Current    12.75 ms/iter   (12.15 ms … 16.72 ms)  12.78 ms  16.72 ms  16.72 ms
Node API: Current         9.27 ms/iter    (9.07 ms … 10.33 ms)    9.3 ms  10.33 ms  10.33 ms

summary for movies.findMany({ take: 2000 })
  Web Assembly: Current
   1.38x slower than Node API: Current
   1.03x slower than Web Assembly: Baseline
   1.02x faster than Web Assembly: Latest

• movies.findMany({ where: {...}, take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline    2.04 ms/iter     (1.83 ms … 3.59 ms)   1.98 ms    3.5 ms   3.56 ms
Web Assembly: Latest      2.17 ms/iter     (1.88 ms … 3.64 ms)   2.09 ms   3.51 ms   3.64 ms
Web Assembly: Current     2.15 ms/iter     (1.88 ms … 3.54 ms)   2.13 ms    3.5 ms   3.52 ms
Node API: Current         1.53 ms/iter     (1.43 ms … 1.79 ms)   1.56 ms   1.77 ms   1.77 ms

summary for movies.findMany({ where: {...}, take: 2000 })
  Web Assembly: Current
   1.41x slower than Node API: Current
   1.06x slower than Web Assembly: Baseline
   1.01x faster than Web Assembly: Latest

• movies.findMany({ include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.39 ms/iter   (11.91 ms … 13.13 ms)  12.59 ms  13.13 ms  13.13 ms
Web Assembly: Latest     12.37 ms/iter   (12.14 ms … 14.35 ms)  12.37 ms  14.35 ms  14.35 ms
Web Assembly: Current     12.6 ms/iter      (12 ms … 13.92 ms)  12.76 ms  13.92 ms  13.92 ms
Node API: Current         9.47 ms/iter    (9.13 ms … 10.29 ms)   9.63 ms  10.29 ms  10.29 ms

summary for movies.findMany({ include: { cast: true } take: 2000 }) (m2m)
  Web Assembly: Current
   1.33x slower than Node API: Current
   1.02x slower than Web Assembly: Latest
   1.02x slower than Web Assembly: Baseline

• movies.findMany({ where: {...}, include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline    1.93 ms/iter     (1.83 ms … 2.96 ms)   1.91 ms   2.83 ms   2.91 ms
Web Assembly: Latest      1.98 ms/iter     (1.87 ms … 4.42 ms)   1.94 ms   3.22 ms   3.35 ms
Web Assembly: Current     1.96 ms/iter     (1.87 ms … 3.84 ms)   1.95 ms   2.93 ms   3.04 ms
Node API: Current         1.55 ms/iter     (1.43 ms … 1.96 ms)   1.57 ms   1.85 ms   1.91 ms

summary for movies.findMany({ where: {...}, include: { cast: true } take: 2000 }) (m2m)
  Web Assembly: Current
   1.26x slower than Node API: Current
   1.02x slower than Web Assembly: Baseline
   1.01x faster than Web Assembly: Latest

• movies.findMany({ take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.33 ms/iter   (11.82 ms … 14.46 ms)  12.44 ms  14.46 ms  14.46 ms
Web Assembly: Latest      12.4 ms/iter   (12.18 ms … 14.73 ms)  12.41 ms  14.73 ms  14.73 ms
Web Assembly: Current     12.5 ms/iter   (12.04 ms … 16.53 ms)  12.55 ms  16.53 ms  16.53 ms
Node API: Current         9.39 ms/iter     (9.1 ms … 10.27 ms)    9.5 ms  10.27 ms  10.27 ms

summary for movies.findMany({ take: 2000, include: { cast: { include: { person: true } } } })
  Web Assembly: Current
   1.33x slower than Node API: Current
   1.01x slower than Web Assembly: Baseline
   1.01x slower than Web Assembly: Latest

• movie.findMany({ where: { ... }, take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline    1.89 ms/iter     (1.81 ms … 2.78 ms)   1.88 ms   2.45 ms   2.46 ms
Web Assembly: Latest      1.93 ms/iter     (1.82 ms … 3.14 ms)   1.91 ms    2.9 ms   3.01 ms
Web Assembly: Current     1.98 ms/iter     (1.85 ms … 3.64 ms)   1.93 ms    3.4 ms   3.54 ms
Node API: Current         1.55 ms/iter     (1.46 ms … 2.01 ms)   1.57 ms   1.79 ms   1.85 ms

summary for movie.findMany({ where: { ... }, take: 2000, include: { cast: { include: { person: true } } } })
  Web Assembly: Current
   1.28x slower than Node API: Current
   1.05x slower than Web Assembly: Baseline
   1.03x slower than Web Assembly: Latest

• movie.findMany({ where: { reviews: { author: { ... } }, take: 100 }) (to-many -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  936.44 µs/iter   (854.17 µs … 1.67 ms) 923.74 µs   1.48 ms   1.53 ms
Web Assembly: Latest    933.63 µs/iter    (859.1 µs … 1.64 ms) 922.77 µs   1.53 ms   1.56 ms
Web Assembly: Current   940.96 µs/iter   (872.01 µs … 1.61 ms) 932.91 µs    1.5 ms   1.56 ms
Node API: Current       849.68 µs/iter    (778.2 µs … 1.15 ms)  866.5 µs      1 ms   1.02 ms

summary for movie.findMany({ where: { reviews: { author: { ... } }, take: 100 }) (to-many -> to-one)
  Web Assembly: Current
   1.11x slower than Node API: Current
   1.01x slower than Web Assembly: Latest
   1x faster than Web Assembly: Baseline

• movie.findMany({ where: { cast: { person: { ... } }, take: 100 }) (m2m -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   934.1 µs/iter   (880.93 µs … 1.45 ms) 927.35 µs   1.34 ms   1.41 ms
Web Assembly: Latest    918.05 µs/iter    (864.89 µs … 1.6 ms) 917.02 µs   1.37 ms   1.41 ms
Web Assembly: Current    972.9 µs/iter   (878.42 µs … 1.95 ms) 945.16 µs   1.67 ms   1.82 ms
Node API: Current       817.31 µs/iter   (759.26 µs … 1.25 ms)  834.3 µs   1.02 ms   1.08 ms

summary for movie.findMany({ where: { cast: { person: { ... } }, take: 100 }) (m2m -> to-one)
  Web Assembly: Current
   1.19x slower than Node API: Current
   1.06x slower than Web Assembly: Latest
   1.04x slower than Web Assembly: Baseline

After changes in 87d7b5d

Unlike NAPI engine, we did not have a proper Quaint <-> JS conversion
for lists and byte buffers. This PR adds it through introduction of
`ToJsValue` trait.

Previously, we relied on serde implementation to support this. However,
to do proper conversion we need to diverge from what serde is doing and
writing `Serialize` implementation by hand is not that easy. Esentially,
resons for `ToJsValue` trait introduction are the same as the resons for
`FromJsValue` were back in the day: allow simple conversions between
rust and JS without low-level serde code.

Fix prisma/team-orm#655
Fix prisma/team-orm#644
Fix prisma/team-orm#711
@SevInf SevInf force-pushed the fix/bytes-lists-wasm branch from 316cc5b to 87d7b5d Compare January 16, 2024 15:07
@SevInf SevInf marked this pull request as ready for review January 16, 2024 15:36
@SevInf SevInf requested a review from a team as a code owner January 16, 2024 15:36
@SevInf SevInf requested review from laplab and jkomyno and removed request for a team January 16, 2024 15:36
@miguelff miguelff self-requested a review January 16, 2024 15:49
Copy link
Contributor

@miguelff miguelff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There has been a consistent regression in performance (about 2%) probably due the conversions of JsArgs that before were done by serde directly (and wrongly).

Other than that. The approach is correct.

@SevInf
Copy link
Contributor Author

SevInf commented Jan 16, 2024

There has been a consistent regression in performance (about 2%) probably due the conversions of JsArgs that before were done by serde directly (and wrongly).

Benchmarks also got quite flaky in your absence and regressions are often reported on the PRs that don't even touch WASM.

@SevInf SevInf merged commit 9c3f205 into main Jan 18, 2024
113 of 114 checks passed
@SevInf SevInf deleted the fix/bytes-lists-wasm branch January 18, 2024 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants