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: Remove Buffer usage #4710

Merged
merged 2 commits into from
Feb 13, 2024
Merged

qe-wasm: Remove Buffer usage #4710

merged 2 commits into from
Feb 13, 2024

Conversation

SevInf
Copy link
Contributor

@SevInf SevInf commented Feb 9, 2024

Convert Byte arguments to Uint8Array instead.

Allows us to stop polyfilling Buffer for _bg.js fiels in engine.

@SevInf SevInf added this to the 5.10.0 milestone Feb 9, 2024
Copy link
Contributor

github-actions bot commented Feb 9, 2024

WASM Size

Engine This PR Base branch Diff
Postgres 2.043MiB 2.043MiB -227.000B
Postgres (gzip) 808.896KiB 809.740KiB -864.000B
Mysql 2.024MiB 2.023MiB 923.000B
Mysql (gzip) 799.862KiB 801.262KiB -1.401KiB
Sqlite 1.985MiB 1.985MiB 379.000B
Sqlite (gzip) 787.206KiB 787.888KiB -699.000B

Copy link

codspeed-hq bot commented Feb 9, 2024

CodSpeed Performance Report

Merging #4710 will not alter performance

Comparing no-buffer (0dd0861) with main (45a0eb4)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

github-actions bot commented Feb 9, 2024

❌ WASM query-engine performance will worsen by 1.54%

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      p999
-------------------------------------------------------------- -----------------------------
• movies.findMany() (all - 25000)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  304.49 ms/iter (302.75 ms … 310.16 ms) 305.34 ms 310.16 ms 310.16 ms
Web Assembly: Latest    381.68 ms/iter (378.63 ms … 395.89 ms) 380.74 ms 395.89 ms 395.89 ms
Web Assembly: Current   387.16 ms/iter (384.45 ms … 398.23 ms) 388.65 ms 398.23 ms 398.23 ms
Node API: Current       229.19 ms/iter (225.65 ms … 235.24 ms) 232.73 ms 235.24 ms 235.24 ms

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

