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: Fix nested objects with $type key in JSON protocol #4670

Merged
merged 2 commits into from
Jan 29, 2024

Conversation

SevInf
Copy link
Contributor

@SevInf SevInf commented Jan 24, 2024

Introduce another special value to JSON protocol, "$type": "Raw".
When encountered, no other nested $type keys would be interpreted as
special and will be written to DB as is. Main usecase is JSON column
values with user-provided $type keys.

This is an alternative to #4668 that keeps JSON lists behaviour unchanged.

Part of the fix for prisma/prisma#21454, will require client adjustments
as well.

@SevInf SevInf added this to the 5.9.0 milestone Jan 24, 2024
@SevInf SevInf force-pushed the fix/nested-json-dollar-type-2 branch from 454abb7 to e840c5a Compare January 24, 2024 13:32
@SevInf SevInf changed the title qe: Fix nested objects with $type key in JSON protocol qe: Fix nested objects with $type key in JSON protocol (alternative idea) Jan 24, 2024
Copy link
Contributor

github-actions bot commented Jan 24, 2024

WASM Size

Engine This PR Base branch Diff
WASM 2.331MiB 2.329MiB 1.959KiB
WASM (gzip) 907.296KiB 906.378KiB 940.000B

Copy link

codspeed-hq bot commented Jan 24, 2024

CodSpeed Performance Report

Merging #4670 will not alter performance

Comparing fix/nested-json-dollar-type-2 (384423d) with main (79f6f09)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

github-actions bot commented Jan 24, 2024

❌ WASM query-engine performance will worsen by 1.78%

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  301.85 ms/iter  (300.6 ms … 303.49 ms) 302.97 ms 303.49 ms 303.49 ms
Web Assembly: Latest    361.12 ms/iter  (359.53 ms … 362.5 ms) 362.31 ms  362.5 ms  362.5 ms
Web Assembly: Current   368.67 ms/iter (366.41 ms … 371.97 ms) 370.73 ms 371.97 ms 371.97 ms
Node API: Current       230.45 ms/iter (220.59 ms … 239.79 ms) 238.13 ms 239.79 ms 239.79 ms

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

• movies.findMany({ take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.35 ms/iter    (12.13 ms … 14.4 ms)  12.29 ms   14.4 ms   14.4 ms
Web Assembly: Latest     14.73 ms/iter   (14.56 ms … 15.94 ms)  14.73 ms  15.94 ms  15.94 ms
Web Assembly: Current    14.99 ms/iter   (14.85 ms … 16.39 ms)  14.96 ms  16.39 ms  16.39 ms
Node API: Current        8,691 µs/iter   (8,496 µs … 9,005 µs)  8,762 µs  9,005 µs  9,005 µs

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

• movies.findMany({ where: {...}, take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1,937 µs/iter   (1,837 µs … 3,134 µs)  1,921 µs  2,670 µs  3,134 µs
Web Assembly: Latest     2,331 µs/iter   (2,241 µs … 3,410 µs)  2,322 µs  3,272 µs  3,410 µs
Web Assembly: Current    2,390 µs/iter   (2,278 µs … 3,991 µs)  2,371 µs  3,383 µs  3,991 µs
Node API: Current        1,481 µs/iter   (1,396 µs … 1,848 µs)  1,502 µs  1,673 µs  1,848 µs

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

• movies.findMany({ include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline    12.2 ms/iter   (12.09 ms … 12.37 ms)  12.23 ms  12.37 ms  12.37 ms
Web Assembly: Latest     14.63 ms/iter   (14.51 ms … 14.77 ms)  14.68 ms  14.77 ms  14.77 ms
Web Assembly: Current     14.9 ms/iter   (14.78 ms … 15.43 ms)  14.93 ms  15.43 ms  15.43 ms
Node API: Current        8,774 µs/iter   (8,609 µs … 9,082 µs)  8,809 µs  9,082 µs  9,082 µs

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

• movies.findMany({ where: {...}, include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1,889 µs/iter   (1,816 µs … 2,603 µs)  1,890 µs  2,315 µs  2,603 µs
Web Assembly: Latest     2,313 µs/iter   (2,236 µs … 2,783 µs)  2,309 µs  2,739 µs  2,783 µs
Web Assembly: Current    2,344 µs/iter   (2,276 µs … 2,844 µs)  2,351 µs  2,624 µs  2,844 µs
Node API: Current        1,479 µs/iter   (1,411 µs … 1,709 µs)  1,485 µs  1,636 µs  1,709 µs

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

• movies.findMany({ take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.16 ms/iter   (12.07 ms … 12.32 ms)   12.2 ms  12.32 ms  12.32 ms
Web Assembly: Latest      14.7 ms/iter   (14.55 ms … 15.69 ms)  14.68 ms  15.69 ms  15.69 ms
Web Assembly: Current    14.93 ms/iter   (14.82 ms … 15.09 ms)  14.95 ms  15.09 ms  15.09 ms
Node API: Current        8,930 µs/iter   (8,581 µs … 10.63 ms)  9,009 µs  10.63 ms  10.63 ms

summary for movies.findMany({ take: 2000, include: { cast: { include: { person: true } } } })
  Web Assembly: Current
   1.67x slower than Node API: Current
   1.23x 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   1,897 µs/iter   (1,823 µs … 2,822 µs)  1,892 µs  2,706 µs  2,822 µs
Web Assembly: Latest     2,307 µs/iter   (2,236 µs … 3,145 µs)  2,308 µs  2,680 µs  3,145 µs
Web Assembly: Current    2,350 µs/iter   (2,263 µs … 3,430 µs)  2,335 µs  3,214 µs  3,430 µs
Node API: Current        1,487 µs/iter   (1,397 µs … 1,666 µs)  1,506 µs  1,656 µs  1,666 µs

summary for movie.findMany({ where: { ... }, take: 2000, include: { cast: { include: { person: true } } } })
  Web Assembly: Current
   1.58x 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.52 µs/iter  (846.48 µs … 1,442 µs) 897.36 µs  1,299 µs  1,442 µs
Web Assembly: Latest     1,134 µs/iter   (1,078 µs … 1,777 µs)  1,135 µs  1,606 µs  1,777 µs
Web Assembly: Current    1,136 µs/iter   (1,090 µs … 1,768 µs)  1,142 µs  1,407 µs  1,768 µs
Node API: Current       823.16 µs/iter  (753.83 µs … 1,008 µs) 837.63 µs 899.49 µs  1,008 µs

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

• movie.findMany({ where: { cast: { person: { ... } }, take: 100 }) (m2m -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   926.9 µs/iter  (854.96 µs … 1,583 µs) 913.89 µs  1,552 µs  1,583 µs
Web Assembly: Latest     1,150 µs/iter   (1,099 µs … 1,749 µs)  1,149 µs  1,601 µs  1,749 µs
Web Assembly: Current    1,167 µs/iter   (1,123 µs … 1,957 µs)  1,166 µs  1,609 µs  1,957 µs
Node API: Current       819.63 µs/iter (753.72 µs … 968.85 µs) 832.47 µs 899.64 µs 968.85 µs

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

After changes in 384423d

@SevInf SevInf force-pushed the fix/nested-json-dollar-type-2 branch 2 times, most recently from 9139b66 to 941744b Compare January 26, 2024 10:56
@SevInf SevInf requested a review from Weakky January 26, 2024 16:12
@SevInf SevInf closed this Jan 26, 2024
@SevInf
Copy link
Contributor Author

SevInf commented Jan 29, 2024

Original idea does not work for edge case with JSON lists, going back to this now.

Sergey Tatarintsev added 2 commits January 29, 2024 10:35
Introduce another special value to JSON protocol, `"$type": "Raw"`.
When encountered, no other nested `$type` keys would be interpreted as
special and will be written to DB as is. Main usecase is JSON column
values with user-provided `$type` keys.

This is an alternative to #4668, that might look cleaner on the engine
side.

Part of the fix for prisma/prisma#21454, will require client adjustments
as well.
@SevInf SevInf force-pushed the fix/nested-json-dollar-type-2 branch from 941744b to 384423d Compare January 29, 2024 09:35
@SevInf SevInf marked this pull request as ready for review January 29, 2024 10:39
@SevInf SevInf requested a review from a team as a code owner January 29, 2024 10:39
@SevInf SevInf requested review from aqrln and removed request for a team January 29, 2024 10:39
SevInf pushed a commit to prisma/prisma that referenced this pull request Jan 29, 2024
Works on top of prisma/prisma-engines#4670.
Adds `$type: Raw` special case to JSON protocol and replaces `$type:
Json` encoding with it.

See engine PR for detailed description of the problem.

Fix #21454
@SevInf SevInf changed the title qe: Fix nested objects with $type key in JSON protocol (alternative idea) qe: Fix nested objects with $type key in JSON protocol Jan 29, 2024
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!

@SevInf SevInf merged commit dbcc905 into main Jan 29, 2024
115 of 116 checks passed
@SevInf SevInf deleted the fix/nested-json-dollar-type-2 branch January 29, 2024 13:31
SevInf pushed a commit to prisma/prisma that referenced this pull request Jan 29, 2024
Works on top of prisma/prisma-engines#4670.
Adds `$type: Raw` special case to JSON protocol and replaces `$type:
Json` encoding with it.

See engine PR for detailed description of the problem.

Fix #21454
SevInf pushed a commit to prisma/prisma that referenced this pull request Jan 29, 2024
Works on top of prisma/prisma-engines#4670.
Adds `$type: Raw` special case to JSON protocol and replaces `$type:
Json` encoding with it.

See engine PR for detailed description of the problem.

Fix #21454
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