Skip to content

Narrow down non-const enity name expressions within element access expressions - #56392

Closed
Mateusz Burzyński (Andarist) wants to merge 3 commits into
microsoft:mainfrom
Andarist:narrow-down-non-const-entity-name-expressions-in-element-accesses
Closed

Narrow down non-const enity name expressions within element access expressions#56392
Mateusz Burzyński (Andarist) wants to merge 3 commits into
microsoft:mainfrom
Andarist:narrow-down-non-const-entity-name-expressions-in-element-accesses

Conversation

@Andarist

Copy link
Copy Markdown
Contributor

an experiment to fix#51368 . I wonder what the perf tests show for this (cc Jake Bailey (@jakebailey) ) 😉

@typescript-botTypeScript Bot (typescript-bot) added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Nov 14, 2023
@jakebailey

Copy link
Copy Markdown
Member

Fails the self check so I'm going to assume that it'll break in testing; can run once you've had a chance to look.

@fatcerberus

Copy link
Copy Markdown

To be clear, this isn't a general fix for #56389, correct?

@Andarist

Copy link
Copy Markdown
ContributorAuthor

Yeah, it isn't. I specifically only looked into what DetachHead mentioned here

@sandersnNathan Shively-Sanders (sandersn) added the Experiment A fork with an experimental idea which might not make it into master label Nov 29, 2023
@Andarist
Mateusz Burzyński (Andarist) marked this pull request as draft December 26, 2023 17:48
@Andarist
Mateusz Burzyński (Andarist) marked this pull request as ready for review December 28, 2023 22:59
@Andarist

Copy link
Copy Markdown
ContributorAuthor

Jake Bailey (@jakebailey) I fixed the self-check here. Would you mind running the perf suite? :)

Comment on lines +2838 to +2840
if (rootDeclaration.kind !== SyntaxKind.VariableDeclaration) {
return true;
}

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: recheck correctness here

@jakebailey

Copy link
Copy Markdown
Member

@typescript-bot

TypeScript Bot (typescript-bot) commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the regular perf test suite on this PR at 5d49f98. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the diff-based top-repos suite on this PR at 5d49f98. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the diff-based user code test suite on this PR at 5d49f98. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the tarball bundle task on this PR at 5d49f98. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the parallelized Definitely Typed test suite on this PR at 5d49f98. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

Hey Jake Bailey (@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/159261/artifacts?artifactName=tgz&fileId=A10C51230D65ED8DD9ABAA7E144F16A014B03A2C082E67DD7993C0F756E4E4C402&fileName=/typescript-5.4.0-insiders.20240104.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/pr-build@5.4.0-pr-56392-10".;

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey) Here are the results of running the user test suite comparing main and refs/pull/56392/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Package install failed"

Otherwise...

Something interesting changed - please have a look.

Details

puppeteer

packages/browsers/test/src/tsconfig.json

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey)
The results of the perf run you requested are in!

Here they are:

Compiler

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Angular - node (v18.15.0, x64)
Memory used295,439k (± 0.01%)295,546k (± 0.01%)+107k (+ 0.04%)295,498k295,572kp=0.008 n=6
Parse Time2.65s (± 0.21%)2.65s (± 0.19%)~2.64s2.65sp=0.640 n=6
Bind Time0.82s (± 0.00%)0.82s (± 0.50%)~0.82s0.83sp=0.405 n=6
Check Time8.14s (± 0.08%)8.16s (± 0.41%)~8.11s8.21sp=0.097 n=6
Emit Time7.10s (± 0.43%)7.10s (± 0.23%)~7.08s7.12sp=0.745 n=6
Total Time18.71s (± 0.16%)18.73s (± 0.23%)~18.66s18.78sp=0.296 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used193,464k (± 1.56%)192,547k (± 1.22%)~191,548k197,344kp=0.378 n=6
Parse Time1.35s (± 1.62%)1.35s (± 1.96%)~1.30s1.38sp=0.732 n=6
Bind Time0.72s (± 0.00%)0.72s (± 0.57%)~0.72s0.73sp=0.405 n=6
Check Time9.27s (± 0.33%)9.26s (± 0.46%)~9.19s9.30sp=0.870 n=6
Emit Time2.61s (± 0.29%)2.62s (± 0.62%)~2.60s2.64sp=0.454 n=6
Total Time13.95s (± 0.23%)13.94s (± 0.36%)~13.85s13.99sp=0.747 n=6
Monaco - node (v18.15.0, x64)
Memory used347,413k (± 0.00%)347,471k (± 0.01%)+58k (+ 0.02%)347,432k347,505kp=0.005 n=6
Parse Time2.46s (± 0.36%)2.46s (± 0.79%)~2.43s2.49sp=0.801 n=6
Bind Time0.93s (± 0.56%)0.93s (± 0.56%)~0.92s0.93sp=1.000 n=6
Check Time6.88s (± 0.33%)6.96s (± 0.36%)+0.08s (+ 1.21%)6.93s6.99sp=0.005 n=6
Emit Time4.07s (± 0.50%)4.05s (± 0.63%)~4.02s4.08sp=0.222 n=6
Total Time14.33s (± 0.28%)14.40s (± 0.38%)+0.07s (+ 0.49%)14.34s14.49sp=0.043 n=6
TFS - node (v18.15.0, x64)
Memory used302,726k (± 0.00%)302,868k (± 0.01%)+142k (+ 0.05%)302,852k302,895kp=0.005 n=6
Parse Time1.99s (± 0.45%)1.99s (± 0.82%)~1.98s2.02sp=0.803 n=6
Bind Time1.01s (± 1.22%)1.00s (± 1.22%)~0.99s1.02sp=1.000 n=6
Check Time6.29s (± 0.31%)6.39s (± 0.39%)+0.11s (+ 1.67%)6.36s6.42sp=0.005 n=6
Emit Time3.58s (± 0.56%)3.57s (± 0.39%)~3.56s3.59sp=0.744 n=6
Total Time12.87s (± 0.28%)12.97s (± 0.29%)+0.10s (+ 0.80%)12.91s13.02sp=0.006 n=6
material-ui - node (v18.15.0, x64)
Memory used506,808k (± 0.01%)506,828k (± 0.01%)~506,795k506,868kp=0.230 n=6
Parse Time2.58s (± 0.53%)2.58s (± 0.47%)~2.57s2.60sp=0.410 n=6
Bind Time0.99s (± 0.52%)0.99s (± 0.82%)~0.98s1.00sp=0.140 n=6
Check Time16.91s (± 0.37%)16.96s (± 0.14%)~16.93s16.99sp=0.090 n=6
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)~0.00s0.00sp=1.000 n=6
Total Time20.48s (± 0.32%)20.54s (± 0.13%)~20.50s20.58sp=0.078 n=6
xstate - node (v18.15.0, x64)
Memory used512,914k (± 0.01%)512,850k (± 0.01%)~512,773k512,906kp=0.066 n=6
Parse Time3.28s (± 0.31%)3.27s (± 0.27%)~3.26s3.28sp=0.273 n=6
Bind Time1.54s (± 0.36%)1.54s (± 0.49%)~1.53s1.55sp=0.137 n=6
Check Time2.83s (± 0.73%)2.81s (± 0.49%)~2.79s2.82sp=0.052 n=6
Emit Time0.07s (± 0.00%)0.07s (± 0.00%)~0.07s0.07sp=1.000 n=6
Total Time7.71s (± 0.35%)7.68s (± 0.27%)~7.66s7.71sp=0.064 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)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

