Fix incorrect flags in modifierFlagsCache #56198

rbuckton · 2023-10-24T00:35:34Z

Due to the overlap between syntactic and JSDoc modifiers, there exists a case where what we report as cached syntactic modifiers can change in a JS file. To avoid this conflict, this PR modifies the caching behavior for modifier flags to give JSDoc modifiers their own cache value when they overlap with a syntactic modifier. The cache-only modifiers are then converted back to what we normally produce for those modifiers so as not to break existing callers.

Fixes #42189

typescript-bot · 2023-10-24T00:35:51Z

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

rbuckton · 2023-10-24T00:46:27Z

@typescript-bot perf test

typescript-bot · 2023-10-24T00:46:29Z

Heya @rbuckton, I've started to run the regular perf test suite on this PR at 428d327. You can monitor the build here.

Update: The results are in!

rbuckton · 2023-10-24T00:46:38Z

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

typescript-bot · 2023-10-24T00:46:41Z

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

Update: The results are in!

typescript-bot · 2023-10-24T00:46:41Z

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

Update: The results are in!

typescript-bot · 2023-10-24T01:03:25Z

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

There were infrastructure failures potentially unrelated to your change:

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

Otherwise...

Everything looks good!

typescript-bot · 2023-10-24T01:10:58Z

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

Here they are:

