Only collect outermost intra expression inference sites #54186

Andarist · 2023-05-08T20:31:10Z

It's a potential perf improvement. However, I don't expect this to yield significant gains in the automated tests - this can probably only show in more pathological cases with big objects used as arguments. It happens that in XState we deal with such so maybe I would be able to gather some more concrete data based on that.

At the moment, in a case like this:

declare function fn<T>(args: {
  foo: {
    bar: {
      baz: {
        producer: (arg: unknown) => T;
      };
    };
  };
  consumer: (val: T) => unknown;
}): T;

fn({
  foo: { // 4
    bar: { // 3
      baz: { // 2
        producer: (arg) => 1, // 1 (function itself)
      },
    },
  },
  consumer: (val) => {},
});

we collect four intra expression inference sites. The innermost function (site 1) "taints" all the objects containing it (minus the root one). This is because isContextSensitive is recursive.

This means that isContextSensitive has to be computed at all of those levels and that we end up inferring from all of those levels separately within inferFromIntraExpressionSites. Most of that work is more than duplicated here since the outermost expression contains all of the information that can be found in the expressions contained in it.

typescript-bot · 2023-05-08T20:31:18Z

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

typescript-bot · 2023-05-08T20:31:19Z

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

jakebailey · 2023-05-08T20:45:32Z

The perf machine is currently offline but I'll run that once it's back again and caught up. If you can show a diff before/after in your xstate case that'd be helpful!

@typescript-bot test this
@typescript-bot test top100
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot pack this

typescript-bot · 2023-05-08T20:45:36Z

Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 2aa9fe4. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-05-08T20:45:36Z

Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 2aa9fe4. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-05-08T20:45:36Z

Heya @jakebailey, I've started to run the tarball bundle task on this PR at 2aa9fe4. You can monitor the build here.

typescript-bot · 2023-05-08T20:45:36Z

Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 2aa9fe4. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-05-08T20:45:36Z

Heya @jakebailey, I've started to run the extended test suite on this PR at 2aa9fe4. You can monitor the build here.

typescript-bot · 2023-05-08T20:47:23Z

Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/153998/artifacts?artifactName=tgz&fileId=E36702A34E86C3816B3C2F07FEF8A0CF4762934C08DD8495071314DAEC9DED1002&fileName=/typescript-5.1.0-insiders.20230508.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

typescript-bot · 2023-05-08T21:14:19Z

@jakebailey Here are the results of running the user test suite comparing main and refs/pull/54186/merge:

There were infrastructure failures potentially unrelated to your change:

1 instance of "Unknown failure"
1 instance of "Package install failed"

Otherwise...

Everything looks good!

typescript-bot · 2023-05-08T22:30:05Z

@jakebailey Here are the results of running the top-repos suite comparing main and refs/pull/54186/merge:

Everything looks good!

typescript-bot · 2023-05-08T22:52:54Z

Hey @jakebailey, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

jakebailey · 2023-05-09T16:18:39Z

@typescript-bot perf test this faster

typescript-bot · 2023-05-09T16:18:43Z

Heya @jakebailey, I've started to run the abridged perf test suite on this PR at a8a2ff5. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-05-09T16:35:04Z

@jakebailey
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..54186

