[pull] canary from vercel:canary #248
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
### What? In development give the module factory a useful name, so it's clear in stack traces that this is the module evaluation part of the execution. This also fixes some bugs with the stack trace parser, which seems to struggle if function names contain brackets. The automatically inferred function name would be the module id, which contains brackets.
### Improve performance of chunk bootstrapping leveraging arrays and maps
Currently we bootstrap the set of the modules in a chunk by passing an object to the runtime, this is suboptimal for a few reasons:
* The runtime just wants to iterate the object and copy it into a runtime datastructure and iterating objects is relatively slow
* The browser needs to spend time computing the 'shape' of this object and possibly changing internal representations
The solution to both these problems is of course, Arrays!. so instead of an object like
```
{ [moduleId]: (__turbopack_context__) => {},...}
```
we use an array like this
```
[ moduleId, (__turbopack_context__) => {}]
```
* a non-empty sequence of module ids come first
- we may have more than one to support scope hoisted modules
* the factory function
Because these objects are temporary this should make both constructing and iterating them faster.
Finally, in the runtime what we want to do is copy these modules into a global map that is represented as an `Object`. Because we are constantly inserting new keys, this is a suboptimal case and a `Map` should be faster for both insertions and lookups. So this adapts the moduleFactory type to use an es6 `Map` which is more optimized for this particular usecase.
Ideally we would do the same for the moduleCache but that would complicate `require.cache` usecases. We could perhaps leverage a `Proxy` to maintain support but this is left as future work.
### Aside: why are arrays faster than objects?
Arrays are faster to parse because they incur fewer 'shape transitions' (background reading: https://v8.dev/blog/fast-properties vs https://v8.dev/blog/elements-kinds). Basically for every new property v8 parses on the object literal it changes the hidden type (or 'shape'). For arrays the same thing happens but the process is much simpler as there is a fixed set of possible shapes and the browser will hit the terminal one after evaluating the second element.
Also arrays are simply faster to iterate than objects (but this is generally a non-controversial statement).
### Performance
Based on the `module-cost` benchmark this is a consistent win in all scenarios
| Scenario | Metric | HEAD (ms) | CHANGE (ms) | Delta (ms) | % Change |
|----------|--------|-----------|-------------|------------|----------|
| **Pages API ESM** | Load Duration | 33.46 | 27.87 | -5.59 | -16.7% ✅ |
| | Execution Duration | 63.22 | 62.54 | -0.68 | -1.1% ✅ |
| **Pages API CommonJS** | Load Duration | 44.11 | 33.76 | -10.35 | -23.5% ✅ |
| | Execution Duration | 32.05 | 25.70 | -6.35 | -19.8% ✅ |
| **Client ESM** | Load Duration | 34.74 | 27.65 | -7.09 | -20.4% ✅ |
| | Execution Duration | 46.13 | 40.35 | -5.78 | -12.5% ✅ |
| **Client CommonJS** | Load Duration | 30.43 | 24.48 | -5.95 | -19.6% ✅ |
| | Execution Duration | 25.15 | 17.18 | -7.97 | -31.7% ✅ |
### Risks
The biggest risk of this change is subtle bugs introduced due to `string` vs `number` confusion. I found one subtle issue in how we rewrite `new Worker()` constructors, there could be more not covered by tests.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.3)
Can you help keep this open source service alive? 💖 Please sponsor : )