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

prisma-fmt: Make get_config error tolerant #4875

Merged
merged 2 commits into from
May 30, 2024
Merged

Conversation

SevInf
Copy link
Contributor

@SevInf SevInf commented May 17, 2024

Adds new function infallible_parse_configuration that never fails and always returns both Config and
Diagnostics. In case of errors, Diagnostic would be non-empty, but
it will still try to return as much of a config as it was able to parse.

This is useful mostly for dev tools - they need to be able to read
preview feature from more invalid files than old get_config would've
allowed. It also improves diagnostics in the CLI since we are now also
able to tell that user wanted to have more cases.

This is a breaking change to consumers of prisma-fmt: both prisma cli
and language-tools would need to be adapted to the new signature.

Contributes to prisma/team-orm#1143
Contributes to prisma/team-orm#1127 (opportunistic refactor)

@SevInf SevInf requested a review from a team as a code owner May 17, 2024 16:29
@SevInf SevInf requested review from Weakky and removed request for a team May 17, 2024 16:29
@SevInf SevInf added this to the 5.15.0 milestone May 17, 2024
Copy link

codspeed-hq bot commented May 17, 2024

CodSpeed Performance Report

Merging #4875 will not alter performance

Comparing get-config-fault-tolerant (033c2c5) with main (64bb6b9)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

github-actions bot commented May 17, 2024

WASM Query Engine file Size

Engine This PR Base branch Diff
Postgres 2.041MiB 2.041MiB 0.000B
Postgres (gzip) 813.643KiB 813.642KiB 1.000B
Mysql 2.011MiB 2.011MiB 0.000B
Mysql (gzip) 800.060KiB 800.059KiB 1.000B
Sqlite 1.914MiB 1.914MiB 0.000B
Sqlite (gzip) 762.786KiB 762.784KiB 2.000B

@SevInf SevInf force-pushed the get-config-fault-tolerant branch from 7f06c79 to f99e7a8 Compare May 17, 2024 16:34
prisma-fmt/src/get_config.rs Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented May 17, 2024

🚀 WASM query-engine performance will improve by 2.44%

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     365 ms/iter       (362 ms … 372 ms)    369 ms    372 ms    372 ms
Web Assembly: Latest       466 ms/iter       (462 ms … 473 ms)    472 ms    473 ms    473 ms
Web Assembly: Current      450 ms/iter       (446 ms … 459 ms)    451 ms    459 ms    459 ms
Node API: Current          199 ms/iter       (194 ms … 214 ms)    200 ms    214 ms    214 ms

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

