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

Design Meeting Notes, 4/5/2024 #58096

Open
DanielRosenwasser opened this issue Apr 5, 2024 · 0 comments
Open

Design Meeting Notes, 4/5/2024 #58096

DanielRosenwasser opened this issue Apr 5, 2024 · 0 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 5, 2024

Control Flow Node Monomorphization

#57977

  • All CFG nodes (FlowNodes) are now created by the same function, and all have the same shape.
  • 4 fields: flags, id, node (basically a payload), antecedent (a possible node or array of nodes)
    • node might be an syntax node or some other piece of data (e.g. FlowSwitchClauseData)
  • The effect of this was probably 0.5-1% of all compile time, but every little bit helps!

Removing Effects-Free Blocks from Control Flow Graph

#58013

  • The control flow graph is often filled with nodes that have no effect over subsequent nodes.
    • In some cases, "downstream" nodes would be better-served to skip over certain nodes.
  • If a stretch of flow nodes has no effect or interruption/termination, then its antecedent can be used in place of the node itself.
    • What is an effect/termination?
      • Assignments like x = ...
      • throw or return
      • Top-level functions (they could be assertion functions like f(x: any): asserts x or exit(): never)
  • Does this change mean that you can't walk back from every termination point to find all references?
    • No
    • Because we were thinking about using the CFG for definite assignment
      • Well that you can do!
  • This eliminates stuff like optional chains and lots of different things that can cause the graph to be bushy
    • It doesn't really affect ifs since most of those have effects.
    • In fact, the PR might not even bother to optimize those if statements?
      • Tried this in the compiler, very few cases where this optimization would kick in.

Region-Based Semantic Diagnostics

#57393
#57842

  • Today, when the editor sends a request for diagnostics (errors), it sends a list of files that are open.
    • TypeScript sends syntactic diagnostics from the parser, and then semantic diagnostics from the type checker.
  • With this PR, the editor can specify a visible region.
    • The syntactic diagnostics are sent in-whole, but the semantic diagnostics may come back in a partial state followed by the full diagnostics for a specific file.
  • For checker.ts, a full check of the file shrinks down from ~3300ms to ~140ms.
  • But what are the tradeoffs?
    • Users can see different diagnostics based on checking order.
      • How much we elaborate, specific error messages, etc.
  • This is a tradeoff between speed and accuracy.
    • The user can see the diagnostics faster, but they might not be the same as what would have happened checking top-down.
  • One thing to think about is that we can cache the diagnostics themselves.
  • Are all of the cases we decide not to do elaboration just cases where we're relating?
    • It's mostly type comparison because we don't want to do structural checks.
  • One of the reasons we decided not to elaborate was in the command-line compiler, where you don't want these massive errors repeated over and over.
    • Not sure if we agree with that in the editor scenarios.
  • One of the issues was is that elaboration is relatively expensive, and the cache is supposed to avoid this work.
    • But the intermediate diagnostics are cached and they were made to work this way - just linked lists of elaborations.
  • How does this work with an input range that partially overlaps with a range?
    • Finds the list of source elements that overlaps.
  • What is the bigger UX problem here?
    • The deduplication doesn't work because the diagnostics differ between checks.
      • Could try to exclude the original span of nodes that were originally checked.
        • Might be possible to do that, but there are checks we can only do when we look at the whole file.
          • Export checks, for example.
          • Still worth exploring.
    • So you end up with two close-ish error message that are actually caused by the same error.
    • The position of the error might also be different.
      • e.g. JSX errors that get issued on the first-checked JSX tag rather than the first JSX tag in the file.
  • TSServer ultimately decides what to send back to the editor.
    • Could decide to check the whole file instead of the region if the region is the entire file.
    • Could also decide to check the whole file if the region is small enough.
  • Is there a way to know if noticeable inconsistencies kick in when doing the full check sweep?
    • We've been testing this with the top 200 repos.
@DanielRosenwasser DanielRosenwasser added the Design Notes Notes from our design meetings label Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Notes Notes from our design meetings
Projects
None yet
Development

No branches or pull requests

1 participant