Compiler

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{Angular - node (v18.15.0, x64)}
_{Memory used}	_{295,097k (± 0.02%)}	_{295,069k (± 0.01%)}	_~	_295,033k	_295,099k	_{p=0.297 n=6}
_{Parse Time}	_{2.63s (± 0.37%)}	_{2.64s (± 0.32%)}	_~	_2.63s	_2.65s	_{p=0.340 n=6}
_{Bind Time}	_{0.83s (± 0.62%)}	_{0.86s (± 0.87%)}	_{+0.03s (+ 3.40%)}	_0.85s	_0.87s	_{p=0.004 n=6}
_{Check Time}	_{8.04s (± 0.17%)}	_{8.12s (± 0.44%)}	_{+0.07s (+ 0.91%)}	_8.05s	_8.15s	_{p=0.014 n=6}
_{Emit Time}	_{7.05s (± 0.32%)}	_{7.05s (± 0.29%)}	_~	_7.02s	_7.08s	_{p=1.000 n=6}
_{Total Time}	_{18.56s (± 0.07%)}	_{18.67s (± 0.23%)}	_{+0.11s (+ 0.58%)}	_18.61s	_18.72s	_{p=0.004 n=6}
_{Compiler-Unions - node (v18.15.0, x64)}
_{Memory used}	_{192,583k (± 1.55%)}	_{192,601k (± 1.56%)}	_~	_190,632k	_196,547k	_{p=0.689 n=6}
_{Parse Time}	_{1.35s (± 0.56%)}	_{1.34s (± 0.94%)}	_~	_1.32s	_1.36s	_{p=0.167 n=6}
_{Bind Time}	_{0.73s (± 0.00%)}	_{0.74s (± 0.00%)}	_{+0.01s (+ 1.37%)}	_0.74s	_0.74s	_{p=0.001 n=6}
_{Check Time}	_{9.13s (± 0.35%)}	_{9.21s (± 0.22%)}	_{+0.08s (+ 0.82%)}	_9.18s	_9.24s	_{p=0.008 n=6}
_{Emit Time}	_{2.64s (± 0.39%)}	_{2.63s (± 0.47%)}	_~	_2.61s	_2.64s	_{p=0.128 n=6}
_{Total Time}	_{13.85s (± 0.23%)}	_{13.92s (± 0.26%)}	_{+0.07s (+ 0.51%)}	_13.86s	_13.97s	_{p=0.012 n=6}
_{Monaco - node (v18.15.0, x64)}
_{Memory used}	_{347,318k (± 0.01%)}	_{347,338k (± 0.00%)}	_~	_347,320k	_347,359k	_{p=0.173 n=6}
_{Parse Time}	_{2.45s (± 1.33%)}	_{2.46s (± 0.21%)}	_~	_2.46s	_2.47s	_{p=1.000 n=6}
_{Bind Time}	_{0.95s (± 1.72%)}	_{0.97s (± 0.42%)}	_{+0.02s (+ 2.64%)}	_0.97s	_0.98s	_{p=0.038 n=6}
_{Check Time}	_{6.92s (± 0.58%)}	_{7.04s (± 0.15%)}	_{+0.12s (+ 1.73%)}	_7.03s	_7.06s	_{p=0.005 n=6}
_{Emit Time}	_{4.02s (± 0.29%)}	_{4.01s (± 0.34%)}	_~	_3.99s	_4.03s	_{p=0.563 n=6}
_{Total Time}	_{14.34s (± 0.25%)}	_{14.49s (± 0.10%)}	_{+0.15s (+ 1.02%)}	_14.47s	_14.51s	_{p=0.005 n=6}
_{TFS - node (v18.15.0, x64)}
_{Memory used}	_{302,567k (± 0.01%)}	_{302,578k (± 0.00%)}	_~	_302,570k	_302,586k	_{p=0.199 n=6}
_{Parse Time}	_{2.00s (± 0.75%)}	_{2.00s (± 1.08%)}	_~	_1.97s	_2.03s	_{p=0.567 n=6}
_{Bind Time}	_{1.01s (± 0.97%)}	_{1.04s (± 0.50%)}	_{+0.03s (+ 2.81%)}	_1.03s	_1.04s	_{p=0.004 n=6}
_{Check Time}	_{6.25s (± 0.39%)}	_{6.29s (± 0.35%)}	_~	_6.25s	_6.31s	_{p=0.052 n=6}
_{Emit Time}	_{3.58s (± 0.88%)}	_{3.56s (± 0.46%)}	_~	_3.54s	_3.58s	_{p=0.367 n=6}
_{Total Time}	_{12.84s (± 0.24%)}	_{12.88s (± 0.36%)}	_~	_12.79s	_12.91s	_{p=0.106 n=6}
_{material-ui - node (v18.15.0, x64)}
_{Memory used}	_{470,475k (± 0.00%)}	_{470,567k (± 0.01%)}	_{+93k (+ 0.02%)}	_470,525k	_470,620k	_{p=0.005 n=6}
_{Parse Time}	_{2.57s (± 0.25%)}	_{2.59s (± 0.24%)}	_{+0.02s (+ 0.78%)}	_2.58s	_2.60s	_{p=0.005 n=6}
_{Bind Time}	_{1.00s (± 1.17%)}	_{1.05s (± 0.93%)}	_{🔻+0.05s (+ 5.34%)}	_1.04s	_1.06s	_{p=0.005 n=6}
_{Check Time}	_{16.61s (± 0.40%)}	_{16.78s (± 0.31%)}	_{+0.16s (+ 0.97%)}	_16.69s	_16.85s	_{p=0.008 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{20.18s (± 0.33%)}	_{20.42s (± 0.26%)}	_{+0.24s (+ 1.16%)}	_20.32s	_20.47s	_{p=0.005 n=6}
_{xstate - node (v18.15.0, x64)}
_{Memory used}	_{512,594k (± 0.02%)}	_{512,685k (± 0.00%)}	_{+91k (+ 0.02%)}	_512,670k	_512,709k	_{p=0.031 n=6}
_{Parse Time}	_{3.27s (± 0.32%)}	_{3.27s (± 0.27%)}	_~	_3.26s	_3.28s	_{p=0.452 n=6}
_{Bind Time}	_{1.55s (± 0.26%)}	_{1.64s (± 0.00%)}	_{🔻+0.09s (+ 5.69%)}	_1.64s	_1.64s	_{p=0.002 n=6}
_{Check Time}	_{2.84s (± 0.68%)}	_{2.87s (± 0.74%)}	_{+0.03s (+ 1.17%)}	_2.85s	_2.90s	_{p=0.020 n=6}
_{Emit Time}	_{0.08s (± 0.00%)}	_{0.08s (± 0.00%)}	_~	_0.08s	_0.08s	_{p=1.000 n=6}
_{Total Time}	_{7.73s (± 0.42%)}	_{7.86s (± 0.17%)}	_{+0.13s (+ 1.73%)}	_7.85s	_7.88s	_{p=0.005 n=6}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

Angular - node (v18.15.0, x64)
Compiler-Unions - node (v18.15.0, x64)
Monaco - node (v18.15.0, x64)
TFS - node (v18.15.0, x64)
material-ui - node (v18.15.0, x64)
xstate - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

tsserver

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{Compiler-UnionsTSServer - node (v18.15.0, x64)}
_{Req 1 - updateOpen}	_{2,402ms (± 1.33%)}	_{2,360ms (± 0.98%)}	_{-42ms (- 1.73%)}	_2,334ms	_2,399ms	_{p=0.037 n=6}
_{Req 2 - geterr}	_{5,334ms (± 1.19%)}	_{5,401ms (± 1.14%)}	_{+67ms (+ 1.25%)}	_5,343ms	_5,520ms	_{p=0.045 n=6}
_{Req 3 - references}	_{330ms (± 1.32%)}	_{325ms (± 1.18%)}	_~	_320ms	_329ms	_{p=0.105 n=6}
_{Req 4 - navto}	_{277ms (± 1.19%)}	_{283ms (± 1.27%)}	_{+6ms (+ 2.29%)}	_276ms	_285ms	_{p=0.033 n=6}
_{Req 5 - completionInfo count}	_{1,356 (± 0.00%)}	_{1,356 (± 0.00%)}	_~	_1,356	_1,356	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{77ms (± 7.43%)}	_{66ms (± 8.82%)}	_{🟩-11ms (-14.22%)}	_61ms	_77ms	_{p=0.043 n=6}
_{CompilerTSServer - node (v18.15.0, x64)}
_{Req 1 - updateOpen}	_{2,477ms (± 1.28%)}	_{2,480ms (± 1.17%)}	_~	_2,432ms	_2,508ms	_{p=0.748 n=6}
_{Req 2 - geterr}	_{4,084ms (± 1.45%)}	_{4,125ms (± 1.26%)}	_{+41ms (+ 1.00%)}	_4,074ms	_4,224ms	_{p=0.045 n=6}
_{Req 3 - references}	_{342ms (± 1.22%)}	_{342ms (± 1.40%)}	_~	_332ms	_345ms	_{p=0.720 n=6}
_{Req 4 - navto}	_{284ms (± 0.36%)}	_{300ms (± 1.91%)}	_{🔻+15ms (+ 5.33%)}	_293ms	_305ms	_{p=0.005 n=6}
_{Req 5 - completionInfo count}	_{1,518 (± 0.00%)}	_{1,518 (± 0.00%)}	_~	_1,518	_1,518	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{86ms (± 6.16%)}	_{76ms (± 5.89%)}	_{🟩-10ms (-11.09%)}	_67ms	_78ms	_{p=0.037 n=6}
_{xstateTSServer - node (v18.15.0, x64)}
_{Req 1 - updateOpen}	_{2,596ms (± 0.32%)}	_{2,635ms (± 0.43%)}	_{+38ms (+ 1.47%)}	_2,613ms	_2,646ms	_{p=0.005 n=6}
_{Req 2 - geterr}	_{1,676ms (± 1.78%)}	_{1,691ms (± 2.82%)}	_~	_1,639ms	_1,762ms	_{p=0.689 n=6}
_{Req 3 - references}	_{119ms (± 8.59%)}	_{108ms (± 8.03%)}	_{🟩-11ms (- 9.28%)}	_102ms	_125ms	_{p=0.028 n=6}
_{Req 4 - navto}	_{360ms (± 0.29%)}	_{364ms (± 0.35%)}	_{+4ms (+ 1.20%)}	_362ms	_365ms	_{p=0.004 n=6}
_{Req 5 - completionInfo count}	_{2,073 (± 0.00%)}	_{2,073 (± 0.00%)}	_~	_2,073	_2,073	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{308ms (± 2.51%)}	_{296ms (± 3.15%)}	_{🟩-12ms (- 4.00%)}	_287ms	_312ms	_{p=0.037 n=6}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

CompilerTSServer - node (v18.15.0, x64)
Compiler-UnionsTSServer - node (v18.15.0, x64)
xstateTSServer - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Startup

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{tsc-startup - node (v18.15.0, x64)}
_{Execution time}	_{152.50ms (± 0.17%)}	_{152.36ms (± 0.20%)}	_{-0.15ms (- 0.10%)}	_151.14ms	_156.00ms	_{p=0.000 n=600}
_{tsserver-startup - node (v18.15.0, x64)}
_{Execution time}	_{227.61ms (± 0.14%)}	_{227.50ms (± 0.15%)}	_{-0.11ms (- 0.05%)}	_225.98ms	_231.16ms	_{p=0.000 n=600}
_{tsserverlibrary-startup - node (v18.15.0, x64)}
_{Execution time}	_{228.75ms (± 0.15%)}	_{228.80ms (± 0.17%)}	_~	_227.27ms	_231.98ms	_{p=0.619 n=600}
_{typescript-startup - node (v18.15.0, x64)}
_{Execution time}	_{228.63ms (± 0.14%)}	_{228.81ms (± 0.17%)}	_{+0.18ms (+ 0.08%)}	_227.11ms	_231.82ms	_{p=0.000 n=600}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

tsc-startup - node (v18.15.0, x64)
tsserver-startup - node (v18.15.0, x64)
tsserverlibrary-startup - node (v18.15.0, x64)
typescript-startup - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

typescript-bot · 2023-10-24T01:39:39Z

Hey @rbuckton, the results of running the DT tests are ready.
There were interesting changes:

Branch only errors:

Package: es-abstract
Error:

Error: /home/vsts/work/1/DefinitelyTyped/types/es-abstract/es6.d.ts:3:1
ERROR: 3:1  expect  TypeScript@local compile error: 
An export assignment cannot have modifiers.

/home/vsts/work/1/DefinitelyTyped/types/es-abstract/es7.d.ts:3:1
ERROR: 3:1  expect  TypeScript@local compile error: 
An export assignment cannot have modifiers.

    at testTypesVersion (/home/vsts/work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.0.184_typescript@5.3.0-dev.20231023/node_modules/@definitelytyped/dtslint/dist/index.js:171:15)
    at async runTests (/home/vsts/work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.0.184_typescript@5.3.0-dev.20231023/node_modules/@definitelytyped/dtslint/dist/index.js:130:9)

Package: es-to-primitive
Error:

Error: /home/vsts/work/1/DefinitelyTyped/types/es-to-primitive/es6.d.ts:3:1
ERROR: 3:1  expect  TypeScript@local compile error: 
An export assignment cannot have modifiers.

    at testTypesVersion (/home/vsts/work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.0.184_typescript@5.3.0-dev.20231023/node_modules/@definitelytyped/dtslint/dist/index.js:171:15)
    at async runTests (/home/vsts/work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.0.184_typescript@5.3.0-dev.20231023/node_modules/@definitelytyped/dtslint/dist/index.js:130:9)

You can check the log here.

rbuckton · 2023-10-24T15:02:24Z

(from the original issue)

I don't know if this already causes a user-facing bug, but it might in the future.

Turns out there was a bug in fixSpelling that this change made apparent as we were requesting only a syntactic override for a code-fix when we should have been requesting the effective override.

rbuckton · 2023-10-24T15:38:11Z

:( I was hoping the perf cost would be negligible, but that doesn't seem to be the case.

jakebailey · 2023-10-24T16:56:06Z

Where does the slowdown come from? A bit hard to tell at a glance. Are we calling getJSDocModifierFlagsNoCache twice somehow?

One interesting bit is that the Deprecated modifier isn't used at all to determine whether or not a symbol is deprecated or not (in fact it's entirely unreferenced); that instead is done directly in the parser through a goofy backdoor (which I once tried to remove in an effort to make JSDoc parsing fully lazy). Maybe that fact doesn't help given people may be querying modifiers for that info externally?

rbuckton · 2023-10-24T17:09:02Z

I'm guessing the issue may be all of the additional conditional branches that have to be hit in a hot code path. I'm reworking it to avoid the branches.

rbuckton · 2023-10-24T17:13:31Z

It's also being too aggressive in collecting JSDoc modifiers in a TS file.

rbuckton · 2023-10-24T17:16:06Z

@typescript-bot perf test

typescript-bot · 2023-10-24T17:16:09Z

Heya @rbuckton, I've started to run the regular perf test suite on this PR at f4253de. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-10-24T17:36:23Z

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

Here they are:

Compiler

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{Angular - node (v18.15.0, x64)}
_{Memory used}	_{295,097k (± 0.01%)}	_{295,071k (± 0.01%)}	_~	_295,013k	_295,107k	_{p=0.230 n=6}
_{Parse Time}	_{2.63s (± 0.34%)}	_{2.62s (± 0.39%)}	_~	_2.61s	_2.64s	_{p=0.273 n=6}
_{Bind Time}	_{0.84s (± 0.97%)}	_{0.83s (± 1.00%)}	_~	_0.83s	_0.85s	_{p=0.718 n=6}
_{Check Time}	_{8.03s (± 0.32%)}	_{8.04s (± 0.33%)}	_~	_8.01s	_8.07s	_{p=0.683 n=6}
_{Emit Time}	_{7.09s (± 0.25%)}	_{7.06s (± 0.32%)}	_~	_7.03s	_7.09s	_{p=0.084 n=6}
_{Total Time}	_{18.59s (± 0.15%)}	_{18.56s (± 0.21%)}	_~	_18.51s	_18.62s	_{p=0.228 n=6}
_{Compiler-Unions - node (v18.15.0, x64)}
_{Memory used}	_{193,054k (± 1.48%)}	_{193,576k (± 1.64%)}	_~	_190,670k	_196,491k	_{p=1.000 n=6}
_{Parse Time}	_{1.35s (± 0.90%)}	_{1.36s (± 1.02%)}	_~	_1.34s	_1.38s	_{p=0.176 n=6}
_{Bind Time}	_{0.73s (± 0.00%)}	_{0.73s (± 0.00%)}	_~	_0.73s	_0.73s	_{p=1.000 n=6}
_{Check Time}	_{9.16s (± 0.30%)}	_{9.15s (± 0.31%)}	_~	_9.11s	_9.20s	_{p=0.413 n=6}
_{Emit Time}	_{2.64s (± 0.29%)}	_{2.63s (± 0.44%)}	_~	_2.62s	_2.65s	_{p=0.279 n=6}
_{Total Time}	_{13.88s (± 0.21%)}	_{13.87s (± 0.22%)}	_~	_13.84s	_13.93s	_{p=0.468 n=6}
_{Monaco - node (v18.15.0, x64)}
_{Memory used}	_{347,350k (± 0.00%)}	_{347,365k (± 0.00%)}	_{+15k (+ 0.00%)}	_347,356k	_347,372k	_{p=0.031 n=6}
_{Parse Time}	_{2.46s (± 0.43%)}	_{2.46s (± 0.31%)}	_~	_2.45s	_2.47s	_{p=0.273 n=6}
_{Bind Time}	_{0.94s (± 0.00%)}	_{0.94s (± 0.00%)}	_~	_0.94s	_0.94s	_{p=1.000 n=6}
_{Check Time}	_{6.92s (± 0.41%)}	_{6.94s (± 0.50%)}	_~	_6.89s	_6.99s	_{p=0.518 n=6}
_{Emit Time}	_{4.04s (± 0.20%)}	_{4.04s (± 0.46%)}	_~	_4.01s	_4.06s	_{p=0.164 n=6}
_{Total Time}	_{14.36s (± 0.27%)}	_{14.38s (± 0.30%)}	_~	_14.31s	_14.44s	_{p=0.373 n=6}
_{TFS - node (v18.15.0, x64)}
_{Memory used}	_{302,559k (± 0.00%)}	_{302,578k (± 0.01%)}	_~	_302,551k	_302,642k	_{p=0.260 n=6}
_{Parse Time}	_{2.00s (± 1.12%)}	_{1.99s (± 2.34%)}	_~	_1.94s	_2.06s	_{p=0.808 n=6}
_{Bind Time}	_{1.01s (± 1.46%)}	_{1.01s (± 1.36%)}	_~	_0.99s	_1.03s	_{p=0.460 n=6}
_{Check Time}	_{6.26s (± 0.28%)}	_{6.26s (± 0.39%)}	_~	_6.23s	_6.29s	_{p=1.000 n=6}
_{Emit Time}	_{3.59s (± 0.85%)}	_{3.58s (± 0.58%)}	_~	_3.56s	_3.61s	_{p=0.935 n=6}
_{Total Time}	_{12.86s (± 0.22%)}	_{12.86s (± 0.37%)}	_~	_12.79s	_12.93s	_{p=0.872 n=6}
_{material-ui - node (v18.15.0, x64)}
_{Memory used}	_{470,512k (± 0.00%)}	_{470,513k (± 0.00%)}	_~	_470,478k	_470,538k	_{p=1.000 n=6}
_{Parse Time}	_{2.58s (± 0.53%)}	_{2.57s (± 0.75%)}	_~	_2.54s	_2.59s	_{p=0.681 n=6}
_{Bind Time}	_{1.00s (± 0.84%)}	_{1.00s (± 1.51%)}	_~	_0.97s	_1.01s	_{p=0.555 n=6}
_{Check Time}	_{16.64s (± 0.25%)}	_{16.64s (± 0.31%)}	_~	_16.54s	_16.68s	_{p=0.685 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{20.21s (± 0.25%)}	_{20.20s (± 0.31%)}	_~	_20.08s	_20.25s	_{p=0.809 n=6}
_{xstate - node (v18.15.0, x64)}
_{Memory used}	_{512,662k (± 0.02%)}	_{512,596k (± 0.01%)}	_~	_512,558k	_512,630k	_{p=0.298 n=6}
_{Parse Time}	_{3.26s (± 0.54%)}	_{3.27s (± 0.36%)}	_~	_3.25s	_3.28s	_{p=0.737 n=6}
_{Bind Time}	_{1.55s (± 0.49%)}	_{1.55s (± 0.26%)}	_~	_1.55s	_1.56s	_{p=0.389 n=6}
_{Check Time}	_{2.84s (± 0.99%)}	_{2.84s (± 0.62%)}	_~	_2.82s	_2.87s	_{p=1.000 n=6}
_{Emit Time}	_{0.08s (± 4.99%)}	_{0.08s (± 0.00%)}	_~	_0.08s	_0.08s	_{p=0.405 n=6}
_{Total Time}	_{7.73s (± 0.52%)}	_{7.73s (± 0.31%)}	_~	_7.71s	_7.77s	_{p=0.935 n=6}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

Angular - node (v18.15.0, x64)
Compiler-Unions - node (v18.15.0, x64)
Monaco - node (v18.15.0, x64)
TFS - node (v18.15.0, x64)
material-ui - node (v18.15.0, x64)
xstate - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

tsserver

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{Compiler-UnionsTSServer - node (v18.15.0, x64)}
_{Req 1 - updateOpen}	_{2,365ms (± 1.27%)}	_{2,377ms (± 1.27%)}	_~	_2,338ms	_2,414ms	_{p=0.688 n=6}
_{Req 2 - geterr}	_{5,410ms (± 1.54%)}	_{5,385ms (± 1.54%)}	_~	_5,299ms	_5,474ms	_{p=0.521 n=6}
_{Req 3 - references}	_{327ms (± 0.83%)}	_{327ms (± 1.62%)}	_~	_322ms	_337ms	_{p=0.515 n=6}
_{Req 4 - navto}	_{274ms (± 1.60%)}	_{278ms (± 1.32%)}	_~	_273ms	_282ms	_{p=0.090 n=6}
_{Req 5 - completionInfo count}	_{1,356 (± 0.00%)}	_{1,356 (± 0.00%)}	_~	_1,356	_1,356	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{84ms (± 6.92%)}	_{85ms (± 7.58%)}	_~	_75ms	_90ms	_{p=0.682 n=6}
_{CompilerTSServer - node (v18.15.0, x64)}
_{Req 1 - updateOpen}	_{2,481ms (± 0.88%)}	_{2,495ms (± 1.19%)}	_~	_2,464ms	_2,535ms	_{p=0.575 n=6}
_{Req 2 - geterr}	_{4,133ms (± 1.94%)}	_{4,107ms (± 2.30%)}	_~	_4,016ms	_4,201ms	_{p=0.230 n=6}
_{Req 3 - references}	_{338ms (± 1.63%)}	_{340ms (± 1.72%)}	_~	_333ms	_347ms	_{p=0.413 n=6}
_{Req 4 - navto}	_{283ms (± 0.72%)}	_{281ms (± 0.52%)}	_~	_279ms	_283ms	_{p=0.164 n=6}
_{Req 5 - completionInfo count}	_{1,518 (± 0.00%)}	_{1,518 (± 0.00%)}	_~	_1,518	_1,518	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{83ms (± 7.76%)}	_{82ms (± 7.27%)}	_~	_76ms	_89ms	_{p=0.871 n=6}
_{xstateTSServer - node (v18.15.0, x64)}
_{Req 1 - updateOpen}	_{2,591ms (± 0.70%)}	_{2,595ms (± 0.60%)}	_~	_2,566ms	_2,610ms	_{p=0.748 n=6}
_{Req 2 - geterr}	_{1,693ms (± 1.65%)}	_{1,725ms (± 1.89%)}	_~	_1,679ms	_1,760ms	_{p=0.128 n=6}
_{Req 3 - references}	_{108ms (± 5.25%)}	_{115ms (± 8.41%)}	_~	_105ms	_126ms	_{p=0.080 n=6}
_{Req 4 - navto}	_{359ms (± 0.11%)}	_{366ms (± 0.22%)}	_{+7ms (+ 1.90%)}	_365ms	_367ms	_{p=0.003 n=6}
_{Req 5 - completionInfo count}	_{2,073 (± 0.00%)}	_{2,073 (± 0.00%)}	_~	_2,073	_2,073	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{308ms (± 2.65%)}	_{313ms (± 1.86%)}	_~	_305ms	_320ms	_{p=0.258 n=6}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

CompilerTSServer - node (v18.15.0, x64)
Compiler-UnionsTSServer - node (v18.15.0, x64)
xstateTSServer - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Startup

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{tsc-startup - node (v18.15.0, x64)}
_{Execution time}	_{152.64ms (± 0.16%)}	_{152.55ms (± 0.18%)}	_{-0.09ms (- 0.06%)}	_151.35ms	_155.36ms	_{p=0.000 n=600}
_{tsserver-startup - node (v18.15.0, x64)}
_{Execution time}	_{227.97ms (± 0.16%)}	_{227.84ms (± 0.19%)}	_{-0.13ms (- 0.06%)}	_226.50ms	_238.63ms	_{p=0.000 n=600}
_{tsserverlibrary-startup - node (v18.15.0, x64)}
_{Execution time}	_{229.11ms (± 0.17%)}	_{229.13ms (± 0.19%)}	_~	_227.32ms	_235.96ms	_{p=0.992 n=600}
_{typescript-startup - node (v18.15.0, x64)}
_{Execution time}	_{228.43ms (± 0.17%)}	_{228.36ms (± 0.17%)}	_~	_226.86ms	_235.08ms	_{p=0.054 n=600}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

tsc-startup - node (v18.15.0, x64)
tsserver-startup - node (v18.15.0, x64)
tsserverlibrary-startup - node (v18.15.0, x64)
typescript-startup - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

rbuckton · 2023-10-24T17:36:50Z

much better

src/compiler/types.ts

jakebailey · 2023-10-24T20:32:34Z

This seems good to me, though is it intended that DT still fail? It seems odd that we'd hard error on a JSDoc comment in TS code like this.

rbuckton · 2023-10-25T17:37:35Z

@typescript-bot run dt

typescript-bot · 2023-10-25T17:37:38Z

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

Update: The results are in!

rbuckton · 2023-10-25T17:38:13Z

This seems good to me, though is it intended that DT still fail? It seems odd that we'd hard error on a JSDoc comment in TS code like this.

That failure may be related to the change in the first approach, I need to retest to see if it still occurs in the new approach.

typescript-bot · 2023-10-25T18:29:34Z

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

jakebailey

LGTM, I don't think this is breaking given nobody should be depending on the exact values anyway.

I'm half worried that this PR appears to entirely fill up the entirety ModifierFlags's bitwise space, but I'm not sure how we could do this different other than to have another flag prop or something.

rbuckton · 2023-10-25T22:22:00Z

LGTM, I don't think this is breaking given nobody should be depending on the exact values anyway.

I'm half worried that this PR appears to entirely fill up the entirety ModifierFlags's bitwise space, but I'm not sure how we could do this different other than to have another flag prop or something.

There are still quite a few bits remaining, and we don't add new modifiers all that often. I'm trying to conserve memory as much as possible here, so I'd rather wait to expand into a second property when it becomes absolutely necessary.

rbuckton requested review from sandersn and jakebailey October 24, 2023 00:35

typescript-bot assigned rbuckton Oct 24, 2023

typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Oct 24, 2023

Fix incorrect flags in modifierFlagsCache

428d327

rbuckton force-pushed the fix-42189 branch from a6c203a to 428d327 Compare October 24, 2023 00:46

rbuckton marked this pull request as draft October 24, 2023 01:10

Use effective 'override' modifier in fixSpelling codefix

6a48508

rbuckton marked this pull request as ready for review October 24, 2023 14:59

Give JSDoc-provided modifiers their own entries in ModifierFlags

f4253de

jakebailey reviewed Oct 24, 2023

View reviewed changes

src/compiler/types.ts Outdated Show resolved Hide resolved

Correct function name in comments

4774cec

jakebailey approved these changes Oct 25, 2023

View reviewed changes

rbuckton merged commit 79da510 into main Oct 25, 2023
19 checks passed

rbuckton deleted the fix-42189 branch October 25, 2023 22:24

rbuckton restored the fix-42189 branch October 25, 2023 22:27

rbuckton deleted the fix-42189 branch October 25, 2023 22:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix incorrect flags in modifierFlagsCache #56198

Fix incorrect flags in modifierFlagsCache #56198

rbuckton commented Oct 24, 2023 •

edited

Loading

typescript-bot commented Oct 24, 2023

rbuckton commented Oct 24, 2023

typescript-bot commented Oct 24, 2023 •

edited

Loading

rbuckton commented Oct 24, 2023

typescript-bot commented Oct 24, 2023 •

edited

Loading

typescript-bot commented Oct 24, 2023 •

edited

Loading

typescript-bot commented Oct 24, 2023

typescript-bot commented Oct 24, 2023

Compiler

tsserver

Startup

typescript-bot commented Oct 24, 2023

rbuckton commented Oct 24, 2023

rbuckton commented Oct 24, 2023

jakebailey commented Oct 24, 2023

rbuckton commented Oct 24, 2023

rbuckton commented Oct 24, 2023

rbuckton commented Oct 24, 2023

typescript-bot commented Oct 24, 2023 •

edited

Loading

typescript-bot commented Oct 24, 2023

Compiler

tsserver

Startup

rbuckton commented Oct 24, 2023

jakebailey commented Oct 24, 2023

rbuckton commented Oct 25, 2023

typescript-bot commented Oct 25, 2023 •

edited

Loading

rbuckton commented Oct 25, 2023

typescript-bot commented Oct 25, 2023

jakebailey left a comment

rbuckton commented Oct 25, 2023

Fix incorrect flags in modifierFlagsCache #56198

Fix incorrect flags in modifierFlagsCache #56198

Conversation

rbuckton commented Oct 24, 2023 • edited Loading

typescript-bot commented Oct 24, 2023

rbuckton commented Oct 24, 2023

typescript-bot commented Oct 24, 2023 • edited Loading

rbuckton commented Oct 24, 2023

typescript-bot commented Oct 24, 2023 • edited Loading

typescript-bot commented Oct 24, 2023 • edited Loading

typescript-bot commented Oct 24, 2023

typescript-bot commented Oct 24, 2023

Compiler

tsserver

Startup

typescript-bot commented Oct 24, 2023

rbuckton commented Oct 24, 2023

rbuckton commented Oct 24, 2023

jakebailey commented Oct 24, 2023

rbuckton commented Oct 24, 2023

rbuckton commented Oct 24, 2023

rbuckton commented Oct 24, 2023

typescript-bot commented Oct 24, 2023 • edited Loading

typescript-bot commented Oct 24, 2023

Compiler

tsserver

Startup

rbuckton commented Oct 24, 2023

jakebailey commented Oct 24, 2023

rbuckton commented Oct 25, 2023

typescript-bot commented Oct 25, 2023 • edited Loading

rbuckton commented Oct 25, 2023

typescript-bot commented Oct 25, 2023

jakebailey left a comment

Choose a reason for hiding this comment

rbuckton commented Oct 25, 2023

rbuckton commented Oct 24, 2023 •

edited

Loading

typescript-bot commented Oct 24, 2023 •

edited

Loading

typescript-bot commented Oct 24, 2023 •

edited

Loading

typescript-bot commented Oct 24, 2023 •

edited

Loading

typescript-bot commented Oct 24, 2023 •

edited

Loading

typescript-bot commented Oct 25, 2023 •

edited

Loading