Improve diagnostics deduplication by gabritto · Pull Request #58220 · microsoft/TypeScript

Gabriela Araujo Britto (gabritto) · 2024-04-17T00:09:47Z

Before, we only considered two diagnostics the same if everything about them was equal, including the message chain and related information. With this PR, we now consider diagnostics at the same location and with the same head message as being the same, regardless of the rest of the message chain and related information.
That is necessary because, as the new test shows, we can generate the "same" diagnostic at different moments in type checking, and they will have different elaboration (different message chains and related information).

This PR also makes it so deduplication keeps the diagnostic with more elaboration between two diagnostics considered to be equal.

Note that this doesn't completely solve the issue, because not all diagnostics that should be considered equal have the same code and head message.
Edit: follow-up PR #58318 fixes this.

Gabriela Araujo Britto (gabritto) · 2024-04-17T00:09:59Z

TypeScript Bot (@typescript-bot) test it

TypeScript Bot (typescript-bot) · 2024-04-17T00:10:03Z

Starting jobs; this comment will be updated as builds start and complete.

Command	Status	Results
`test top400`	✅ Started	✅ Results
`user test this`	✅ Started	✅ Results
`run dt`	✅ Started
`perf test this faster`	✅ Started	👀 Results

TypeScript Bot (typescript-bot) · 2024-04-17T00:37:55Z

Gabriela Araujo Britto (@gabritto) Here are the results of running the user tests comparing main and refs/pull/58220/merge:

Everything looks good!

TypeScript Bot (typescript-bot) · 2024-04-17T00:44:53Z

