Switch let/const to var in the scanner & parser for top-levelish variables. #52832

DanielRosenwasser · 2023-02-17T22:45:54Z

This pull request regains some of the performance we lost in #51387 when we switched our emit from ES5 to ES2018 - a significant part due to TDZ checks that engines need to perform. These TDZ checks can often be eliminated, but engines probably have a hard time knowing whether a function can eliminate these checks.

By switching many of the top-level shared closure variables in the parser and scanner from let/const to var, we're able to eliminate those checks. Doing so should be fairly safe, since our functions follow the same initialization mechanism of declaring variables, and then calling the "workhorse" function afterwards.

DanielRosenwasser · 2023-02-17T22:46:23Z

@typescript-bot perf test this faster

typescript-bot · 2023-02-17T22:46:27Z

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

DanielRosenwasser · 2023-02-17T22:50:01Z

@typescript-bot perf test this faster

typescript-bot · 2023-02-17T22:50:03Z

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

Update: The results are in!

typescript-bot · 2023-02-17T23:05:16Z

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

Here they are:

Comparison Report - main..52832

_Metric	_main	₅₂₈₃₂	_Delta	_Best	_Worst	_p-value
_{Angular - node (v16.17.1, x64)}
_{Memory used}	_{359,093k (± 0.01%)}	_{358,732k (± 0.00%)}	_{-361k (- 0.10%)}	_358,719k	_358,755k	_{p=0.005 n=6}
_{Parse Time}	_{4.16s (± 0.42%)}	_{3.72s (± 0.55%)}	_{🟩-0.44s (-10.61%)}	_3.69s	_3.75s	_{p=0.005 n=6}
_{Bind Time}	_{1.24s (± 0.33%)}	_{1.22s (± 0.68%)}	_{-0.02s (- 1.34%)}	_1.22s	_1.24s	_{p=0.009 n=6}
_{Check Time}	_{9.52s (± 0.44%)}	_{9.50s (± 0.25%)}	_~	_9.47s	_9.53s	_{p=0.332 n=6}
_{Emit Time}	_{8.10s (± 0.68%)}	_{8.05s (± 0.51%)}	_~	_7.98s	_8.08s	_{p=0.142 n=6}
_{Total Time}	_{23.02s (± 0.40%)}	_{22.50s (± 0.27%)}	_{-0.53s (- 2.28%)}	_22.40s	_22.57s	_{p=0.005 n=6}
_{Compiler-Unions - node (v16.17.1, x64)}
_{Memory used}	_{191,731k (± 0.02%)}	_{191,531k (± 0.05%)}	_{-200k (- 0.10%)}	_191,454k	_191,717k	_{p=0.013 n=6}
_{Parse Time}	_{1.81s (± 0.97%)}	_{1.57s (± 0.35%)}	_{🟩-0.24s (-13.38%)}	_1.56s	_1.57s	_{p=0.004 n=6}
_{Bind Time}	_{0.84s (± 0.48%)}	_{0.84s (± 0.48%)}	_~	_0.84s	_0.85s	_{p=1.000 n=6}
_{Check Time}	_{10.13s (± 0.70%)}	_{10.16s (± 0.69%)}	_~	_10.07s	_10.26s	_{p=0.419 n=6}
_{Emit Time}	_{3.04s (± 0.70%)}	_{3.07s (± 1.41%)}	_~	_3.01s	_3.12s	_{p=0.374 n=6}
_{Total Time}	_{15.82s (± 0.62%)}	_{15.63s (± 0.64%)}	_{-0.18s (- 1.16%)}	_15.51s	_15.80s	_{p=0.030 n=6}
_{Monaco - node (v16.17.1, x64)}
_{Memory used}	_{343,537k (± 0.01%)}	_{343,235k (± 0.01%)}	_{-302k (- 0.09%)}	_343,196k	_343,251k	_{p=0.005 n=6}
_{Parse Time}	_{3.10s (± 0.53%)}	_{2.78s (± 0.42%)}	_{🟩-0.31s (-10.17%)}	_2.77s	_2.80s	_{p=0.005 n=6}
_{Bind Time}	_{1.11s (± 0.73%)}	_{1.11s (± 0.00%)}	_~	_1.11s	_1.11s	_{p=0.290 n=6}
_{Check Time}	_{7.79s (± 0.70%)}	_{7.80s (± 0.38%)}	_~	_7.76s	_7.83s	_{p=0.746 n=6}
_{Emit Time}	_{4.51s (± 0.33%)}	_{4.50s (± 0.50%)}	_~	_4.48s	_4.54s	_{p=0.796 n=6}
_{Total Time}	_{16.51s (± 0.37%)}	_{16.20s (± 0.24%)}	_{-0.32s (- 1.92%)}	_16.13s	_16.24s	_{p=0.005 n=6}
_{TFS - node (v16.17.1, x64)}
_{Memory used}	_{299,646k (± 0.00%)}	_{299,372k (± 0.00%)}	_{-274k (- 0.09%)}	_299,356k	_299,381k	_{p=0.005 n=6}
_{Parse Time}	_{2.45s (± 1.08%)}	_{2.18s (± 0.73%)}	_{🟩-0.27s (-10.89%)}	_2.15s	_2.19s	_{p=0.004 n=6}
_{Bind Time}	_{1.26s (± 0.65%)}	_{1.26s (± 0.82%)}	_~	_1.24s	_1.27s	_{p=0.932 n=6}
_{Check Time}	_{7.23s (± 0.43%)}	_{7.22s (± 0.45%)}	_~	_7.17s	_7.25s	_{p=0.686 n=6}
_{Emit Time}	_{4.21s (± 0.47%)}	_{4.22s (± 1.08%)}	_~	_4.16s	_4.29s	_{p=0.570 n=6}
_{Total Time}	_{15.14s (± 0.29%)}	_{14.88s (± 0.36%)}	_{-0.27s (- 1.76%)}	_14.82s	_14.98s	_{p=0.005 n=6}
_{material-ui - node (v16.17.1, x64)}
_{Memory used}	_{475,972k (± 0.00%)}	_{475,695k (± 0.01%)}	_{-278k (- 0.06%)}	_475,656k	_475,720k	_{p=0.005 n=6}
_{Parse Time}	_{3.68s (± 0.24%)}	_{3.33s (± 0.52%)}	_{🟩-0.35s (- 9.56%)}	_3.31s	_3.36s	_{p=0.005 n=6}
_{Bind Time}	_{1.02s (± 0.62%)}	_{1.02s (± 0.00%)}	_~	_1.02s	_1.02s	_{p=1.000 n=6}
_{Check Time}	_{18.31s (± 1.24%)}	_{18.21s (± 0.54%)}	_~	_18.11s	_18.34s	_{p=0.572 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{23.01s (± 0.97%)}	_{22.56s (± 0.49%)}	_{-0.45s (- 1.96%)}	_22.46s	_22.69s	_{p=0.005 n=6}
_{xstate - node (v16.17.1, x64)}
_{Memory used}	_{546,950k (± 0.02%)}	_{546,050k (± 0.01%)}	_{-901k (- 0.16%)}	_545,981k	_546,178k	_{p=0.005 n=6}
_{Parse Time}	_{4.77s (± 0.54%)}	_{4.24s (± 0.23%)}	_{🟩-0.53s (-11.11%)}	_4.23s	_4.26s	_{p=0.004 n=6}
_{Bind Time}	_{1.85s (± 0.22%)}	_{1.80s (± 0.30%)}	_{-0.05s (- 2.52%)}	_1.80s	_1.81s	_{p=0.003 n=6}
_{Check Time}	_{3.07s (± 0.78%)}	_{3.08s (± 0.57%)}	_~	_3.05s	_3.10s	_{p=0.870 n=6}
_{Emit Time}	_{0.09s (± 4.45%)}	_{0.09s (± 0.00%)}	_~	_0.09s	_0.09s	_{p=0.405 n=6}
_{Total Time}	_{9.78s (± 0.45%)}	_{9.21s (± 0.26%)}	_{🟩-0.57s (- 5.84%)}	_9.18s	_9.25s	_{p=0.005 n=6}

System

_{Machine Name}	_ts-ci-ubuntu
_Platform	_{linux 5.4.0-135-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-02-17T23:05:39Z

Yay!!!

jakebailey · 2023-02-17T23:06:41Z

Aside: is it just the scanner? Can we revert parser and test only the scanner? Is it a specific variable? That would be very interesting to know.

jakebailey

bakkot · 2023-02-18T18:25:15Z

src/compiler/scanner.ts

@@ -1031,6 +1032,8 @@ export function createScanner(languageVersion: ScriptTarget,
        scanRange,
    };

+    /* eslint-disable no-var */


Drive-by - was this maybe supposed to be eslint-enable? It's already disabled above, and below there's no new vars.

Uh oh, sure was!

Switch let to var in the parser for top-levelish variables.

450cfe1

Switch more in parser to var, switch scanner shared state to var too.

cd8b3a6

DanielRosenwasser changed the title ~~Switch let to var in the parser for top-levelish variables.~~ Switch let to var in the scanner & parser for top-levelish variables. Feb 17, 2023

DanielRosenwasser changed the title ~~Switch let to var in the scanner & parser for top-levelish variables.~~ Switch let/const to var in the scanner & parser for top-levelish variables. Feb 17, 2023

DanielRosenwasser marked this pull request as ready for review February 17, 2023 23:06

This was referenced Feb 17, 2023

[experiment] Run babel over output for let/const to var #52656

Draft

Switch let/const to var in the scanner for top-levelish variables. #52833

Closed

jakebailey approved these changes Feb 18, 2023

View reviewed changes

typescript-bot assigned DanielRosenwasser Feb 18, 2023

typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Feb 18, 2023

DanielRosenwasser merged commit 0463766 into main Feb 18, 2023

DanielRosenwasser deleted the letToVarInParser branch February 18, 2023 01:07

DanielRosenwasser mentioned this pull request Feb 18, 2023

Swap closure state in the type-checker to var to avoid TDZ checks. #52835

Merged

bakkot reviewed Feb 18, 2023

View reviewed changes

jakebailey mentioned this pull request Feb 21, 2023

Switch to var in binder for top level variables #52903

Merged

DanielRosenwasser mentioned this pull request Feb 22, 2023

Using var in limited contexts to avoid runtime TDZ checks #52924

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch let/const to var in the scanner & parser for top-levelish variables. #52832

Switch let/const to var in the scanner & parser for top-levelish variables. #52832

DanielRosenwasser commented Feb 17, 2023 •

edited

DanielRosenwasser commented Feb 17, 2023

typescript-bot commented Feb 17, 2023 •

edited

DanielRosenwasser commented Feb 17, 2023

typescript-bot commented Feb 17, 2023 •

edited

typescript-bot commented Feb 17, 2023

jakebailey commented Feb 17, 2023

jakebailey commented Feb 17, 2023

jakebailey left a comment

bakkot Feb 18, 2023 •

edited

jakebailey Feb 18, 2023

Switch let/const to var in the scanner & parser for top-levelish variables. #52832

Switch let/const to var in the scanner & parser for top-levelish variables. #52832

Conversation

DanielRosenwasser commented Feb 17, 2023 • edited

DanielRosenwasser commented Feb 17, 2023

typescript-bot commented Feb 17, 2023 • edited

DanielRosenwasser commented Feb 17, 2023

typescript-bot commented Feb 17, 2023 • edited

typescript-bot commented Feb 17, 2023

jakebailey commented Feb 17, 2023

jakebailey commented Feb 17, 2023

jakebailey left a comment

Choose a reason for hiding this comment

bakkot Feb 18, 2023 • edited

Choose a reason for hiding this comment

jakebailey Feb 18, 2023

Choose a reason for hiding this comment

DanielRosenwasser commented Feb 17, 2023 •

edited

typescript-bot commented Feb 17, 2023 •

edited

typescript-bot commented Feb 17, 2023 •

edited

bakkot Feb 18, 2023 •

edited