Strangle nix #4713

miguelff · 2024-02-09T13:09:31Z

Closes https://github.com/prisma/team-orm/issues/921

codspeed-hq · 2024-02-09T13:21:09Z

CodSpeed Performance Report

Merging #4713 will not alter performance

_{Comparing strangle-nix (113e0ec) with main (9878210)}

Summary

✅ 11 untouched benchmarks

.github/workflows/wasm-benchmarks.yml

.github/workflows/include/rust-wasm-setup/action.yml

github-actions · 2024-02-09T17:35:23Z

WASM Size

Engine	This PR	Base branch	Diff
Postgres	2.073MiB	2.073MiB	0.000B
Postgres (gzip)	815.757KiB	814.135KiB	1.623KiB
Mysql	2.055MiB	2.055MiB	0.000B
Mysql (gzip)	807.255KiB	805.318KiB	1.938KiB
Sqlite	2.015MiB	2.015MiB	0.000B
Sqlite (gzip)	794.301KiB	792.465KiB	1.836KiB

github-actions · 2024-02-10T18:19:55Z

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

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  316.17 ms/iter (313.33 ms … 321.86 ms) 319.15 ms 321.86 ms 321.86 ms
Web Assembly: Latest    394.09 ms/iter (387.26 ms … 406.26 ms) 397.63 ms 406.26 ms 406.26 ms
Web Assembly: Current   396.13 ms/iter (386.22 ms … 402.32 ms) 400.89 ms 402.32 ms 402.32 ms
Node API: Current       222.87 ms/iter (220.16 ms … 225.95 ms) 225.78 ms 225.95 ms 225.95 ms

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