• movies.findMany({ take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.01 ms/iter   (11.82 ms … 13.69 ms)  11.94 ms  13.69 ms  13.69 ms
Web Assembly: Latest     16.01 ms/iter   (15.31 ms … 20.91 ms)  15.86 ms  20.91 ms  20.91 ms
Web Assembly: Current       16 ms/iter   (15.52 ms … 18.24 ms)  16.07 ms  18.24 ms  18.24 ms
Node API: Current        8,769 µs/iter   (8,531 µs … 9,315 µs)  8,864 µs  9,315 µs  9,315 µs

summary for movies.findMany({ take: 2000 })
  Web Assembly: Current
   1.82x slower than Node API: Current
   1.33x slower than Web Assembly: Baseline
   1x faster than Web Assembly: Latest

• movies.findMany({ where: {...}, take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1,927 µs/iter   (1,822 µs … 3,347 µs)  1,901 µs  2,848 µs  3,347 µs
Web Assembly: Latest     2,469 µs/iter   (2,384 µs … 3,077 µs)  2,460 µs  3,041 µs  3,077 µs
Web Assembly: Current    2,524 µs/iter   (2,420 µs … 3,711 µs)  2,512 µs  3,604 µs  3,711 µs
Node API: Current        1,490 µs/iter   (1,414 µs … 1,947 µs)  1,501 µs  1,696 µs  1,947 µs

summary for movies.findMany({ where: {...}, take: 2000 })
  Web Assembly: Current
   1.69x slower than Node API: Current
   1.31x slower than Web Assembly: Baseline
   1.02x slower than Web Assembly: Latest

• movies.findMany({ include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.71 ms/iter   (12.16 ms … 18.43 ms)  12.35 ms  18.43 ms  18.43 ms
Web Assembly: Latest     15.71 ms/iter   (15.61 ms … 15.91 ms)  15.73 ms  15.91 ms  15.91 ms
Web Assembly: Current    16.18 ms/iter   (15.78 ms … 19.95 ms)  15.98 ms  19.95 ms  19.95 ms
Node API: Current        8,866 µs/iter   (8,626 µs … 9,229 µs)  8,929 µs  9,229 µs  9,229 µs

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

• movies.findMany({ where: {...}, include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1,909 µs/iter   (1,829 µs … 2,619 µs)  1,903 µs  2,475 µs  2,619 µs
Web Assembly: Latest     2,469 µs/iter   (2,386 µs … 3,139 µs)  2,460 µs  2,967 µs  3,139 µs
Web Assembly: Current    2,524 µs/iter   (2,419 µs … 3,977 µs)  2,508 µs  3,447 µs  3,977 µs
Node API: Current        1,516 µs/iter   (1,380 µs … 2,396 µs)  1,502 µs  2,207 µs  2,396 µs

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

• movies.findMany({ take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.15 ms/iter   (12.06 ms … 12.33 ms)  12.17 ms  12.33 ms  12.33 ms
Web Assembly: Latest     15.84 ms/iter   (15.54 ms … 20.74 ms)  15.72 ms  20.74 ms  20.74 ms
Web Assembly: Current    15.84 ms/iter   (15.69 ms … 16.35 ms)  15.87 ms  16.35 ms  16.35 ms
Node API: Current        8,814 µs/iter   (8,625 µs … 9,155 µs)  8,876 µs  9,155 µs  9,155 µs

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

• movie.findMany({ where: { ... }, take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1,982 µs/iter   (1,823 µs … 3,582 µs)  1,929 µs  3,533 µs  3,582 µs
Web Assembly: Latest     2,419 µs/iter   (2,357 µs … 2,869 µs)  2,426 µs  2,752 µs  2,869 µs
Web Assembly: Current    2,457 µs/iter   (2,396 µs … 2,875 µs)  2,458 µs  2,798 µs  2,875 µs
Node API: Current        1,482 µs/iter   (1,412 µs … 1,960 µs)  1,496 µs  1,687 µs  1,960 µs

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

• movie.findMany({ where: { reviews: { author: { ... } }, take: 100 }) (to-many -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  898.37 µs/iter  (846.52 µs … 1,444 µs) 896.23 µs  1,346 µs  1,444 µs
Web Assembly: Latest     1,188 µs/iter   (1,139 µs … 1,846 µs)  1,190 µs  1,465 µs  1,846 µs
Web Assembly: Current    1,230 µs/iter   (1,160 µs … 1,966 µs)  1,225 µs  1,870 µs  1,966 µs
Node API: Current       809.82 µs/iter  (742.43 µs … 1,024 µs)  823.7 µs 960.49 µs  1,024 µs

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

• movie.findMany({ where: { cast: { person: { ... } }, take: 100 }) (m2m -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  896.28 µs/iter   (854.7 µs … 1,326 µs) 900.25 µs  1,179 µs  1,326 µs
Web Assembly: Latest     1,200 µs/iter   (1,138 µs … 1,873 µs)  1,196 µs  1,740 µs  1,873 µs
Web Assembly: Current    1,203 µs/iter   (1,154 µs … 1,516 µs)  1,209 µs  1,475 µs  1,516 µs
Node API: Current       795.18 µs/iter   (746.8 µs … 1,145 µs)    819 µs 882.06 µs  1,145 µs

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

After changes in 0dd0861

Copy link
Contributor

@Jolg42 Jolg42 left a comment

Choose a reason for hiding this comment

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

Looks good, though there are some test failures. (I didn't check if it was related or not)

Sergey Tatarintsev added 2 commits February 12, 2024 19:07
Convert `Byte` arguments to `Uint8Array` instead.

Allows us to stop polyfilling `Buffer` for `_bg.js` fiels in engine.
@SevInf SevInf marked this pull request as ready for review February 12, 2024 18:08
@SevInf SevInf requested a review from a team as a code owner February 12, 2024 18:08
@SevInf SevInf requested review from laplab and Druue and removed request for a team February 12, 2024 18:08
@millsp
Copy link
Member

millsp commented Feb 12, 2024

I wondered if we built with wasm-bindgen --browser instead of a Node.js target if that would actually fix the issue in a different way? Since we are targeting browser environments, I guess we should change the current target even if this PR fixes this?

@SevInf SevInf merged commit e24eacf into main Feb 13, 2024
113 of 114 checks passed
@SevInf SevInf deleted the no-buffer branch February 13, 2024 09:05
@SevInf
Copy link
Contributor Author

SevInf commented Feb 13, 2024

I wondered if we built with wasm-bindgen --browser instead of a Node.js target if that would actually fix the issue in a different way? Since we are targeting browser environments, I guess we should change the current target even if this PR fixes this?

Buffer usage is on us here, it's not automatically inserted by wasm-bindgen. We used it to convert Bytes arguments for JS consumption

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