[pull] canary from vercel:canary - #1348
Merged
Merged
Conversation
Followup to #95310 Purely a refactoring. These functions were 99% identical
) Improve with-docker example env var usage documentation. Closes: #97959
…port branches (#97991) The previous detection mechanism special-cased `canary` which lead to large diffs when backport branches ran CI. We stop special casing `canary` reducing complexity and giving us another flake-detecting attempt when the change is merged. That way we can safely enable flake detection on backport branches without having to test large diffs when PRs are merged targetting backport branches. New backport branches will no longer have flake detecton and new deploy test runs disabled.
Followup to #95310 Now we can correctly track env vars in cases like these were a `JsValue::Alternative` is involved ```js // An alternative (process|unknown).env let p2 if (foo) { p2 = foo } else { p2 = process } console.log(p2.env.FOO8) ``` The inlining logic behaves the same as before: - If `get_definable_name` returns exactly one result, we can inline. - Otherwise, we can't inline (previously this case returned `None`) A bit of trivia why we need that boolean in the return type in the first place (because conceptually it should be possible to just model it via `JsValue::Alternative` anyway: > but why is that a problem? if the value is reassigned, then there should just an alternative, then get_definable_name returns multiple values and it should still not be inlining > > Because module is a `FreeVar`, not a normal `Variable`. > > The assignment is stored separately in `VarGraph`: > > ``` > free_var_ids["module"] -> id > values[id] -> assigned function > ``` > > But later reads still evaluate to: > > `JsValue::FreeVar("module")` > > The linker expands JsValue::Variable, but not JsValue::FreeVar. Therefore there is no JsValue::Alternatives at this call site. > > Instead, `get_definable_name()` detects the graph entry and returns: > > `[Some((["module", TypeOf], true))]` > > That boolean is how reassignment is represented for free variables. Cardinality remains one, so ignoring the boolean at references/mod.rs:4007 incorrectly permits inlining. > > Actual `Alternatives` primarily represent multiple values assigned to tracked variables. For free globals, the original ambient value plus reassignment is represented by `FreeVar` plus `potentially_reassigned`. No perf impact: ``` commit cfc7da3 (HEAD -> canary, origin/canary, origin/HEAD) 393.17s user 31.34s system 722% cpu 58.775 total 391.91s user 35.37s system 746% cpu 57.256 total 389.48s user 30.40s system 770% cpu 54.513 total commit 38f2708 (HEAD -> mischnic/get-definable-name-list) 392.86s user 34.00s system 745% cpu 57.221 total 388.10s user 32.72s system 766% cpu 54.912 total 391.84s user 37.82s system 717% cpu 59.865 total ```
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.
3 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 : )