tsserver

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Compiler-UnionsTSServer - node (v18.15.0, x64)
Req 1 - updateOpen2,344ms (± 0.83%)2,353ms (± 0.63%)~2,334ms2,371msp=0.296 n=6
Req 2 - geterr5,405ms (± 1.15%)5,466ms (± 1.34%)+61ms (+ 1.13%)5,400ms5,572msp=0.045 n=6
Req 3 - references324ms (± 0.42%)324ms (± 0.74%)~321ms328msp=0.806 n=6
Req 4 - navto276ms (± 1.16%)274ms (± 0.95%)~270ms277msp=0.287 n=6
Req 5 - completionInfo count1,356 (± 0.00%)1,356 (± 0.00%)~1,3561,356p=1.000 n=6
Req 5 - completionInfo89ms (± 6.71%)92ms (± 4.76%)~84ms95msp=0.413 n=6
CompilerTSServer - node (v18.15.0, x64)
Req 1 - updateOpen2,470ms (± 0.92%)2,490ms (± 1.33%)~2,455ms2,532msp=0.230 n=6
Req 2 - geterr4,180ms (± 1.98%)4,097ms (± 1.67%)~4,056ms4,235msp=0.149 n=6
Req 3 - references336ms (± 1.34%)339ms (± 1.28%)~333ms343msp=0.420 n=6
Req 4 - navto285ms (± 1.27%)286ms (± 1.02%)~284ms292msp=0.216 n=6
Req 5 - completionInfo count1,518 (± 0.00%)1,518 (± 0.00%)~1,5181,518p=1.000 n=6
Req 5 - completionInfo83ms (± 6.55%)90ms (± 1.37%)🔻+6ms (+ 7.62%)87ms90msp=0.026 n=6
xstateTSServer - node (v18.15.0, x64)
Req 1 - updateOpen2,600ms (± 0.57%)2,613ms (± 0.35%)~2,601ms2,621msp=0.172 n=6
Req 2 - geterr1,729ms (± 2.74%)1,741ms (± 1.57%)~1,703ms1,769msp=0.873 n=6
Req 3 - references116ms (± 8.61%)112ms (±10.30%)~100ms123msp=0.462 n=6
Req 4 - navto365ms (± 0.27%)368ms (± 1.42%)~364ms378msp=0.203 n=6
Req 5 - completionInfo count2,073 (± 0.00%)2,073 (± 0.00%)~2,0732,073p=1.000 n=6
Req 5 - completionInfo309ms (± 1.70%)310ms (± 0.80%)~307ms313msp=0.685 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)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Startup

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
tsc-startup - node (v18.15.0, x64)
Execution time153.63ms (± 0.20%)153.57ms (± 0.20%)~152.55ms157.42msp=0.087 n=600
tsserver-startup - node (v18.15.0, x64)
Execution time229.14ms (± 0.15%)228.88ms (± 0.16%)-0.26ms (- 0.11%)227.60ms234.31msp=0.000 n=600
tsserverlibrary-startup - node (v18.15.0, x64)
Execution time230.28ms (± 0.20%)230.30ms (± 0.19%)~228.60ms233.66msp=0.784 n=600
typescript-startup - node (v18.15.0, x64)
Execution time230.60ms (± 0.21%)230.72ms (± 0.20%)+0.12ms (+ 0.05%)229.04ms237.79msp=0.003 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)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Developer Information:

Download Benchmarks

@typescript-bot

Copy link
Copy Markdown
Contributor

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

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey) Here are the results of running the top-repos suite comparing main and refs/pull/56392/merge:

Everything looks good!

@Andarist

Copy link
Copy Markdown
ContributorAuthor

superseded by #57847

@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ExperimentA fork with an experimental idea which might not make it into masterFor Uncommitted BugPR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

narrowing object by property doesn't work if key isn't a const

5 participants

@Andarist@jakebailey@fatcerberus@typescript-bot@sandersn