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

query-engine-c-abi: Remove openssl dependency #4840

Merged
merged 3 commits into from
Apr 25, 2024
Merged

query-engine-c-abi: Remove openssl dependency #4840

merged 3 commits into from
Apr 25, 2024

Conversation

SevInf
Copy link
Contributor

@SevInf SevInf commented Apr 24, 2024

Native drivers are not properly excluded in c-abi build: quaint's
native feature, pulled through request-handlers -> sql-connector
chain pulls all drivers in. In turn, tiberius brings openssl
requirement with it and that requires custom build for Android.

This PR ensures that tiberus is properly excluded from quaint
dependencies when building for native sqlite only. Unfortunately, that
means that native-* features now propogate to sql-query-connector
and request-handlers crates as well.

Contributes to prisma/team-orm#1106

Copy link
Contributor

github-actions bot commented Apr 24, 2024

WASM Query Engine file Size

Engine This PR Base branch Diff
Postgres 2.137MiB 2.130MiB 7.381KiB
Postgres (gzip) 840.811KiB 838.791KiB 2.021KiB
Mysql 2.107MiB 2.098MiB 8.727KiB
Mysql (gzip) 827.978KiB 825.525KiB 2.454KiB
Sqlite 1.998MiB 1.991MiB 7.182KiB
Sqlite (gzip) 787.575KiB 785.918KiB 1.657KiB

@SevInf SevInf added this to the 5.14.0 milestone Apr 24, 2024
Copy link
Contributor

github-actions bot commented Apr 24, 2024

✅ WASM query-engine performance won't change substantially (1.007x)

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.20.2 (x64-linux)

benchmark                   time (avg)             (min … max)       p75       p99      p999
-------------------------------------------------------------- -----------------------------
• movies.findMany() (all - ~50K)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline     374 ms/iter       (372 ms … 381 ms)    379 ms    381 ms    381 ms
Web Assembly: Latest       455 ms/iter       (451 ms … 459 ms)    459 ms    459 ms    459 ms
Web Assembly: Current      461 ms/iter       (455 ms … 473 ms)    469 ms    473 ms    473 ms
Node API: Current          196 ms/iter       (195 ms … 198 ms)    197 ms    198 ms    198 ms

summary for movies.findMany() (all - ~50K)
  Web Assembly: Current
   2.35x slower than Node API: Current
   1.23x slower than Web Assembly: Baseline
   1.01x slower than Web Assembly: Latest

