[pull] main from react:main - #643
Merged
Merged
Conversation
…7225) ## Summary By replicating the `IndexSet` behavior with a simple inline array, this is able to avoid any heap allocation / memory thrash for `AbstractValue`. It also reduces the size of AbstractValue from 72 bytes + all of the heap allocations, to only 18 bytes. ## How did you test this change? Ran all of the fixtures to confirm byte output is identical. Effects on memory and compile time: | Benchmark | Peak allocation | Allocation count | Wall time | |------------------|-----------------------------|------------------|-----------| | legacy/image.tsx | 58.21 -> 33.40 MiB (-42.6%) | -51.1% | -39.5% | | next-client | 58.21 -> 33.40 (-42.6%) | -40.5% | -25.1% | | devtools | 26.39 -> 16.29 (-38.3%) | -26.7% | -14.8% | | fixtures | 17.35 → 9.41 (-45.8%) | -9.9% | -9.0% |
## Summary In the mutation / aliasing inference, the state is used twice, and both get cloned. Only one clone is necessary, the second can be safely moved. This has minimal impact on peak memory usage, but it does reduce wall time by *10%* on real Next.js benchmark apps. ## How did you test this change? Ran against benchmark apps to confirm identical byte output. Ran against the 1800+ Rust Compiler test fixtures.
tl;dr this reduces peak memory allocation by 5-15%, and reduces codegen time by 30-70% depending on payload. On heavy components with deep ASTs the impact is more exaggerated. ## Summary Codegen of temp vars records the expressions they replaced, so that they can be unwound. In the TS version this uses a `Map` and stores pointers to AST nodes - relatively cheap. For borrow-checking reasons, the Rust version clones the AST. This results in recurring deep clones, making codegen accidentally quadratic and using significant amounts of memory. This introduces a convenience data structure for emitting temp vars in an unwindable manner, without heavy AST allocation. It also avoids a separate AST deep clone when propagating null values. ## How did you test this change? All fixtures pass with byte-identical outputs. Ran this against real codebases and pathological benchmark cases, confirming byte-identical output as well.
#37357) This follows #37315, which added the render lifetime controller to bound the abort listener that `attachAbortSignal` attaches to a caller's signal. `RequestInstance` constructed one for every request, so a render that is given no signal allocated a controller, aborted it on completion, and nothing ever observed either. The controller is now created in `attachAbortSignal`, and the three places that end the lifetime go through `endRenderLifetime`, which does nothing when there is no controller. Callers that pass a signal are unaffected. Callers that do not no longer allocate one, and `signal` is optional in every browser, edge and static entry point, while `renderToPipeableStream` and `resumeToPipeableStream` accept no signal at all. They also no longer reach `AbortController` at all, which matters more than the allocation. Fizz had no runtime dependency on it before #37315, and an unconditional one reaches environments that provide the API through a polyfill. An incomplete polyfill can then fail a render that never asked for abort support. The new test asserts that no controller is constructed when no signal is passed. It fails with the eager construction restored, since nothing else in the suite would notice a regression to it.
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.4)
Can you help keep this open source service alive? 💖 Please sponsor : )