Repository navigation
[experiment] order independence + dynamic checkers / work stealing - #65367
Jake Bailey (jakebailey) wants to merge 43 commits into
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
33e4a7b to
50d7631
Compare
|
TypeScript Bot (@typescript-bot) run dt |
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hey Jake Bailey (@jakebailey), the results of running the DT tests are ready. There were interesting changes: Branch only errors:
|
|
Jake Bailey (@jakebailey) Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
50d7631 to
2a565a2
Compare
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
2a565a2 to
f59a1e8
Compare
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Jake Bailey (@jakebailey) Here are the results of running the top 1000 repos with tsc comparing Something interesting changed - please have a look. Details
|
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
When a test stops producing a baseline that has a reference, the harness wrote the .delete marker but did not fail the test, so the change went unnoticed until someone ran baseline-accept and lost the reference without reviewing why.
Checker results should not depend on which files were checked first, on which types an editor happened to ask for before diagnostics were requested, or on whether a file was emitted before it was checked. Tests had no way to show such dependence, so fixes for it could only be described, not demonstrated. The harness-only checkOrder option queries the checker in a different order: files in reverse, each file in its own program, or every expression's type, symbol, contextual type, resolved signature, and the types of names in type annotations before any diagnostics. The same orders apply to emit. Listing several orders produces a baseline for each, so order dependence shows up as differing baselines. Fixed orders over whole files miss results that depend on which single query a checker answers first. With TS_CHECK_ORDER_FUZZ=n, each test without a checkOrder option is checked again n times in fresh programs that first answer a seeded random selection of queries and single file emits. Any change in diagnostics, emitted output, or the type or symbol of a node fails the test with the seed and a minimized list of queries that reproduces it. Symbol IDs in printed names are ignored, as they follow creation order by design. The fuzzer is off by default, so it does not slow the normal suite.
Order independence should also mean that results do not depend on which files a checker has been given, since project and compiler checker pools assign files differently. Let tests pick the checker of every file, and have the check order fuzzer use a seeded random assignment over two to four checkers for half of its seeds.
e51f23d to
b8c46ea
Compare
Editors ask the checker for more than the types and symbols the fuzzer queries: hover, signature help, completions and inlay hints reach parts of the checker those queries do not, such as the node builder and call resolution for signature help. With TS_CHECK_ORDER_FUZZ_LS set, the fuzzer also makes these requests, and compares their results.
Declaration emit marks the declarations it depends on as visible, such as those an export assignment names. Whether a declaration is visible was cached together with those marks, so a declaration whose visibility was asked before emit marked its container, as hovering over a use of it does, stayed invisible, and emit left it out. Caching only the visibility the rules of a declaration give, and applying the marks when asked, gives the same answer whenever emit marks them.
The node builder resolves names to decide whether it can reuse a type node written in the source, which marked them referenced. Printing a type, as hovering does, then hid unused declaration errors that checking alone reports.
Resolving the name of a JSDoc link, as hovering over a documented declaration does, checks it as an expression, which marked the imports it names as referenced. Emit then kept imports that checking alone elides.
The type of a get accessor without a type annotation is the type its getter returns, but it is inferred apart from the return type of the getter's signature. Checking resolves the type of the accessor first, so a cycle through the accessor is detected and reported there. Asked for the return type of the getter first, as inlay hints do, the cycle through the accessor was detected only within the return type, whose result then hid the error. Resolving the type of the accessor first, as checking does, gives the same result either way.
A type whose get and set accessors have different types prints them as accessors, which the inlay hint for it did not expect, so asking for the inlay hints of a variable of such a type crashed.
A type declared inside a generic function or method is printed as a member of that function or method, with its type arguments. A name with type arguments before its last part is an expression with type arguments, which appending the next member to it did not expect, so printing such a type, as hovering over a variable of it does, crashed.
Whether a conditional type is definitely true is checked with type parameters replaced by restrictive forms without constraints. A type parameter needed none if it had no constraint declaration and its constraint had not been computed yet, so an infer type parameter with a constraint implied by where it appears needed one only once something had asked for its constraint. Deciding from the declaration instead gives the same answer however the constraint was reached.
A node checked again inside a check of itself, as when the call resolving its type re-enters through a callback, has a contextual type pushed for each check. The lookup took the outermost one, so the inner check of an argument saw the parameter type the outer resolution had instantiated with its inferences, rather than its own, and whether `const` type parameters made the argument readonly depended on whether its call had been resolved before. The original checker searches from the innermost, as this does now.
A comparison that used variances it could not measure reliably marks its cached result, and reusing the result marks whatever is measuring variances, just as the comparison would. The fast path through cached results in isTypeRelatedTo did not, so a variance measured after the comparison had been cached came out different from one measured before, and whether two references of a generic type were related depended on which declaration came first (64891). The declaration order test now agrees across its two orders.
The checker caches nearly everything it computes, and many results depend on the results that happened to be cached when they were computed: which file was checked first, which types an editor asked for before requesting diagnostics, whether a file was emitted before it was checked. The check order tests and fuzzer show hundreds of such differences on main. As long as they exist, checkers cannot share results, a cached result cannot be dropped and recomputed later, and concurrent checking needs separate checkers with fixed file assignments to stay deterministic. The goal is a checker that behaves like a pure function with a cache: the answer to any query is the answer of computing it from scratch. The cached state of a checker moves to a memo store, leaving the Checker with only the state of the computation in progress. Every cached result is a memo: a cell, a keyed map, or a check-once set. Reading one records it as a dependency of the computation in progress. Whether a memo holds a result depends on the results computed before, so only the computation of a result reads its memo: its getter, which reuses the result, and the computation itself. A computation reading the result a computation re-entering it cached early says so with inProgress, which asserts that it is in progress. Everything else asks the getter. A result filled in as a side output of another computation, such as the symbol a name resolves to when its expression is checked, is read through an accessor that runs that computation first, and a part an entity's creator supplies, such as the type arguments of a reference that is not deferred, through one saying so. Creators set such parts with setIdentity or intern, which are kept when a computation is discarded. Tests find any memo read outside these, and any creator setting part of an entity it creates in a way that can be discarded. Computing a result can produce more than the result: diagnostics, reference marks for unused checks, deferred nodes, required emit helpers. When a query computed a result first, those effects were either reported for code checking never visits or lost when checking reused the result. A query now records them on the result instead, along with the results it reused, and checking replays them, exactly once, when it reuses the result, as if it had computed it. Effects produced outside any memoized computation belong only to the query and are dropped. Circularities are where most of the order dependence lives. A computation that re-enters one in progress sees a provisional state, so whatever is cached until the outermost computation of the cycle finishes depends on where the cycle was entered. Those results are now discarded when it finishes, and recomputed when next requested. The computations in progress when the cycle was detected keep their fallbacks, as each reaches the cycle when computed from scratch too. Results that define identity, such as declared types and instantiations, are kept, so no second, distinct result can exist. Main detects many re-entries silently, through markers and probes rather than the resolution stack; those now go through the resolution stack too, so their cycles are seen. Where a cycle's result depends on which member was entered, the cycle is resolved from a canonical entry, the member declared first, as when checking in program order: cycles through the types of symbols, mutually dependent member tables and module tables, and type parameter defaults. Variances of circular types are measured with TypeScript's rule from the canonical type of each strongly connected component, so that they no longer depend on which type was measured first. Instantiation depth is measured from the start of each memoized computation, and a reused result adds none, so whether a result reaches the depth limit does not depend on the results cached. The count limit, which only stops runaway instantiations, still counts per statement or expression, and starts again for each outermost computation; its error is reported once, where it is reached. Smaller fixes needed for the same reason are part of this change: errors for accessor pairs are reported by each accessor without a flag, type alias type parameters are read through the declared type, name lookups that cannot match a late-bound member do not resolve late-bound members, symbols derived from the same declaration are ordered without ids, awaited types that refer back to themselves fail as a cycle, relation complexity overflows are reported in every comparison that needs them, statements not allowed in an ambient context are reported at the first of their block rather than the first checked, emit resolves the names and exported aliases it refers to rather than reading the symbols checking cached, union and intersection properties are not shared between augmented and plain lookups, a conditional type is a restrictive instantiation if it is created as one, the API computes the parts of mapped types it returns, and a call deferred while inferring from an enclosing call leaves no mark of its resolution behind.
A declared type parameter with a circular constraint is circular no matter how it is used, and checking its declaration reports it. Which use happened to resolve the constraint before that is an accident of check order, so pointing at it as the origin of the cycle made the diagnostic depend on that order. Only an instantiated type parameter has a cycle that originates at a use.
A mapped type resolves its constraint eagerly so that errors in the constraint are reported at the mapped type. It did so only when the type was created, so when the type was first created while checking another node, the mapped type itself never reported those errors.
A cycle through return types gave different results depending on where it was entered. Entered at rebuild's return type, re-entering it fails rebuild, so both members report an implicit any. Entered at b's return type, subtype reduction of rebuild's return type finds b in progress and compares as if it returned any, without failing anything, so rebuild returns B and was kept as a participant of the cycle, though computing it from scratch fails. Cycles through the types of symbols are already resolved from the member declared first (see yieldCycle). The same now applies to the return types of declared signatures, both when a cycle re-enters a return type and when it only observes one in progress, so that each order gives the result of resolving the first declared member first (seen in effect's SchemaAST). Contextually checking a function expression infers its return type from its body, and that inference was pushed as a return type resolution of the function's signature. When the body calls a function whose return type depends on the call enclosing the function expression, resolving that call requests the function expression's return type, which, seen in progress, returned any without detecting a cycle, so the enclosing function's return type was inferred from a provisional any instead of reporting the circularity (seen in OmniRoute). The inference now runs without marking the signature in progress, as in TypeScript: the nested request computes the return type itself, reaches the enclosing function, and the cycle is reported on both. The result is cached only if no request has already cached one, the same as when the return type is requested directly. The flag marking a function as contextually checked is set before its return type is inferred, and is TypeScript's guard against re-checking the function from within its own check, so it is never discarded. The inferred return type is now kept whenever the flag is, so that a discarded cycle in between cannot leave the flag without the return type, which made a const assertion in the body readonly when queried before its binding pattern.
Completing contextual signatures before anyone reads them switched parameter assignment from the signature of the function's symbol to the signature built from its declaration. In JavaScript the symbol's signature comes from a @type tag when one is present, and the function's parameters take their types from that tag. Using the declaration's signature wrote the outer contextual types onto those parameters instead, producing errors inside the function body that the tag rules out (seen in webpack). Prefer the tag's signature, as before, and fall back to the declaration's signature otherwise.
An instantiation stopped by the limit on the depth of the whole instantiation names every type that recurs in it, in the order in which they recur. Which type of the cycle the instantiation entered first depends on the results cached before, so checkers that had checked different files named the same cycle starting from different types, and reported the same error twice with different messages (seen in zod, with checkers running concurrently). The names now start from the type declared first, so the message is the same wherever the cycle is entered. The limit on the depth of a single computation already names only the types that computation instantiated, which does not depend on where it was entered, so its messages keep the order in which the types recur.
While the type implied by a binding pattern is computed as the
contextual type of its initializer, references to the pattern's
elements from the elements' defaults are any. The calls and types
computed from them were cached anyway, so asking for the contextual
type of the initializer first left `extra` in
const { kind, payload, extra = kind === "a" ? ... : ... } = u;
as any rather than unknown. TypeScript behaves the same way.
Track the patterns as resolutions in progress rather than a separate
stack, so that a reference returning any marks the computation circular
and the results cached from it are discarded.
(seen in check order fuzzing)
A call resolution hides the resolutions below it, so a symbol whose initializer reaches itself through a call argument is computed again, nested in itself, as in main. The nested computation observes things in progress that the outer one entered: here, a mapped property whose contextual type is skipped, and the reverse mapping whose depth cutoff it reaches. Those mark everything since as circular, so the nested result was discarded, and the outer computation, reaching the same types again outside that context, then failed with a circularity. When everything the nested computation observed in progress was entered by the outermost hidden resolution of the same target, the nested computation is the one that resolution always computes first from scratch, whichever entry led to it, so its result is kept as a cycle participant, as main keeps the first result set. Main also keeps the reverse-mapped property type computed under the depth cutoff, so it types the property as unknown. That result still depends on the cutoff and is discarded, so the property is computed again from scratch afterwards, giving its full type: an improvement over main rather than a match (seen in arktype).
When a query enters a cycle somewhere other than its program-order entry, the cycle yields to its first member and recomputes it with the resolutions below hidden. Two things made that recomputation differ from computing the member first: - The reverse mapping stacks, contextual types and inference contexts of the hidden resolutions stayed visible, so the recomputation saw contexts that a computation from scratch never has. A yield now hides them along with the resolutions and restores them when it ends. - Results were kept as cycle participants only by comparing the outermost cycle head in progress against the resolution. A cycle headed below the yield, which the yield cannot observe, caused the yield root and the hidden re-entries of itself to be discarded, so the member was computed again and genuinely re-entered itself. Each type resolution now tracks the lowest cycle head it observed, and a result is kept when the computations it observed were entered from scratch: by the outer resolution of itself, by the yield it is the root of, or by the outermost measurement of variances. (seen in circularScopeThunkReverseMapped)
Which symbols are referenced, which nodes need deferred or unused checks, and which aliases are referenced are facts about a node that only the check of its file consumes. Recording them as effects made nearly every query result carry effects, so emit and other queries without a check retained and walked a large effect graph: --noCheck emit of VS Code used twice the time and memory of main. Each fact now depends only on its node, and the check reaches every node, so whatever computes a fact first records it. Write-only names are not uses wherever a name is resolved. The check now also checks methods and accessors in destructuring targets and decorators where they are not valid, which only a query computing their types reached before, and follows referenced import = declarations itself once the file's references are marked, so that following them reports errors whatever cached them first. Marks are written only by checking, or by a computation a check started that entered a cycle. A query records them on its result, and they are replayed even when its other effects are dropped, as whether a computation reaches the nodes they mark does not depend on the results it read. A test finds marks written anywhere else.
When no overload applies, call resolution caches its result before reporting errors, so that a circular request for the call while they are reported finds it. Discarding that result in a cycle restored the marker put in place when resolution began, leaving the call marked as being resolved. Caching it like the final result clears the marker instead.
Signature help resolves a call from the arguments typed so far, without reporting errors, and string literal completions resolve it with inference from the argument being typed blocked. The results those requests cache depend on that state, which is not part of their keys, so checking that reuses them could infer other types or miss errors. Nothing such a request caches is kept: it runs in a cycle headed below every computation, which ends with the request. The state of such a request, the number of arguments typed so far and the nodes inference is blocked from, is held in a request that exists only while it runs, rather than in fields of the checker that any computation could read.
Files are assigned to checkers up front, so the checker with the most work left finishes last while the others sit idle. Results no longer depend on which checker checks a file or which files it checked before, so a checker that runs out of files can take one from another without changing any output. Each checker still starts with the files assigned to it, keeping the locality of the assignment, and takes the file another checker would reach last. A file stays associated with the checker that checked it, so later requests for it reuse that checker's caches.
The checker count was fixed at 4 because which checker checked a file could change the results, so changing it changed the output. That is no longer the case, and with checkers taking files from each other it only trades time for memory, so the default can follow the machine. Checking is fastest with about one checker per CPU, until the work every checker duplicates (library and shared types) outweighs the parallelism another checker adds. Measured on 4 and 8 CPUs, one checker per logical CPU was fastest for nearly every project, hyperthreads included; on 20 CPUs, 8 checkers were fastest for most projects, and only very large programs gained up to 12-16. So the default is one checker per CPU, up to 8, rising with the square root of the checked work up to 16, and no more than one per 32 checked files, but at least the 4 of before when there are as many CPUs. Files whose checking is skipped, such as declaration files under skipLibCheck, count for nothing. Build mode divides the CPUs among the projects it builds concurrently, but never gives a project fewer than the 4 checkers of before, since projects that depend on each other are built one at a time.
b8c46ea to
60ca9ed
Compare
|
TypeScript Bot (@typescript-bot) run dt |
|
Hey Jake Bailey (@jakebailey), the results of running the DT tests are ready. There were interesting changes: Branch only errors:
|
|
Jake Bailey (@jakebailey) Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This is #65064 + more dynamic checker counts + work stealing between checkers, as this should actually be safe now.