• movies.findMany({ take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  14'917 µs/iter (14'736 µs … 15'780 µs) 14'930 µs 15'780 µs 15'780 µs
Web Assembly: Latest    18'248 µs/iter (18'072 µs … 18'767 µs) 18'341 µs 18'767 µs 18'767 µs
Web Assembly: Current   18'443 µs/iter (18'232 µs … 19'233 µs) 18'504 µs 19'233 µs 19'233 µs
Node API: Current        8'323 µs/iter  (7'929 µs … 11'905 µs)  8'277 µs 11'905 µs 11'905 µs

summary for movies.findMany({ take: 2000 })
  Web Assembly: Current
   2.22x slower than Node API: Current
   1.24x slower than Web Assembly: Baseline
   1.01x slower than Web Assembly: Latest

• movies.findMany({ where: {...}, take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   2'319 µs/iter   (2'222 µs … 3'456 µs)  2'304 µs  3'266 µs  3'456 µs
Web Assembly: Latest     2'824 µs/iter   (2'735 µs … 3'521 µs)  2'831 µs  3'380 µs  3'521 µs
Web Assembly: Current    2'845 µs/iter   (2'760 µs … 3'552 µs)  2'843 µs  3'434 µs  3'552 µs
Node API: Current        1'405 µs/iter   (1'306 µs … 1'645 µs)  1'419 µs  1'606 µs  1'645 µs

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

• movies.findMany({ include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline     568 ms/iter       (560 ms … 587 ms)    585 ms    587 ms    587 ms
Web Assembly: Latest       777 ms/iter       (772 ms … 794 ms)    783 ms    794 ms    794 ms
Web Assembly: Current      787 ms/iter       (776 ms … 802 ms)    802 ms    802 ms    802 ms
Node API: Current          475 ms/iter       (467 ms … 492 ms)    484 ms    492 ms    492 ms

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

• movies.findMany({ where: {...}, include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  78'217 µs/iter (77'975 µs … 78'516 µs) 78'423 µs 78'516 µs 78'516 µs
Web Assembly: Latest       108 ms/iter       (108 ms … 110 ms)    109 ms    110 ms    110 ms
Web Assembly: Current      110 ms/iter       (110 ms … 111 ms)    111 ms    111 ms    111 ms
Node API: Current       63'127 µs/iter (62'464 µs … 63'795 µs) 63'630 µs 63'795 µs 63'795 µs

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

• movies.findMany({ take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1'012 ms/iter     (997 ms … 1'035 ms)  1'035 ms  1'035 ms  1'035 ms
Web Assembly: Latest     1'298 ms/iter   (1'283 ms … 1'313 ms)  1'312 ms  1'313 ms  1'313 ms
Web Assembly: Current    1'319 ms/iter   (1'309 ms … 1'333 ms)  1'327 ms  1'333 ms  1'333 ms
Node API: Current          879 ms/iter       (841 ms … 903 ms)    902 ms    903 ms    903 ms

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

• movie.findMany({ where: { ... }, take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline     140 ms/iter       (139 ms … 140 ms)    140 ms    140 ms    140 ms
Web Assembly: Latest       180 ms/iter       (179 ms … 181 ms)    180 ms    181 ms    181 ms
Web Assembly: Current      189 ms/iter       (188 ms … 190 ms)    189 ms    190 ms    190 ms
Node API: Current          108 ms/iter       (105 ms … 110 ms)    109 ms    110 ms    110 ms

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

• movie.findMany({ where: { reviews: { author: { ... } }, take: 100 }) (to-many -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1'062 µs/iter   (1'006 µs … 1'741 µs)  1'057 µs  1'486 µs  1'741 µs
Web Assembly: Latest     1'359 µs/iter   (1'286 µs … 1'938 µs)  1'362 µs  1'867 µs  1'938 µs
Web Assembly: Current    1'386 µs/iter   (1'308 µs … 1'963 µs)  1'383 µs  1'827 µs  1'963 µs
Node API: Current          759 µs/iter     (705 µs … 1'130 µs)    777 µs    822 µs  1'130 µs

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

• movie.findMany({ where: { cast: { person: { ... } }, take: 100 }) (m2m -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1'045 µs/iter     (993 µs … 1'796 µs)  1'047 µs  1'446 µs  1'796 µs
Web Assembly: Latest     1'492 µs/iter   (1'293 µs … 2'385 µs)  1'413 µs  2'318 µs  2'385 µs
Web Assembly: Current    1'386 µs/iter   (1'314 µs … 2'135 µs)  1'389 µs  1'893 µs  2'135 µs
Node API: Current          743 µs/iter     (705 µs … 1'252 µs)    753 µs    838 µs  1'252 µs

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

After changes in 5ccaafd

Copy link

codspeed-hq bot commented Apr 24, 2024

CodSpeed Performance Report

Merging #4840 will not alter performance

Comparing no-tiberus (5ccaafd) with main (5d2f494)

Summary

✅ 11 untouched benchmarks

@SevInf SevInf marked this pull request as ready for review April 24, 2024 17:18
@SevInf SevInf requested a review from a team as a code owner April 24, 2024 17:18
@SevInf SevInf requested review from laplab and removed request for a team April 24, 2024 17:18
Native drivers are not properly excluded in c-abi build: `quaint`'s
`native` feature, pulled through `request-handlers` -> `sql-connector`
chain pulls all drivers in. In turn, `tiberius` brings openssl
requirement with it and that requires custom build for Android.

This PR ensures that tiberus is properly excluded from quaint
dependencies when building for native sqlite only. Unfortunately, that
means that `native-*` features now propogate to `sql-query-connector`
and `request-handlers` crates as well.
Comment on lines +6 to +11
#[cfg(any(
feature = "mssql-native",
feature = "mysql-native",
feature = "postgresql-native",
feature = "sqlite-native"
))]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we alias it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aliases have to be set up per-crate and unless I am missing something this is a single usage of this combination in sql-query-connectror, so not worth it, IMHO.

Copy link
Contributor

@Weakky Weakky left a comment

Choose a reason for hiding this comment

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

I'm painfully approving this.

@SevInf SevInf merged commit 031bd63 into main Apr 25, 2024
208 checks passed
@SevInf SevInf deleted the no-tiberus branch April 25, 2024 12:28
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.

2 participants