Gabriela Araujo Britto (@gabritto)
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{Angular - node (v18.15.0, x64)}
_{Memory used}	_{297,009k (± 0.01%)}	_{297,007k (± 0.00%)}	_~	_296,984k	_297,022k	_{p=0.873 n=6}
_{Parse Time}	_{2.71s (± 0.54%)}	_{2.71s (± 0.40%)}	_~	_2.69s	_2.72s	_{p=0.805 n=6}
_{Bind Time}	_{0.82s (± 0.00%)}	_{0.82s (± 0.00%)}	_~	_0.82s	_0.82s	_{p=1.000 n=6}
_{Check Time}	_{8.35s (± 0.36%)}	_{8.32s (± 0.39%)}	_~	_8.26s	_8.35s	_{p=0.683 n=6}
_{Emit Time}	_{7.06s (± 0.39%)}	_{7.07s (± 0.39%)}	_~	_7.05s	_7.12s	_{p=0.871 n=6}
_{Total Time}	_{18.93s (± 0.11%)}	_{18.93s (± 0.28%)}	_~	_18.84s	_19.00s	_{p=1.000 n=6}
_{Compiler-Unions - node (v18.15.0, x64)}
_{Memory used}	_{193,019k (± 0.98%)}	_{192,420k (± 0.79%)}	_~	_191,766k	_195,511k	_{p=1.000 n=6}
_{Parse Time}	_{1.63s (± 1.53%)}	_{1.66s (± 1.13%)}	_~	_1.62s	_1.67s	_{p=0.121 n=6}
_{Bind Time}	_{0.87s (± 1.40%)}	_{0.87s (± 0.94%)}	_~	_0.86s	_0.88s	_{p=1.000 n=6}
_{Check Time}	_{11.32s (± 0.37%)}	_{11.31s (± 0.20%)}	_~	_11.29s	_11.35s	_{p=0.572 n=6}
_{Emit Time}	_{3.15s (± 0.37%)}	_{3.13s (± 0.73%)}	_~	_3.10s	_3.16s	_{p=0.164 n=6}
_{Total Time}	_{16.97s (± 0.33%)}	_{16.96s (± 0.28%)}	_~	_16.88s	_17.01s	_{p=1.000 n=6}
_{mui-docs - node (v18.15.0, x64)}
_{Memory used}	_{1,749,594k (± 0.00%)}	_{1,749,611k (± 0.00%)}	_~	_1,749,549k	_1,749,680k	_{p=0.521 n=6}
_{Parse Time}	_{9.99s (± 0.64%)}	_{9.97s (± 0.45%)}	_~	_9.91s	_10.04s	_{p=0.688 n=6}
_{Bind Time}	_{3.35s (± 0.61%)}	_{3.35s (± 0.64%)}	_~	_3.33s	_3.39s	_{p=0.805 n=6}
_{Check Time}	_{82.00s (± 0.26%)}	_{81.92s (± 0.41%)}	_~	_81.26s	_82.23s	_{p=0.936 n=6}
_{Emit Time}	_{0.20s (± 2.06%)}	_{0.20s (± 2.02%)}	_~	_0.20s	_0.21s	_{p=0.218 n=6}
_{Total Time}	_{95.54s (± 0.24%)}	_{95.43s (± 0.36%)}	_~	_94.77s	_95.68s	_{p=1.000 n=6}
_{self-build-src - node (v18.15.0, x64)}
_{Memory used}	_{2,307,395k (± 0.01%)}	_{2,307,221k (± 0.04%)}	_~	_2,305,962k	_2,308,279k	_{p=0.810 n=6}
_{Parse Time}	_{7.42s (± 0.56%)}	_{7.43s (± 0.45%)}	_~	_7.39s	_7.49s	_{p=1.000 n=6}
_{Bind Time}	_{2.74s (± 0.36%)}	_{2.73s (± 0.69%)}	_~	_2.71s	_2.76s	_{p=0.332 n=6}
_{Check Time}	_{49.58s (± 0.93%)}	_{49.41s (± 0.63%)}	_~	_49.05s	_49.81s	_{p=0.471 n=6}
_{Emit Time}	_{3.96s (± 1.34%)}	_{4.08s (± 2.73%)}	_{+0.13s (+ 3.16%)}	_3.95s	_4.24s	_{p=0.045 n=6}
_{Total Time}	_{63.69s (± 0.62%)}	_{63.67s (± 0.39%)}	_~	_63.27s	_63.99s	_{p=0.810 n=6}
_{self-build-src-public-api - node (v18.15.0, x64)}
_{Memory used}	_{2,382,448k (± 0.01%)}	_{2,382,950k (± 0.03%)}	_~	_2,381,858k	_2,383,619k	_{p=0.128 n=6}
_{Parse Time}	_{7.65s (± 0.84%)}	_{7.66s (± 0.59%)}	_~	_7.61s	_7.72s	_{p=0.936 n=6}
_{Bind Time}	_{2.51s (± 0.91%)}	_{2.51s (± 0.83%)}	_~	_2.49s	_2.54s	_{p=0.809 n=6}
_{Check Time}	_{49.73s (± 0.72%)}	_{49.65s (± 0.65%)}	_~	_49.27s	_50.15s	_{p=0.748 n=6}
_{Emit Time}	_{3.89s (± 2.49%)}	_{3.91s (± 2.78%)}	_~	_3.81s	_4.11s	_{p=0.936 n=6}
_{Total Time}	_{63.79s (± 0.60%)}	_{63.76s (± 0.58%)}	_~	_63.24s	_64.28s	_{p=0.936 n=6}
_{self-compiler - node (v18.15.0, x64)}
_{Memory used}	_{419,464k (± 0.01%)}	_{419,484k (± 0.01%)}	_~	_419,436k	_419,535k	_{p=0.471 n=6}
_{Parse Time}	_{4.23s (± 0.25%)}	_{4.21s (± 0.46%)}	_~	_4.18s	_4.23s	_{p=0.250 n=6}
_{Bind Time}	_{1.60s (± 1.93%)}	_{1.60s (± 1.23%)}	_~	_1.58s	_1.63s	_{p=0.809 n=6}
_{Check Time}	_{22.34s (± 0.38%)}	_{22.34s (± 0.23%)}	_~	_22.26s	_22.40s	_{p=0.809 n=6}
_{Emit Time}	_{1.73s (± 1.03%)}	_{1.69s (± 2.93%)}	_~	_1.64s	_1.77s	_{p=0.107 n=6}
_{Total Time}	_{29.89s (± 0.33%)}	_{29.84s (± 0.20%)}	_~	_29.77s	_29.94s	_{p=0.336 n=6}
_{ts-pre-modules - node (v18.15.0, x64)}
_{Memory used}	_{368,996k (± 0.02%)}	_{369,004k (± 0.02%)}	_~	_368,931k	_369,168k	_{p=1.000 n=6}
_{Parse Time}	_{3.66s (± 0.90%)}	_{3.66s (± 0.36%)}	_~	_3.64s	_3.68s	_{p=0.870 n=6}
_{Bind Time}	_{1.90s (± 1.58%)}	_{1.91s (± 1.61%)}	_~	_1.87s	_1.94s	_{p=0.677 n=6}
_{Check Time}	_{19.38s (± 0.26%)}	_{19.37s (± 0.31%)}	_~	_19.28s	_19.43s	_{p=1.000 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{24.94s (± 0.37%)}	_{24.95s (± 0.38%)}	_~	_24.78s	_25.04s	_{p=0.936 n=6}
_{vscode - node (v18.15.0, x64)}
_{Memory used}	_{2,914,727k (± 0.00%)}	_{2,914,826k (± 0.00%)}	_~	_2,914,737k	_2,914,879k	_{p=0.173 n=6}
_{Parse Time}	_{13.44s (± 0.40%)}	_{13.43s (± 0.37%)}	_~	_13.34s	_13.48s	_{p=0.629 n=6}
_{Bind Time}	_{4.13s (± 2.57%)}	_{4.10s (± 2.06%)}	_~	_4.05s	_4.27s	_{p=0.618 n=6}
_{Check Time}	_{72.23s (± 0.52%)}	_{72.59s (± 0.32%)}	_~	_72.30s	_72.91s	_{p=0.093 n=6}
_{Emit Time}	_{20.79s (±10.20%)}	_{20.15s (± 7.82%)}	_~	_19.42s	_23.36s	_{p=1.000 n=6}
_{Total Time}	_{110.58s (± 1.81%)}	_{110.26s (± 1.32%)}	_~	_109.48s	_113.19s	_{p=0.688 n=6}
_{webpack - node (v18.15.0, x64)}
_{Memory used}	_{409,388k (± 0.01%)}	_{409,384k (± 0.01%)}	_~	_409,312k	_409,441k	_{p=0.936 n=6}
_{Parse Time}	_{3.27s (± 0.94%)}	_{3.24s (± 0.58%)}	_~	_3.21s	_3.26s	_{p=0.145 n=6}
_{Bind Time}	_{1.38s (± 0.59%)}	_{1.38s (± 0.54%)}	_~	_1.37s	_1.39s	_{p=0.729 n=6}
_{Check Time}	_{14.39s (± 0.16%)}	_{14.38s (± 0.20%)}	_~	_14.34s	_14.41s	_{p=0.622 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{19.04s (± 0.18%)}	_{19.00s (± 0.20%)}	_~	_18.96s	_19.05s	_{p=0.170 n=6}
_{xstate-main - node (v18.15.0, x64)}
_{Memory used}	_{458,720k (± 0.01%)}	_{458,689k (± 0.01%)}	_~	_458,603k	_458,786k	_{p=0.471 n=6}
_{Parse Time}	_{3.22s (± 0.68%)}	_{3.22s (± 0.39%)}	_~	_3.21s	_3.24s	_{p=1.000 n=6}
_{Bind Time}	_{1.17s (± 0.88%)}	_{1.17s (± 0.64%)}	_~	_1.16s	_1.18s	_{p=0.931 n=6}
_{Check Time}	_{18.08s (± 0.42%)}	_{18.11s (± 0.25%)}	_~	_18.03s	_18.16s	_{p=0.375 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{22.48s (± 0.35%)}	_{22.50s (± 0.24%)}	_~	_22.40s	_22.55s	_{p=0.336 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)
mui-docs - node (v18.15.0, x64)
self-build-src - node (v18.15.0, x64)
self-build-src-public-api - node (v18.15.0, x64)
self-compiler - node (v18.15.0, x64)
ts-pre-modules - node (v18.15.0, x64)
vscode - node (v18.15.0, x64)
webpack - node (v18.15.0, x64)
xstate-main - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

TypeScript Bot (typescript-bot) · 2024-04-17T01:53:48Z

Gabriela Araujo Britto (@gabritto) Here are the results of running the top 400 repos comparing main and refs/pull/58220/merge:

Everything looks good!

Jake Bailey (jakebailey) · 2024-04-17T18:02:36Z

TypeScript Bot (@typescript-bot) run dt

TypeScript Bot (typescript-bot) · 2024-04-17T18:02:39Z

Starting jobs; this comment will be updated as builds start and complete.

Command	Status	Results
`run dt`	✅ Started	✅ Results

TypeScript Bot (typescript-bot) · 2024-04-17T18:32:30Z

Hey Jake Bailey (@jakebailey), the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

Wesley Wigham (weswigham) · 2024-04-17T19:17:36Z


 export function sortAndDeduplicateDiagnostics<T extends Diagnostic>(diagnostics: readonly T[]): SortedReadonlyArray<T> {
- return sortAndDeduplicate<T>(diagnostics, compareDiagnostics);
+ return sortAndDeduplicate<T>(diagnostics, compareDiagnostics, diagnosticsEqualityComparer);


As far as I can tell, DiagnosticCollection's .add method is still using compareDiagnosticsSkipRelatedInformation, so it's just first-in-wins (with the same root message) - should that also get updated to prefer the "lesser" message like this?

compareDiagnosticsSkipRelatedInformation is still the same in terms of what it considers equal - two diagnostics with the same location and head message but different elaboration will still be considered different by that function, and so both will be added via DiagnosticCollection.add. Only later, when we call sortAndDeduplicateDiagnostics, will we use diagnosticsEqualityComparer and get rid of all but one of the diagnostics that we now consider equal.

Now that I think about it, if we want to keep using functions like sortAndDeduplicate and insertSorted, I think we need to have two separate functions for comparison and equality: one that fully compares diagnostics for purposes of sorting (and sorting a diagnostic with more elaboration before one with less), and another for purposes of deduplication that is more permissive and only compares location and head message for equality. Otherwise we'd have to implement a function that somehow says "yes, those two diagnostics are equal, but one of them is preferred".

…sertion

Gabriela Araujo Britto (gabritto) added 2 commits April 16, 2024 23:09

add test for duplicate diagnostic
49a65b1

improve diagnostics deduplication
165869c

TypeScript Bot (typescript-bot) assigned Gabriela Araujo Britto (gabritto)Apr 17, 2024

TypeScript Bot (typescript-bot) added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 17, 2024

Wesley Wigham (weswigham) reviewed Apr 17, 2024
View reviewed changes

Gabriela Araujo Britto (gabritto) added 3 commits April 19, 2024 05:53

fix comparison; add test; keep deduped and sorted diagnostics upon in…
3257850
…sertion

more comparison fixes, more tests
35ed28c

rename functions
ef2eb5c

Gabriela Araujo Britto (gabritto) mentioned this pull request Apr 25, 2024
Improve diagnostics deduplication 2 #58318
Merged

Gabriela Araujo Britto (gabritto) requested review from Andrew Branch (andrewbranch) and Wesley Wigham (weswigham)April 26, 2024 19:42

Andrew Branch (andrewbranch) approved these changes Apr 26, 2024
View reviewed changes

Wesley Wigham (weswigham) approved these changes Apr 26, 2024
View reviewed changes

Gabriela Araujo Britto (gabritto) merged commit ebcb09d into mainApr 26, 2024

Gabriela Araujo Britto (gabritto) deleted the gabritto/issue58207 branch April 26, 2024 22:44

Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025

Navigation Menu

Uh oh!

Improve diagnostics deduplication - #58220

Improve diagnostics deduplication#58220
Gabriela Araujo Britto (gabritto) merged 5 commits into
mainfrom
gabritto/issue58207

Gabriela Araujo Britto (gabritto) commented Apr 17, 2024•
edited
Loading

Uh oh!

Gabriela Araujo Britto (gabritto) commented Apr 17, 2024

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024•
edited
Loading

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024

tsc

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024

Uh oh!

Jake Bailey (jakebailey) commented Apr 17, 2024

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024•
edited
Loading

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024

Uh oh!

Wesley Wigham (weswigham)Apr 17, 2024•
edited
Loading

Uh oh!

Gabriela Araujo Britto (gabritto)Apr 17, 2024

Uh oh!

Gabriela Araujo Britto (gabritto)Apr 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

Gabriela Araujo Britto (gabritto) commented Apr 17, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gabriela Araujo Britto (gabritto) commented Apr 17, 2024

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024

tsc

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024

Uh oh!

Jake Bailey (jakebailey) commented Apr 17, 2024

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 17, 2024

Uh oh!

Wesley Wigham (weswigham)Apr 17, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gabriela Araujo Britto (gabritto)Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

Gabriela Araujo Britto (gabritto)Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Gabriela Araujo Britto (gabritto) commented Apr 17, 2024•
edited
Loading

TypeScript Bot (typescript-bot) commented Apr 17, 2024•
edited
Loading

TypeScript Bot (typescript-bot) commented Apr 17, 2024•
edited
Loading

Wesley Wigham (weswigham)Apr 17, 2024•
edited
Loading