• movies.findMany({ take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  14'772 µs/iter (14'620 µs … 15'747 µs) 14'807 µs 15'747 µs 15'747 µs
Web Assembly: Latest    18'759 µs/iter (18'524 µs … 20'369 µs) 18'717 µs 20'369 µs 20'369 µs
Web Assembly: Current   18'118 µs/iter (18'006 µs … 18'506 µs) 18'154 µs 18'506 µs 18'506 µs
Node API: Current        8'172 µs/iter  (7'850 µs … 10'339 µs)  8'052 µs 10'339 µs 10'339 µs

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

• movies.findMany({ where: {...}, take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   2'318 µs/iter   (2'194 µs … 3'882 µs)  2'311 µs  3'476 µs  3'882 µs
Web Assembly: Latest     2'929 µs/iter   (2'841 µs … 3'648 µs)  2'913 µs  3'492 µs  3'648 µs
Web Assembly: Current    2'847 µs/iter   (2'757 µs … 3'709 µs)  2'839 µs  3'413 µs  3'709 µs
Node API: Current        1'405 µs/iter   (1'310 µs … 1'800 µs)  1'414 µs  1'677 µs  1'800 µs

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

• movies.findMany({ include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline     576 ms/iter       (568 ms … 596 ms)    581 ms    596 ms    596 ms
Web Assembly: Latest       783 ms/iter       (780 ms … 788 ms)    786 ms    788 ms    788 ms
Web Assembly: Current      779 ms/iter       (770 ms … 801 ms)    781 ms    801 ms    801 ms
Node API: Current          475 ms/iter       (463 ms … 483 ms)    482 ms    483 ms    483 ms

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

• movies.findMany({ where: {...}, include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  78'793 µs/iter (78'436 µs … 79'208 µs) 79'088 µs 79'208 µs 79'208 µs
Web Assembly: Latest       110 ms/iter       (109 ms … 110 ms)    110 ms    110 ms    110 ms
Web Assembly: Current      109 ms/iter       (108 ms … 109 ms)    109 ms    109 ms    109 ms
Node API: Current       62'908 µs/iter (61'826 µs … 64'189 µs) 63'710 µs 64'189 µs 64'189 µs

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

• movies.findMany({ take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1'013 ms/iter   (1'004 ms … 1'031 ms)  1'027 ms  1'031 ms  1'031 ms
Web Assembly: Latest     1'312 ms/iter   (1'298 ms … 1'324 ms)  1'322 ms  1'324 ms  1'324 ms
Web Assembly: Current    1'296 ms/iter   (1'289 ms … 1'312 ms)  1'300 ms  1'312 ms  1'312 ms
Node API: Current          870 ms/iter       (849 ms … 885 ms)    879 ms    885 ms    885 ms

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

• movie.findMany({ where: { ... }, take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline     142 ms/iter       (141 ms … 146 ms)    145 ms    146 ms    146 ms
Web Assembly: Latest       183 ms/iter       (181 ms … 186 ms)    184 ms    186 ms    186 ms
Web Assembly: Current      182 ms/iter       (180 ms … 187 ms)    183 ms    187 ms    187 ms
Node API: Current          109 ms/iter       (107 ms … 112 ms)    111 ms    112 ms    112 ms

summary for movie.findMany({ where: { ... }, take: 2000, include: { cast: { include: { person: true } } } })
  Web Assembly: Current
   1.66x slower than Node API: Current
   1.28x 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   1'039 µs/iter     (975 µs … 1'695 µs)  1'038 µs  1'585 µs  1'695 µs
Web Assembly: Latest     1'456 µs/iter   (1'356 µs … 2'417 µs)  1'435 µs  2'285 µs  2'417 µs
Web Assembly: Current    1'381 µs/iter   (1'315 µs … 2'427 µs)  1'375 µs  2'055 µs  2'427 µs
Node API: Current          786 µs/iter     (703 µs … 9'622 µs)    792 µs  1'117 µs  9'622 µs

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

• movie.findMany({ where: { cast: { person: { ... } }, take: 100 }) (m2m -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   1'018 µs/iter     (972 µs … 1'588 µs)  1'019 µs  1'441 µs  1'588 µs
Web Assembly: Latest     1'402 µs/iter   (1'341 µs … 2'086 µs)  1'401 µs  1'812 µs  2'086 µs
Web Assembly: Current    1'357 µs/iter   (1'305 µs … 2'059 µs)  1'362 µs  1'687 µs  2'059 µs
Node API: Current          768 µs/iter     (713 µs … 1'169 µs)    784 µs    902 µs  1'169 µs

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

After changes in 49eaa97

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.

This looks good to me!

@SevInf SevInf force-pushed the get-config-fault-tolerant branch from 52596dc to 6ddcfcd Compare May 27, 2024 15:35
@SevInf SevInf changed the title prisma-fmt: Make parse_confgiration_multi error tolerant prisma-fmt: Make get_config error tolerant May 27, 2024
@SevInf SevInf requested a review from Jolg42 May 27, 2024 15:56
psl/psl/src/lib.rs Outdated Show resolved Hide resolved
/// blocks. It never fails, but when the returned `Diagnostics` contains errors, it implies that the
/// `Configuration` content is partial.
/// Consumers may then decide whether to convert `Diagnostics` into an error.
pub fn infallible_parse_configuration(files: &[(String, SourceFile)]) -> (Files, Configuration, Diagnostics) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I keep getting confused by this being called infallible and thinking ah, yes, no issues at all, wait, why are we getting diagnostics, what happened to our infallible leader!?

Copy link
Contributor

Choose a reason for hiding this comment

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

@SevInf, maybe parse_config_best_effort could better emphasise the role of this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jkomyno @Druue what would you say about error_tolerant_parse_configuration?

Copy link
Member

@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.

All good 👍

Now the function never fails and always returns both `Config` and
`Diagnostics`. In case of errors, `Diagnostic` would be non-empty, but
it will still try to return as much of a config as it was able to parse.

This is useful mostly for dev tools - they need to be able to read
preview feature from more invalid files than old `get_config` would've
allowed. It also improves diagnostics in the CLI since we are now also
able to tell that user wanted to have more cases.

This is a breaking change to consumers of `prisma-fmt`: both prisma cli
and language-tools would need to be adapted to the new signature.

Contributes to prisma/team-orm#1143
Contributes to prisma/team-orm#1127 (opportunistic refactor)
@SevInf SevInf force-pushed the get-config-fault-tolerant branch from 49eaa97 to 033c2c5 Compare May 30, 2024 13:04
SevInf added a commit to prisma/prisma that referenced this pull request May 30, 2024
SevInf added a commit to prisma/prisma that referenced this pull request May 30, 2024
@SevInf SevInf merged commit aa2d11f into main May 30, 2024
212 checks passed
@SevInf SevInf deleted the get-config-fault-tolerant branch May 30, 2024 15:45
SevInf added a commit to prisma/prisma that referenced this pull request May 30, 2024
SevInf added a commit to prisma/prisma that referenced this pull request May 30, 2024
…24349)

* feat(schema-files-loader): Read preview feature from invalid schemas

Integrates prisma/prisma-engines#4875.
Adapts `get_config` to changes from internals.

Closes prisma/team-orm#1143

* Bump engine
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.

None yet

5 participants