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

add possible unknown mutation to objects and arrays #3882

Merged
merged 3 commits into from Feb 21, 2023

Conversation

sokra
Copy link
Member

@sokra sokra commented Feb 20, 2023

Before the static evaluation didn't consider the fact that objects and arrays can be mutated. So we can't just assume for sure that they have certain properties/items. Instead we add an unknown mutation alternative to handle that.

This avoids if(obj.prop) to be replaced with if(true) when prop is initialized with true. It might be modified in future.

@vercel
Copy link

vercel bot commented Feb 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
examples-kitchensink-blog 🔄 Building (Inspect) Feb 21, 2023 at 3:09PM (UTC)
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 21, 2023 at 3:09PM (UTC)
8 Ignored Deployments
Name Status Preview Comments Updated
examples-basic-web ⬜️ Ignored (Inspect) Feb 21, 2023 at 3:09PM (UTC)
examples-cra-web ⬜️ Ignored (Inspect) Feb 21, 2023 at 3:09PM (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Feb 21, 2023 at 3:09PM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Feb 21, 2023 at 3:09PM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Feb 21, 2023 at 3:09PM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Feb 21, 2023 at 3:09PM (UTC)
examples-tailwind-web ⬜️ Ignored (Inspect) Feb 21, 2023 at 3:09PM (UTC)
examples-vite-web ⬜️ Ignored (Inspect) Feb 21, 2023 at 3:09PM (UTC)

@github-actions
Copy link
Contributor

github-actions bot commented Feb 20, 2023

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

@github-actions
Copy link
Contributor

Benchmark for f6c943e

Test Base PR % Significant %
bench_hydration/Turbopack RCC/1000 modules 3654.90ms ± 13.05ms 3743.79ms ± 28.29ms +2.43% +0.17%
bench_startup/Turbopack RCC/1000 modules 2258.52ms ± 4.42ms 2279.98ms ± 5.94ms +0.95% +0.03%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9441.18µs ± 73.98µs 9468.08µs ± 63.18µs +0.28%
bench_hmr_to_commit/Turbopack RCC/1000 modules 12.43ms ± 0.23ms 12.63ms ± 0.16ms +1.59%
bench_hmr_to_commit/Turbopack RSC/1000 modules 502.60ms ± 1.76ms 501.03ms ± 1.74ms -0.31%
bench_hmr_to_commit/Turbopack SSR/1000 modules 9625.46µs ± 61.75µs 9585.82µs ± 78.63µs -0.41%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8529.31µs ± 61.25µs 8435.02µs ± 48.95µs -1.11%
bench_hmr_to_eval/Turbopack RCC/1000 modules 10.83ms ± 0.35ms 11.00ms ± 0.25ms +1.50%
bench_hmr_to_eval/Turbopack SSR/1000 modules 8635.91µs ± 77.41µs 8592.38µs ± 68.56µs -0.50%
bench_hydration/Turbopack RCC/1000 modules 3654.90ms ± 13.05ms 3743.79ms ± 28.29ms +2.43% +0.17%
bench_hydration/Turbopack RSC/1000 modules 3337.74ms ± 8.58ms 3338.09ms ± 4.78ms +0.01%
bench_hydration/Turbopack SSR/1000 modules 3404.45ms ± 5.80ms 3382.02ms ± 12.60ms -0.66%
bench_startup/Turbopack CSR/1000 modules 2676.90ms ± 8.46ms 2679.85ms ± 7.51ms +0.11%
bench_startup/Turbopack RCC/1000 modules 2258.52ms ± 4.42ms 2279.98ms ± 5.94ms +0.95% +0.03%
bench_startup/Turbopack RSC/1000 modules 2195.21ms ± 5.76ms 2205.33ms ± 6.73ms +0.46%
bench_startup/Turbopack SSR/1000 modules 2075.53ms ± 4.76ms 2076.33ms ± 2.80ms +0.04%

@github-actions
Copy link
Contributor

Benchmark for 45d9188

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 10.08ms ± 0.08ms 10.02ms ± 0.06ms -0.60%
bench_hmr_to_commit/Turbopack RCC/1000 modules 12.62ms ± 0.24ms 12.68ms ± 0.32ms +0.43%
bench_hmr_to_commit/Turbopack RSC/1000 modules 513.68ms ± 2.61ms 514.44ms ± 1.58ms +0.15%
bench_hmr_to_commit/Turbopack SSR/1000 modules 10.12ms ± 0.08ms 10.10ms ± 0.12ms -0.18%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8984.51µs ± 77.59µs 8997.06µs ± 60.83µs +0.14%
bench_hmr_to_eval/Turbopack RCC/1000 modules 11.14ms ± 0.25ms 11.17ms ± 0.12ms +0.27%
bench_hmr_to_eval/Turbopack SSR/1000 modules 9019.87µs ± 58.17µs 9036.71µs ± 80.57µs +0.19%
bench_hydration/Turbopack RCC/1000 modules 3796.19ms ± 25.27ms 3753.51ms ± 18.68ms -1.12%
bench_hydration/Turbopack RSC/1000 modules 3407.22ms ± 3.97ms 3421.67ms ± 17.03ms +0.42%
bench_hydration/Turbopack SSR/1000 modules 3463.46ms ± 11.17ms 3506.83ms ± 16.21ms +1.25%
bench_startup/Turbopack CSR/1000 modules 2758.33ms ± 7.95ms 2758.66ms ± 13.00ms +0.01%
bench_startup/Turbopack RCC/1000 modules 2325.57ms ± 8.46ms 2348.54ms ± 7.91ms +0.99%
bench_startup/Turbopack RSC/1000 modules 2243.18ms ± 8.72ms 2270.29ms ± 12.37ms +1.21%
bench_startup/Turbopack SSR/1000 modules 2144.71ms ± 3.84ms 2143.95ms ± 3.78ms -0.04%

@sokra sokra marked this pull request as ready for review February 21, 2023 10:34
@sokra sokra requested a review from a team as a code owner February 21, 2023 10:34
@sokra sokra requested a review from alexkirsz February 21, 2023 12:57
crates/turbopack-ecmascript/src/analyzer/builtin.rs Outdated Show resolved Hide resolved
"[{}]",
elems
"{}[{}]",
if *mutable { "" } else { "frozen " },
Copy link
Contributor

Choose a reason for hiding this comment

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

olaf

Co-authored-by: Alex Kirszenberg <alex.kirszenberg@vercel.com>
@sokra sokra added the pr: automerge Kodiak will merge these automatically after checks pass label Feb 21, 2023
@kodiakhq kodiakhq bot merged commit 24801f4 into main Feb 21, 2023
@kodiakhq kodiakhq bot deleted the sokra/web-613-compile-time-evaluation-is-too-greedy branch February 21, 2023 15:26
@github-actions
Copy link
Contributor

Benchmark for f0c120b

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9684.71µs ± 92.71µs 9708.94µs ± 85.19µs +0.25%
bench_hmr_to_commit/Turbopack RCC/1000 modules 12.87ms ± 0.38ms 13.27ms ± 0.37ms +3.11%
bench_hmr_to_commit/Turbopack RSC/1000 modules 501.83ms ± 2.67ms 503.71ms ± 1.91ms +0.38%
bench_hmr_to_commit/Turbopack SSR/1000 modules 9882.28µs ± 94.22µs 9857.47µs ± 80.39µs -0.25%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8783.36µs ± 70.93µs 8883.22µs ± 91.80µs +1.14%
bench_hmr_to_eval/Turbopack RCC/1000 modules 11.16ms ± 0.24ms 11.37ms ± 0.14ms +1.89%
bench_hmr_to_eval/Turbopack SSR/1000 modules 8869.71µs ± 120.41µs 8830.87µs ± 46.84µs -0.44%
bench_hydration/Turbopack RCC/1000 modules 3716.46ms ± 13.47ms 3758.51ms ± 15.62ms +1.13%
bench_hydration/Turbopack RSC/1000 modules 3341.81ms ± 8.78ms 3363.44ms ± 10.49ms +0.65%
bench_hydration/Turbopack SSR/1000 modules 3438.29ms ± 19.97ms 3416.47ms ± 9.72ms -0.63%
bench_startup/Turbopack CSR/1000 modules 2677.03ms ± 9.95ms 2687.33ms ± 11.01ms +0.38%
bench_startup/Turbopack RCC/1000 modules 2281.29ms ± 3.53ms 2297.95ms ± 7.05ms +0.73%
bench_startup/Turbopack RSC/1000 modules 2225.41ms ± 5.60ms 2223.06ms ± 5.45ms -0.11%
bench_startup/Turbopack SSR/1000 modules 2078.55ms ± 2.80ms 2080.81ms ± 3.20ms +0.11%

sokra added a commit to vercel/next.js that referenced this pull request Feb 21, 2023
# Performance Improvements

* improve performance of TaskScopes and collectibes ([#3849](https://github.com/vercel/turbo/pull3849/))
* fix consistency issue with reading collectibles ([#3878](vercel/turbo#3878))
* change to NoHashHasher for ID keys ([#3864](https://github.com/vercel/turbo/pull3864/))

# Bug Fixes

* strip UNC prefix on windows paths ([#3847](https://github.com/vercel/turbo/pull3847/))
* make error overlay message scrollable ([#3861](vercel/turbo#3861))
* alias @swc/helpers to the version within next.js ([#3865](https://github.com/vercel/turbo/pull3865/))
* add possible unknown mutation to objects and arrays ([#3882](vercel/turbo#3882))

# Contributing

* Update profiling instructions for macOS ([#3837](https://github.com/vercel/turbo/pull3837/))
ijjk pushed a commit to vercel/next.js that referenced this pull request Feb 21, 2023
jridgewell pushed a commit to vercel/next.js that referenced this pull request Mar 10, 2023
Before the static evaluation didn't consider the fact that objects and arrays can be mutated. So we can't just assume for sure that they have certain properties/items. Instead we add an unknown mutation alternative to handle that.

This avoids `if(obj.prop)` to be replaced with `if(true)` when prop is initialized with true. It might be modified in future.
sokra added a commit to vercel/next.js that referenced this pull request Mar 13, 2023
Before the static evaluation didn't consider the fact that objects and arrays can be mutated. So we can't just assume for sure that they have certain properties/items. Instead we add an unknown mutation alternative to handle that.

This avoids `if(obj.prop)` to be replaced with `if(true)` when prop is initialized with true. It might be modified in future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: automerge Kodiak will merge these automatically after checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants