Skip to content

Go 5 levels deep when determining if an object type is deeply nested #56140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Go 5 levels deep when determining if an object type is deeply nested #56140

wants to merge 1 commit into from

Conversation

RyanCavanaugh
Copy link
Member

Fixes #56138, but at what cost?

@RyanCavanaugh
Copy link
Member Author

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 17, 2023 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 17, 2023 •
edited
Loading

Heya @RyanCavanaugh, I've started to run the diff-based top-repos suite (tsserver) on this PR at df4b02a. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 17, 2023 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 17, 2023 •
edited
Loading

Heya @RyanCavanaugh, I've started to run the diff-based user code test suite (tsserver) on this PR at df4b02a. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 17, 2023 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 17, 2023 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 17, 2023 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 17, 2023 •
edited
Loading

Hey @RyanCavanaugh, 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/158253/artifacts?artifactName=tgz&fileId=C8C08B693257409FD141BAAF882B7D634C35321E20E1445EA8D07EB47DCBD1FC02&fileName=/typescript-5.3.0-insiders.20231017.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
Copy link
Collaborator

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

Everything looks good!

@typescript-bot
Copy link
Collaborator

@RyanCavanaugh Here are the results of running the user test suite comparing main and refs/pull/56140/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
Copy link
Collaborator

@RyanCavanaugh
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,151k (± 0.02%) 295,089k (± 0.01%) -62k (- 0.02%) 295,035k 295,118k p=0.031 n=6
Parse Time 2.64s (± 0.56%) 2.63s (± 0.91%) ~ 2.59s 2.65s p=0.368 n=6
Bind Time 0.83s (± 0.98%) 0.84s (± 1.17%) ~ 0.83s 0.85s p=0.336 n=6
Check Time 8.06s (± 0.35%) 8.00s (± 0.32%) -0.06s (- 0.76%) 7.97s 8.04s p=0.010 n=6
Emit Time 7.04s (± 0.35%) 7.05s (± 0.29%) ~ 7.03s 7.09s p=0.371 n=6
Total Time 18.58s (± 0.13%) 18.53s (± 0.28%) ~ 18.49s 18.63s p=0.064 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 192,608k (± 1.58%) 191,617k (± 1.28%) ~ 190,578k 196,631k p=0.093 n=6
Parse Time 1.34s (± 0.56%) 1.34s (± 1.54%) ~ 1.32s 1.37s p=0.869 n=6
Bind Time 0.73s (± 0.00%) 0.73s (± 0.00%) ~ 0.73s 0.73s p=1.000 n=6
Check Time 9.11s (± 0.52%) 9.10s (± 0.18%) ~ 9.07s 9.12s p=0.744 n=6
Emit Time 2.63s (± 0.57%) 2.64s (± 0.48%) ~ 2.62s 2.65s p=0.157 n=6
Total Time 13.81s (± 0.43%) 13.81s (± 0.24%) ~ 13.76s 13.85s p=0.573 n=6
Monaco - node (v18.15.0, x64)
Memory used 347,287k (± 0.00%) 347,304k (± 0.00%) +18k (+ 0.01%) 347,288k 347,322k p=0.045 n=6
Parse Time 2.45s (± 0.31%) 2.45s (± 0.42%) ~ 2.44s 2.47s p=0.931 n=6
Bind Time 0.94s (± 0.00%) 0.94s (± 0.00%) ~ 0.94s 0.94s p=1.000 n=6
Check Time 6.95s (± 0.49%) 6.93s (± 0.32%) ~ 6.89s 6.95s p=0.326 n=6
Emit Time 4.01s (± 0.34%) 4.01s (± 0.13%) ~ 4.01s 4.02s p=0.796 n=6
Total Time 14.35s (± 0.33%) 14.34s (± 0.23%) ~ 14.28s 14.38s p=0.517 n=6
TFS - node (v18.15.0, x64)
Memory used 302,525k (± 0.00%) 302,543k (± 0.00%) +18k (+ 0.01%) 302,516k 302,558k p=0.037 n=6
Parse Time 2.00s (± 0.77%) 2.00s (± 0.58%) ~ 1.99s 2.02s p=0.666 n=6
Bind Time 1.01s (± 0.97%) 1.00s (± 0.83%) ~ 1.00s 1.02s p=0.588 n=6
Check Time 6.25s (± 0.38%) 6.26s (± 0.49%) ~ 6.24s 6.32s p=0.566 n=6
Emit Time 3.57s (± 0.29%) 3.56s (± 0.34%) ~ 3.55s 3.58s p=0.675 n=6
Total Time 12.83s (± 0.19%) 12.83s (± 0.23%) ~ 12.79s 12.87s p=0.747 n=6
material-ui - node (v18.15.0, x64)
Memory used 470,497k (± 0.00%) 470,499k (± 0.01%) ~ 470,461k 470,552k p=0.936 n=6
Parse Time 2.57s (± 0.59%) 2.59s (± 0.76%) ~ 2.57s 2.62s p=0.080 n=6
Bind Time 1.00s (± 0.52%) 0.99s (± 1.18%) ~ 0.98s 1.01s p=0.351 n=6
Check Time 16.58s (± 0.39%) 16.58s (± 0.34%) ~ 16.48s 16.63s p=0.936 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 20.15s (± 0.32%) 20.16s (± 0.32%) ~ 20.05s 20.21s p=0.688 n=6
xstate - node (v18.15.0, x64)
Memory used 512,622k (± 0.01%) 517,221k (± 0.01%) +4,600k (+ 0.90%) 517,169k 517,333k p=0.005 n=6
Parse Time 3.26s (± 0.30%) 3.26s (± 0.60%) ~ 3.23s 3.28s p=0.557 n=6
Bind Time 1.55s (± 0.26%) 1.55s (± 0.26%) ~ 1.54s 1.55s p=1.000 n=6
Check Time 2.86s (± 1.40%) 3.12s (± 0.44%) 🔻+0.26s (+ 8.97%) 3.10s 3.14s p=0.005 n=6
Emit Time 0.08s (± 4.99%) 0.08s (± 0.00%) ~ 0.08s 0.08s p=0.405 n=6
Total Time 7.75s (± 0.47%) 8.01s (± 0.18%) +0.26s (+ 3.38%) 7.98s 8.02s 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 6
Baseline baseline 6

tsserver

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-UnionsTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,387ms (± 1.25%) 2,344ms (± 1.27%) ~ 2,319ms 2,401ms p=0.066 n=6
Req 2 - geterr 5,344ms (± 1.45%) 5,397ms (± 1.57%) ~ 5,302ms 5,482ms p=0.261 n=6
Req 3 - references 327ms (± 0.53%) 329ms (± 1.03%) ~ 325ms 333ms p=0.628 n=6
Req 4 - navto 278ms (± 1.18%) 275ms (± 1.41%) ~ 271ms 279ms p=0.137 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 79ms (± 7.83%) 84ms (± 8.71%) ~ 76ms 91ms p=0.124 n=6
CompilerTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,492ms (± 0.68%) 2,495ms (± 1.18%) ~ 2,454ms 2,539ms p=0.689 n=6
Req 2 - geterr 4,081ms (± 1.34%) 4,109ms (± 1.79%) ~ 4,031ms 4,183ms p=0.873 n=6
Req 3 - references 341ms (± 1.36%) 337ms (± 1.32%) ~ 332ms 342ms p=0.107 n=6
Req 4 - navto 285ms (± 0.29%) 284ms (± 0.43%) ~ 282ms 285ms p=0.227 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 (± 5.21%) 80ms (± 8.75%) ~ 72ms 89ms p=0.565 n=6
xstateTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,598ms (± 0.33%) 2,589ms (± 0.57%) ~ 2,567ms 2,612ms p=0.261 n=6
Req 2 - geterr 1,705ms (± 2.76%) 2,111ms (± 0.66%) 🔻+406ms (+23.80%) 2,092ms 2,130ms p=0.005 n=6
Req 3 - references 111ms (± 7.44%) 112ms (± 7.05%) ~ 105ms 125ms p=0.934 n=6
Req 4 - navto 359ms (± 0.11%) 362ms (± 0.49%) +3ms (+ 0.88%) 360ms 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 (± 1.60%) 313ms (± 2.20%) ~ 305ms 321ms p=0.198 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 6
Baseline baseline 6

Startup

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
tsc-startup - node (v18.15.0, x64)
Execution time 152.21ms (± 0.18%) 152.02ms (± 0.16%) -0.19ms (- 0.12%) 150.89ms 155.80ms p=0.000 n=600
tsserver-startup - node (v18.15.0, x64)
Execution time 227.38ms (± 0.16%) 227.13ms (± 0.20%) -0.26ms (- 0.11%) 225.84ms 239.46ms p=0.000 n=600
tsserverlibrary-startup - node (v18.15.0, x64)
Execution time 228.48ms (± 0.17%) 228.29ms (± 0.18%) -0.19ms (- 0.08%) 226.72ms 234.63ms p=0.000 n=600
typescript-startup - node (v18.15.0, x64)
Execution time 228.03ms (± 0.15%) 227.96ms (± 0.17%) -0.08ms (- 0.03%) 226.61ms 232.04ms p=0.014 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 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

Hey @RyanCavanaugh, it looks like the DT test run failed. Please check the log for more details.
You can check the log here.

@jakebailey
Copy link
Member

FWIW DT failed because it ran before we could merge the big refactor there (not sure if that was a factor in you dropping this)

@RyanCavanaugh
Copy link
Member Author

xstate perf 🫠

@jakebailey
Copy link
Member

oh eek

well, that's xstate vOld so maybe it'd be better later (new perf when, @jakebailey)

@typescript-bot
Copy link
Collaborator

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

Everything looks good!

1 similar comment
@typescript-bot
Copy link
Collaborator

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

Everything looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TypeScript ignoring checks for deeply nested computed object types
3 participants
@RyanCavanaugh @typescript-bot @jakebailey