_Metric	_main	₅₄₁₈₆	_Delta	_Best	_Worst	_p-value
_{Angular - node (v16.17.1, x64)}
_{Memory used}	_{365,382k (± 0.01%)}	_{365,389k (± 0.02%)}	_~	_365,303k	_365,432k	_{p=1.000 n=6}
_{Parse Time}	_{3.55s (± 0.38%)}	_{3.55s (± 0.65%)}	_~	_3.52s	_3.58s	_{p=1.000 n=6}
_{Bind Time}	_{1.17s (± 0.44%)}	_{1.17s (± 0.88%)}	_~	_1.16s	_1.19s	_{p=0.928 n=6}
_{Check Time}	_{9.62s (± 0.40%)}	_{9.55s (± 0.43%)}	_{-0.07s (- 0.73%)}	_9.51s	_9.63s	_{p=0.019 n=6}
_{Emit Time}	_{7.93s (± 0.50%)}	_{7.91s (± 0.41%)}	_~	_7.85s	_7.94s	_{p=0.226 n=6}
_{Total Time}	_{22.27s (± 0.24%)}	_{22.18s (± 0.21%)}	_{-0.08s (- 0.38%)}	_22.14s	_22.26s	_{p=0.030 n=6}
_{Compiler-Unions - node (v16.17.1, x64)}
_{Memory used}	_{193,362k (± 0.70%)}	_{192,766k (± 0.04%)}	_~	_192,696k	_192,866k	_{p=0.173 n=6}
_{Parse Time}	_{1.59s (± 1.72%)}	_{1.60s (± 0.66%)}	_~	_1.58s	_1.61s	_{p=0.935 n=6}
_{Bind Time}	_{0.82s (± 0.77%)}	_{0.82s (± 0.99%)}	_~	_0.81s	_0.83s	_{p=0.432 n=6}
_{Check Time}	_{10.36s (± 0.50%)}	_{10.28s (± 0.51%)}	_{-0.08s (- 0.76%)}	_10.23s	_10.37s	_{p=0.036 n=6}
_{Emit Time}	_{3.02s (± 0.77%)}	_{3.01s (± 0.71%)}	_~	_2.98s	_3.04s	_{p=0.683 n=6}
_{Total Time}	_{15.78s (± 0.50%)}	_{15.71s (± 0.26%)}	_~	_15.67s	_15.79s	_{p=0.076 n=6}
_{Monaco - node (v16.17.1, x64)}
_{Memory used}	_{345,845k (± 0.01%)}	_{345,854k (± 0.01%)}	_~	_345,830k	_345,882k	_{p=0.575 n=6}
_{Parse Time}	_{2.74s (± 0.64%)}	_{2.72s (± 0.60%)}	_~	_2.71s	_2.75s	_{p=0.117 n=6}
_{Bind Time}	_{1.08s (± 0.48%)}	_{1.09s (± 0.50%)}	_~	_1.08s	_1.09s	_{p=0.640 n=6}
_{Check Time}	_{7.89s (± 0.33%)}	_{7.84s (± 0.36%)}	_{-0.05s (- 0.63%)}	_7.81s	_7.87s	_{p=0.020 n=6}
_{Emit Time}	_{4.53s (± 1.23%)}	_{4.46s (± 0.27%)}	_{-0.06s (- 1.44%)}	_4.45s	_4.48s	_{p=0.015 n=6}
_{Total Time}	_{16.25s (± 0.45%)}	_{16.12s (± 0.28%)}	_{-0.13s (- 0.79%)}	_16.06s	_16.17s	_{p=0.020 n=6}
_{TFS - node (v16.17.1, x64)}
_{Memory used}	_{300,094k (± 0.01%)}	_{300,097k (± 0.01%)}	_~	_300,051k	_300,139k	_{p=0.936 n=6}
_{Parse Time}	_{2.17s (± 1.00%)}	_{2.16s (± 0.65%)}	_~	_2.14s	_2.18s	_{p=0.870 n=6}
_{Bind Time}	_{1.24s (± 0.98%)}	_{1.23s (± 1.11%)}	_~	_1.22s	_1.25s	_{p=0.868 n=6}
_{Check Time}	_{7.28s (± 0.34%)}	_{7.27s (± 0.55%)}	_~	_7.22s	_7.33s	_{p=0.803 n=6}
_{Emit Time}	_{4.38s (± 0.47%)}	_{4.37s (± 0.37%)}	_~	_4.35s	_4.39s	_{p=0.121 n=6}
_{Total Time}	_{15.07s (± 0.37%)}	_{15.04s (± 0.41%)}	_~	_14.96s	_15.10s	_{p=0.295 n=6}
_{material-ui - node (v16.17.1, x64)}
_{Memory used}	_{481,628k (± 0.02%)}	_{481,660k (± 0.01%)}	_~	_481,550k	_481,722k	_{p=0.298 n=6}
_{Parse Time}	_{3.26s (± 0.39%)}	_{3.24s (± 0.37%)}	_~	_3.23s	_3.26s	_{p=0.067 n=6}
_{Bind Time}	_{0.94s (± 0.67%)}	_{0.93s (± 0.44%)}	_{-0.01s (- 0.89%)}	_0.93s	_0.94s	_{p=0.033 n=6}
_{Check Time}	_{18.04s (± 0.71%)}	_{17.88s (± 0.43%)}	_{-0.16s (- 0.91%)}	_17.80s	_17.99s	_{p=0.024 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{22.24s (± 0.59%)}	_{22.06s (± 0.39%)}	_{-0.19s (- 0.84%)}	_21.97s	_22.17s	_{p=0.025 n=6}
_{xstate - node (v16.17.1, x64)}
_{Memory used}	_{560,483k (± 0.01%)}	_{560,400k (± 0.02%)}	_~	_560,313k	_560,518k	_{p=0.128 n=6}
_{Parse Time}	_{4.01s (± 0.32%)}	_{3.99s (± 0.41%)}	_~	_3.97s	_4.02s	_{p=0.071 n=6}
_{Bind Time}	_{1.76s (± 0.31%)}	_{1.75s (± 0.47%)}	_{-0.01s (- 0.66%)}	_1.75s	_1.77s	_{p=0.027 n=6}
_{Check Time}	_{3.06s (± 0.43%)}	_{3.05s (± 0.64%)}	_~	_3.02s	_3.07s	_{p=0.406 n=6}
_{Emit Time}	_{0.09s (± 7.02%)}	_{0.09s (± 4.62%)}	_~	_0.08s	_0.09s	_{p=0.673 n=6}
_{Total Time}	_{8.92s (± 0.28%)}	_{8.89s (± 0.29%)}	_~	_8.86s	_8.92s	_{p=0.065 n=6}

System

_{Machine Name}	_ts-ci-ubuntu
_Platform	_{linux 5.4.0-148-generic}
_Architecture	_x64
_{Available Memory}	_{16 GB}
_{Available Memory}	_{15 GB}
_CPUs	_{4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz}

Hosts

node (v16.17.1, x64)

Scenarios

Angular - node (v16.17.1, x64)
Compiler-Unions - node (v16.17.1, x64)
Monaco - node (v16.17.1, x64)
TFS - node (v16.17.1, x64)
material-ui - node (v16.17.1, x64)
xstate - node (v16.17.1, x64)

_Benchmark	_Name	_Iterations
_Current	₅₄₁₈₆	₆
_Baseline	_main	₆

Developer Information:

Download Benchmark

jakebailey · 2023-05-09T17:06:14Z

@typescript-bot perf test this faster

typescript-bot · 2023-05-09T17:06:17Z

Heya @jakebailey, I've started to run the abridged perf test suite on this PR at a8a2ff5. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-05-09T22:45:38Z

@jakebailey
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..54186

_Metric	_main	₅₄₁₈₆	_Delta	_Best	_Worst	_p-value
_{Angular - node (v16.17.1, x64)}
_{Memory used}	_{365,269k (± 0.00%)}	_{365,399k (± 0.01%)}	_{+131k (+ 0.04%)}	_365,365k	_365,447k	_{p=0.005 n=6}
_{Parse Time}	_{3.56s (± 0.43%)}	_{3.57s (± 0.78%)}	_~	_3.54s	_3.62s	_{p=0.869 n=6}
_{Bind Time}	_{1.18s (± 1.16%)}	_{1.17s (± 0.35%)}	_~	_1.17s	_1.18s	_{p=0.528 n=6}
_{Check Time}	_{9.60s (± 0.79%)}	_{9.62s (± 0.49%)}	_~	_9.54s	_9.68s	_{p=0.747 n=6}
_{Emit Time}	_{7.86s (± 0.55%)}	_{7.95s (± 0.70%)}	_{+0.09s (+ 1.10%)}	_7.90s	_8.03s	_{p=0.030 n=6}
_{Total Time}	_{22.20s (± 0.60%)}	_{22.31s (± 0.47%)}	_~	_22.18s	_22.49s	_{p=0.229 n=6}
_{Compiler-Unions - node (v16.17.1, x64)}
_{Memory used}	_{192,661k (± 0.05%)}	_{192,786k (± 0.02%)}	_{+124k (+ 0.06%)}	_192,731k	_192,834k	_{p=0.031 n=6}
_{Parse Time}	_{1.58s (± 1.90%)}	_{1.59s (± 0.73%)}	_~	_1.58s	_1.61s	_{p=0.935 n=6}
_{Bind Time}	_{0.83s (± 0.66%)}	_{0.83s (± 0.62%)}	_~	_0.82s	_0.83s	_{p=0.640 n=6}
_{Check Time}	_{10.41s (± 0.52%)}	_{10.31s (± 0.61%)}	_{-0.10s (- 0.98%)}	_10.22s	_10.41s	_{p=0.030 n=6}
_{Emit Time}	_{3.05s (± 5.14%)}	_{3.02s (± 0.83%)}	_~	_2.99s	_3.05s	_{p=0.227 n=6}
_{Total Time}	_{15.87s (± 1.27%)}	_{15.75s (± 0.35%)}	_~	_15.70s	_15.84s	_{p=0.108 n=6}
_{Monaco - node (v16.17.1, x64)}
_{Memory used}	_{345,873k (± 0.01%)}	_{345,859k (± 0.00%)}	_~	_345,839k	_345,873k	_{p=0.199 n=6}
_{Parse Time}	_{2.72s (± 0.63%)}	_{2.73s (± 0.40%)}	_~	_2.72s	_2.74s	_{p=0.357 n=6}
_{Bind Time}	_{1.08s (± 0.95%)}	_{1.09s (± 0.37%)}	_~	_1.08s	_1.09s	_{p=0.257 n=6}
_{Check Time}	_{7.89s (± 0.31%)}	_{7.90s (± 0.44%)}	_~	_7.84s	_7.94s	_{p=0.872 n=6}
_{Emit Time}	_{4.46s (± 0.96%)}	_{4.49s (± 0.59%)}	_~	_4.45s	_4.52s	_{p=0.195 n=6}
_{Total Time}	_{16.16s (± 0.41%)}	_{16.20s (± 0.29%)}	_~	_16.13s	_16.27s	_{p=0.227 n=6}
_{TFS - node (v16.17.1, x64)}
_{Memory used}	_{300,103k (± 0.01%)}	_{300,097k (± 0.01%)}	_~	_300,054k	_300,125k	_{p=0.810 n=6}
_{Parse Time}	_{2.17s (± 0.48%)}	_{2.17s (± 0.77%)}	_~	_2.15s	_2.20s	_{p=0.801 n=6}
_{Bind Time}	_{1.23s (± 0.98%)}	_{1.24s (± 0.66%)}	_~	_1.23s	_1.25s	_{p=0.677 n=6}
_{Check Time}	_{7.29s (± 0.31%)}	_{7.28s (± 0.46%)}	_~	_7.24s	_7.31s	_{p=0.935 n=6}
_{Emit Time}	_{4.33s (± 0.75%)}	_{4.39s (± 0.95%)}	_{+0.06s (+ 1.46%)}	_4.33s	_4.43s	_{p=0.035 n=6}
_{Total Time}	_{15.02s (± 0.15%)}	_{15.08s (± 0.41%)}	_~	_14.98s	_15.16s	_{p=0.077 n=6}
_{material-ui - node (v16.17.1, x64)}
_{Memory used}	_{480,933k (± 0.01%)}	_{481,641k (± 0.01%)}	_{+708k (+ 0.15%)}	_481,596k	_481,707k	_{p=0.005 n=6}
_{Parse Time}	_{3.25s (± 0.57%)}	_{3.25s (± 0.61%)}	_~	_3.23s	_3.29s	_{p=0.868 n=6}
_{Bind Time}	_{0.94s (± 0.43%)}	_{0.93s (± 0.59%)}	_~	_0.93s	_0.94s	_{p=0.282 n=6}
_{Check Time}	_{17.91s (± 1.28%)}	_{17.94s (± 0.57%)}	_~	_17.80s	_18.06s	_{p=0.471 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{22.10s (± 1.04%)}	_{22.13s (± 0.53%)}	_~	_21.97s	_22.28s	_{p=0.471 n=6}
_{xstate - node (v16.17.1, x64)}
_{Memory used}	_{560,492k (± 0.02%)}	_{560,463k (± 0.02%)}	_~	_560,354k	_560,665k	_{p=0.689 n=6}
_{Parse Time}	_{4.01s (± 0.38%)}	_{4.01s (± 0.38%)}	_~	_4.00s	_4.04s	_{p=0.563 n=6}
_{Bind Time}	_{1.76s (± 0.62%)}	_{1.76s (± 0.59%)}	_~	_1.75s	_1.78s	_{p=0.546 n=6}
_{Check Time}	_{3.05s (± 0.67%)}	_{3.07s (± 0.87%)}	_~	_3.04s	_3.12s	_{p=0.292 n=6}
_{Emit Time}	_{0.09s (± 0.00%)}	_{0.09s (± 4.45%)}	_~	_0.09s	_0.10s	_{p=0.405 n=6}
_{Total Time}	_{8.91s (± 0.44%)}	_{8.94s (± 0.25%)}	_~	_8.92s	_8.98s	_{p=0.063 n=6}

System

_{Machine Name}	_ts-ci-ubuntu
_Platform	_{linux 5.4.0-148-generic}
_Architecture	_x64
_{Available Memory}	_{16 GB}
_{Available Memory}	_{15 GB}
_CPUs	_{4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz}

Hosts

node (v16.17.1, x64)

Scenarios

Angular - node (v16.17.1, x64)
Compiler-Unions - node (v16.17.1, x64)
Monaco - node (v16.17.1, x64)
TFS - node (v16.17.1, x64)
material-ui - node (v16.17.1, x64)
xstate - node (v16.17.1, x64)

_Benchmark	_Name	_Iterations
_Current	₅₄₁₈₆	₆
_Baseline	_main	₆

Developer Information:

Download Benchmark

weswigham

IMO, it'd be nice if we had a pathlogical test that this fixes in our test suite, eg, a constructed examples this takes from 50s (timing out) down to 5s (since, unless I'm mistaken, this is the category of change that, in constructed examples, takes something from exponential time to linear time). While this is good, since it doesn't show much change in the perf suites, without anything benching the improvement, this might get lost in a future update without such a test.

Andarist · 2023-06-05T20:08:33Z

I see that for some reason main is failing after merging this -> I'm investigating this.

jakebailey · 2023-06-05T20:11:32Z

Probably just that the branch was out of date when it was merged.

Andarist · 2023-06-05T20:17:58Z

Ye, right - I narrowed it down to a silent conflict with: #54380 , looking now if I can make it work in a reasonable time.

…osoft#54186)" This reverts commit 4749c38.

Only collect outermost intra expression inference sites

2aa9fe4

typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label May 8, 2023

Merge branch 'main' into fix/only-add-outermost-intra-expression-sites

a8a2ff5

sandersn assigned weswigham May 22, 2023

sandersn requested review from weswigham and ahejlsberg May 22, 2023 22:59

weswigham approved these changes May 24, 2023

View reviewed changes

weswigham merged commit 4749c38 into microsoft:main Jun 5, 2023

Andarist deleted the fix/only-add-outermost-intra-expression-sites branch June 5, 2023 19:43

Andarist mentioned this pull request Jun 5, 2023

Deconflict values of CheckMode.RestBindingElement and CheckMode.TypeOnly #54540

Merged

Andarist added a commit to Andarist/TypeScript that referenced this pull request Jun 12, 2023

Revert "Only collect outermost intra expression inference sites (micr…

35ede87

…osoft#54186)" This reverts commit 4749c38.

Andarist mentioned this pull request Jun 12, 2023

Revert collecting only outermost intra expression inference sites #54613

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only collect outermost intra expression inference sites #54186

Only collect outermost intra expression inference sites #54186

Andarist commented May 8, 2023

typescript-bot commented May 8, 2023

typescript-bot commented May 8, 2023

jakebailey commented May 8, 2023

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 8, 2023

typescript-bot commented May 8, 2023

typescript-bot commented May 8, 2023

jakebailey commented May 9, 2023

typescript-bot commented May 9, 2023 •

edited

Loading

typescript-bot commented May 9, 2023

jakebailey commented May 9, 2023

typescript-bot commented May 9, 2023 •

edited

Loading

typescript-bot commented May 9, 2023

weswigham left a comment

Andarist commented Jun 5, 2023

jakebailey commented Jun 5, 2023

Andarist commented Jun 5, 2023

Only collect outermost intra expression inference sites #54186

Only collect outermost intra expression inference sites #54186

Conversation

Andarist commented May 8, 2023

typescript-bot commented May 8, 2023

typescript-bot commented May 8, 2023

jakebailey commented May 8, 2023

typescript-bot commented May 8, 2023 • edited Loading

typescript-bot commented May 8, 2023 • edited Loading

typescript-bot commented May 8, 2023 • edited Loading

typescript-bot commented May 8, 2023 • edited Loading

typescript-bot commented May 8, 2023 • edited Loading

typescript-bot commented May 8, 2023 • edited Loading

typescript-bot commented May 8, 2023

typescript-bot commented May 8, 2023

typescript-bot commented May 8, 2023

jakebailey commented May 9, 2023

typescript-bot commented May 9, 2023 • edited Loading

typescript-bot commented May 9, 2023

jakebailey commented May 9, 2023

typescript-bot commented May 9, 2023 • edited Loading

typescript-bot commented May 9, 2023

weswigham left a comment

Choose a reason for hiding this comment

Andarist commented Jun 5, 2023

jakebailey commented Jun 5, 2023

Andarist commented Jun 5, 2023

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 8, 2023 •

edited

Loading

typescript-bot commented May 9, 2023 •

edited

Loading

typescript-bot commented May 9, 2023 •

edited

Loading