• movies.findMany({ take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.38 ms/iter   (12.16 ms … 13.95 ms)  12.38 ms  13.95 ms  13.95 ms
Web Assembly: Latest     16.01 ms/iter   (15.73 ms … 17.64 ms)  15.97 ms  17.64 ms  17.64 ms
Web Assembly: Current    15.85 ms/iter   (15.69 ms … 16.68 ms)  15.87 ms  16.68 ms  16.68 ms
Node API: Current        8,845 µs/iter   (8,573 µs … 9,688 µs)  8,960 µs  9,688 µs  9,688 µs

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

• movies.findMany({ where: {...}, take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1,927 µs/iter   (1,832 µs … 3,127 µs)  1,928 µs  2,466 µs  3,127 µs
Web Assembly: Latest     2,505 µs/iter   (2,397 µs … 3,725 µs)  2,510 µs  3,083 µs  3,725 µs
Web Assembly: Current    2,496 µs/iter   (2,387 µs … 3,712 µs)  2,474 µs  3,538 µs  3,712 µs
Node API: Current        1,538 µs/iter   (1,389 µs … 2,167 µs)  1,564 µs  2,150 µs  2,167 µs

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

• movies.findMany({ include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.25 ms/iter   (12.13 ms … 12.43 ms)  12.32 ms  12.43 ms  12.43 ms
Web Assembly: Latest     15.84 ms/iter    (15.7 ms … 16.23 ms)  15.88 ms  16.23 ms  16.23 ms
Web Assembly: Current    15.83 ms/iter    (15.69 ms … 16.1 ms)  15.86 ms   16.1 ms   16.1 ms
Node API: Current        8,840 µs/iter   (8,540 µs … 9,235 µs)  8,918 µs  9,235 µs  9,235 µs

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

• movies.findMany({ where: {...}, include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1,907 µs/iter   (1,834 µs … 2,427 µs)  1,912 µs  2,294 µs  2,427 µs
Web Assembly: Latest     2,520 µs/iter   (2,392 µs … 3,862 µs)  2,503 µs  3,661 µs  3,862 µs
Web Assembly: Current    2,481 µs/iter   (2,390 µs … 3,064 µs)  2,478 µs  3,031 µs  3,064 µs
Node API: Current        1,551 µs/iter   (1,436 µs … 2,088 µs)  1,560 µs  1,896 µs  2,088 µs

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

• movies.findMany({ take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.35 ms/iter    (12.21 ms … 12.7 ms)  12.41 ms   12.7 ms   12.7 ms
Web Assembly: Latest     16.22 ms/iter   (15.77 ms … 17.24 ms)  16.43 ms  17.24 ms  17.24 ms
Web Assembly: Current    15.78 ms/iter   (15.63 ms … 16.23 ms)  15.84 ms  16.23 ms  16.23 ms
Node API: Current        8,961 µs/iter   (8,626 µs … 9,320 µs)  9,101 µs  9,320 µs  9,320 µs

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

• movie.findMany({ where: { ... }, take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1,909 µs/iter   (1,838 µs … 2,314 µs)  1,916 µs  2,191 µs  2,314 µs
Web Assembly: Latest     2,457 µs/iter   (2,374 µs … 2,986 µs)  2,467 µs  2,796 µs  2,986 µs
Web Assembly: Current    2,462 µs/iter   (2,382 µs … 3,021 µs)  2,457 µs  2,894 µs  3,021 µs
Node API: Current        1,504 µs/iter   (1,409 µs … 1,823 µs)  1,520 µs  1,755 µs  1,823 µs

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

• movie.findMany({ where: { reviews: { author: { ... } }, take: 100 }) (to-many -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  984.28 µs/iter  (852.95 µs … 1,735 µs) 946.54 µs  1,661 µs  1,735 µs
Web Assembly: Latest     1,225 µs/iter   (1,168 µs … 1,885 µs)  1,224 µs  1,668 µs  1,885 µs
Web Assembly: Current    1,229 µs/iter   (1,178 µs … 1,906 µs)  1,227 µs  1,691 µs  1,906 µs
Node API: Current       829.95 µs/iter  (762.09 µs … 1,336 µs) 845.19 µs 955.85 µs  1,336 µs

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

• movie.findMany({ where: { cast: { person: { ... } }, take: 100 }) (m2m -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  892.42 µs/iter  (854.16 µs … 1,197 µs)  899.9 µs  1,163 µs  1,197 µs
Web Assembly: Latest     1,206 µs/iter   (1,155 µs … 1,528 µs)  1,212 µs  1,469 µs  1,528 µs
Web Assembly: Current    1,211 µs/iter   (1,157 µs … 1,572 µs)  1,221 µs  1,504 µs  1,572 µs
Node API: Current       827.58 µs/iter  (739.15 µs … 1,279 µs) 835.78 µs  1,193 µs  1,279 µs

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

After changes in 113e0ec

miguelff · 2024-02-12T09:43:23Z

query-engine/query-engine-wasm/build.sh

@@ -45,14 +44,15 @@ fi
 if [ "$WASM_BUILD_PROFILE" = "dev" ]; then
    WASM_TARGET_SUBDIR="debug"
 else
-    WASM_TARGET_SUBDIR="release"
+    WASM_TARGET_SUBDIR="$WASM_BUILD_PROFILE"


This was just a bug in the code before. The profiling profile outputs at said directory.

miguelff · 2024-02-12T11:36:58Z

Opened integration PR @ #4717 to check automatic engines publishing

miguelff · 2024-02-12T11:37:52Z

@aqrln I think you are in the best position to review this despite coauthoring. I will also re-read the PR looking for loose ends.

miguelff · 2024-02-12T12:50:01Z

Makefile

+clean-qe-wasm:
+	@echo "Cleaning query-engine/query-engine-wasm/pkg" && \
+	cd query-engine/query-engine-wasm/pkg && find . ! -name '.' ! -name '..' ! -name 'README.md' -exec rm -rf {} +
+
+clean-cargo:
+	@echo "Cleaning cargo" && \
+	cargo clean
+
+clean: clean-qe-wasm clean-cargo
+


I'm confident there are more things to clean. Also, the fact that pkg exists and its checked in doesn't look right to me, being this an output directory.

I like the approach with rendering the npm package in target that we took for prisma-schema-wasm in this PR, maybe we should do the same for QE (in a follow-up PR since it's out of scope here)?

https://github.com/prisma/team-orm/issues/944

miguelff · 2024-02-12T13:17:53Z

.github/workflows/include/rust-wasm-setup/action.yml

+    - name: Install wasm-bindgen, wasm-opt
+      shell: bash
+      run: |
+        cargo binstall -y \


miguelff · 2024-02-12T13:18:42Z

query-engine/query-engine-wasm/rust-toolchain.toml

@@ -0,0 +1,4 @@
+[toolchain]
+channel = "nightly-2024-01-25"


Pinned, ported into here from #4699

.github/workflows/include/rust-wasm-setup/action.yml

.github/workflows/on-push-to-main.yml

aqrln · 2024-02-12T15:44:38Z

.github/workflows/wasm-size.yml

+        # with:
+        #   ref: ${{ github.event.pull_request.base.sha }}


Commented-out to make the check green in this PR and will be uncommented in a follow-up PR?

aqrln · 2024-02-12T15:46:42Z

Makefile

+clean-qe-wasm:
+	@echo "Cleaning query-engine/query-engine-wasm/pkg" && \
+	cd query-engine/query-engine-wasm/pkg && find . ! -name '.' ! -name '..' ! -name 'README.md' -exec rm -rf {} +
+
+clean-cargo:
+	@echo "Cleaning cargo" && \
+	cargo clean
+
+clean: clean-qe-wasm clean-cargo
+


I like the approach with rendering the npm package in target that we took for prisma-schema-wasm in this PR, maybe we should do the same for QE (in a follow-up PR since it's out of scope here)?

aqrln · 2024-02-12T15:48:23Z

Makefile

@@ -77,6 +115,11 @@ test-qe-verbose-st:
 test-qe-black-box: build-qe
 	cargo test --package black-box-tests -- --test-threads 1

+check-schema-wasm-package: build-schema-wasm
+	PRISMA_SCHEMA_WASM="$(REPO_ROOT)/target/prisma-schema-wasm" \
+	out=$(shell mktemp -d) \


We can remove this line and the line that touches out in the corresponding script, the nix derivation didn't have any meaningful outputs in this case.

nix/.gitignore

nix/publish-engine-size.nix

query-engine/query-engine-wasm/build.sh

aqrln · 2024-02-12T16:09:43Z

rust-toolchain.toml

@@ -0,0 +1,4 @@
+[toolchain]
+channel = "1.76.0"


Pinning it here may cause problems in the release images, Soph has tried it previously: #4611 (comment). Did the integration release succeed with these changes?

It only did in a previous comment, but didn't got to release after this change. We might need to reopen the PR.

#4717 reopened and updated

cherry-picked from #4699

Extracted from the scope to: https://github.com/prisma/team-orm/issues/943

aqrln

The updated CI pipelines look correct and were tested. They will no longer cause friction for team members not using Nix when they try to update those. Building query-engine-wasm is cleaner this way compared to the impure build that was only nominally using Nix but was going to great lengths to work around it because of using nightly cargo flags incompatible with vendored dependencies.

One pipeline was de-scoped from this PR and will be ported separately.

The dev shell is intact and continues to provide value for team members using Nix. Most of removed Nix code that was not used on CI was also not used by any of the current team members and was not maintained, so it makes sense to not keep dead code around. Anything that will be necessary can be reintroduced, but in a way usable by all team members whenever possible as per the team consensus. The only package in the flake I personally relied on nix build for was reintroduced as a makefile target in this PR.

aqrln · 2024-02-13T10:34:46Z

.github/workflows/include/rust-wasm-setup/action.yml

+      run: |
+        cargo binstall -y \
+            [email protected] \
+            [email protected]


I'm not sure if we want to pin wasm-opt, there seems to be no harm in always using the latest version, and I'm concerned that we may just ignore updating it and miss on improved optimizations. But it's not blocking this PR, just something to think about.

.github/workflows/on-push-to-main.yml

This was commented out in #4713 to make the pipeline pass in the PR because the changes weren't compatible with the pipeline on `main` with the intention to uncomment immediately after landing the changes, see #4713 (comment).

Due to the lack of Nix buy-in from the team and with most of Nix users leaving, and given the increased overhead of keeping multiple separate build systems and workflows in sync after the introduction of the WebAssembly tooling and difficulties in making changes in Nix code for team members who don't use it, it was previously decided to decrease our reliance on Nix and stop using it on CI, leaving it as optional and only for local development, which mostly happened [in February](#4713). However, the engines size dashboard was still powered by Nix because we ran out of the allocated time for the tech debt task. After the Nix flake was updated last time, the workflow was broken because `wasm-bindgen-cli` in the flake was at 0.2.95 while we are currently pinned to 0.2.93 and are blocked from upgrading to a newer version at the moment. Rather than pinning `wasm-bindgen-cli` to 0.2.93 in the flake by taking it from a different nixpkgs commit, it's a good opportunity to start using the same infrastructure we use for other GitHub Actions jobs instead. With that, and given the fact that our workflows and build scripts are heavily dependent on rustup and we even used rustup within the dev shell instead of the toolchain from `rust-overlay`, there's not much benefit for the local dev shell for Nix users to be a flake, a classic `shell.nix` is more appropriate: pinning the state of the environment in `flake.lock` is no longer useful and only gets in the way. As an added bonus, classic Nix doesn't require copying the sources to the store, which makes the shell startup a bit faster. Fixes: prisma/team-orm#1444 Closes: #5072

…5095) Due to the lack of Nix buy-in from the team and with most of Nix users leaving, and given the increased overhead of keeping multiple separate build systems and workflows in sync after the introduction of the WebAssembly tooling and difficulties in making changes in Nix code for team members who don't use it, it was previously decided to decrease our reliance on Nix and stop using it on CI, leaving it as optional and only for local development, which mostly happened [in February](#4713). However, the engines size dashboard was still powered by Nix because we ran out of the allocated time for the tech debt task. After the Nix flake was updated last time, the workflow was broken because `wasm-bindgen-cli` in the flake was at 0.2.95 while we are currently pinned to 0.2.93 and are blocked from upgrading to a newer version at the moment. Rather than pinning `wasm-bindgen-cli` to 0.2.93 in the flake by taking it from a different nixpkgs commit, it's a good opportunity to start using the same infrastructure we use for other GitHub Actions jobs instead. With that, and given the fact that our workflows and build scripts are heavily dependent on rustup and we even used rustup within the dev shell instead of the toolchain from `rust-overlay`, there's not much benefit for the local dev shell for Nix users to be a flake, a classic `shell.nix` is more appropriate: pinning the state of the environment in `flake.lock` is no longer useful and only gets in the way. As an added bonus, classic Nix doesn't require copying the sources to the store, which makes the shell startup a bit faster. Fixes: prisma/team-orm#1444 Closes: #5072

aqrln reviewed Feb 9, 2024

View reviewed changes

.github/workflows/wasm-benchmarks.yml Outdated Show resolved Hide resolved

aqrln reviewed Feb 9, 2024

View reviewed changes

.github/workflows/include/rust-wasm-setup/action.yml Outdated Show resolved Hide resolved

aqrln reviewed Feb 9, 2024

View reviewed changes

.github/workflows/include/rust-wasm-setup/action.yml Outdated Show resolved Hide resolved

miguelff force-pushed the strangle-nix branch 2 times, most recently from 03a4c94 to 0b0c2b8 Compare February 9, 2024 17:21

miguelff force-pushed the strangle-nix branch 2 times, most recently from 24ca8cb to a1912ed Compare February 12, 2024 08:50

miguelff commented Feb 12, 2024

View reviewed changes

miguelff force-pushed the strangle-nix branch 2 times, most recently from ba42eb6 to 6264cb3 Compare February 12, 2024 10:33

miguelff mentioned this pull request Feb 12, 2024

Integration branch to check engine publishing for strangle-nix #4717

Closed

miguelff marked this pull request as ready for review February 12, 2024 11:36

miguelff requested a review from a team as a code owner February 12, 2024 11:36

miguelff requested review from aqrln and Weakky and removed request for a team February 12, 2024 11:36

miguelff force-pushed the strangle-nix branch from d677cbb to b75ecef Compare February 12, 2024 11:43

miguelff added this to the 5.10.0 milestone Feb 12, 2024

miguelff force-pushed the strangle-nix branch 2 times, most recently from bb0e934 to d2631c5 Compare February 12, 2024 12:47

miguelff commented Feb 12, 2024

View reviewed changes

aqrln reviewed Feb 12, 2024

View reviewed changes

miguelff assigned aqrln Feb 13, 2024

aqrln and others added 12 commits February 13, 2024 10:06

add wasm target for schema-wasm

c6c51e7

Use standard posix bc syntax

e105466

deduplicate wasm target

a2038f2

Output sizes in bytes, no KiB

a46aa1e

Remove PENDING.md

113632f

Ping cargo binstall versions

d88bede

Remove nix files except the shell

7a126e7

Be intentional about the use of nix

61b9cb7

Extract toml config to rust-toolchain.toml

b2b9e85

cherry-picked from #4699

Fix shellcheck

919abd1

Fix publishing prisma-schema-wasm

7b61203

Address some feedback

c1768d1

miguelff force-pushed the strangle-nix branch from 88f233a to c1768d1 Compare February 13, 2024 09:06

Miguel Fernández and others added 3 commits February 13, 2024 10:31

Merge branch 'main' into strangle-nix

bd2d195

Revert rust-toolchain.toml

d4c2be9

Remove engine size publishing from pipeline momentarily. Read more

02df1a7

Extracted from the scope to: https://github.com/prisma/team-orm/issues/943

miguelff unassigned aqrln Feb 13, 2024

miguelff added 2 commits February 13, 2024 10:57

Take back publishing engine sizes

d771f85

Update flake.nix

113e0ec

aqrln approved these changes Feb 13, 2024

View reviewed changes

miguelff merged commit e9f802b into main Feb 13, 2024
142 checks passed

miguelff deleted the strangle-nix branch February 13, 2024 11:13

aqrln mentioned this pull request Feb 13, 2024

ci(wasm-size): compare with the base branch and not self #4728

Merged

aqrln mentioned this pull request Dec 19, 2024

chore(ci): migrate engine size dashboard to pure bash and GH Actions #5095

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Strangle nix #4713

Strangle nix #4713

miguelff commented Feb 9, 2024 •

edited

Loading

codspeed-hq bot commented Feb 9, 2024 •

edited

Loading

github-actions bot commented Feb 9, 2024 •

edited

Loading

github-actions bot commented Feb 10, 2024 •

edited

Loading

miguelff Feb 12, 2024

miguelff commented Feb 12, 2024

miguelff commented Feb 12, 2024

miguelff Feb 12, 2024 •

edited

Loading

aqrln Feb 12, 2024

miguelff Feb 13, 2024 •

edited

Loading

miguelff Feb 12, 2024

miguelff Feb 12, 2024

aqrln Feb 12, 2024

miguelff Feb 12, 2024

aqrln Feb 12, 2024

aqrln Feb 12, 2024

aqrln Feb 12, 2024

miguelff Feb 12, 2024

miguelff Feb 12, 2024

aqrln left a comment

aqrln Feb 13, 2024

Strangle nix #4713

Strangle nix #4713

Conversation

miguelff commented Feb 9, 2024 • edited Loading

codspeed-hq bot commented Feb 9, 2024 • edited Loading

Merging #4713 will not alter performance

Summary

github-actions bot commented Feb 9, 2024 • edited Loading

WASM Size

github-actions bot commented Feb 10, 2024 • edited Loading

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

Choose a reason for hiding this comment

miguelff commented Feb 12, 2024

miguelff commented Feb 12, 2024

miguelff Feb 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miguelff Feb 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aqrln left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miguelff commented Feb 9, 2024 •

edited

Loading

codspeed-hq bot commented Feb 9, 2024 •

edited

Loading

github-actions bot commented Feb 9, 2024 •

edited

Loading

github-actions bot commented Feb 10, 2024 •

edited

Loading

miguelff Feb 12, 2024 •

edited

Loading

miguelff Feb 13, 2024 •

edited

Loading