Skip to content
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.

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

Performance issue on updating Babel monorepo typescript from 4.7.4 to 4.8.2 #50515

Closed
JLHwung opened this issue Aug 29, 2022 · 6 comments · Fixed by #50535
Closed

Performance issue on updating Babel monorepo typescript from 4.7.4 to 4.8.2 #50515

JLHwung opened this issue Aug 29, 2022 · 6 comments · Fixed by #50535
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@JLHwung
Copy link

JLHwung commented Aug 29, 2022

Bug Report

🔎 Search Terms

out of memory
Symbols
Types
4.8 regression

🕗 Version & Regression Information

  • This is a crash
  • This changed between versions 4.8.0-dev.20220527 and 4.8.0-dev.20220528

💻 Code

// We can quickly address your report if:
//  - The code sample is short. Nearly all TypeScript bugs can be demonstrated in 20-30 lines of code!
//  - It doesn't use external libraries. These are often issues with the type definitions rather than TypeScript bugs.
//  - The incorrectness of the behavior is readily apparent from reading the sample.
// Reports are slower to investigate if:
//  - We have to pare too much extraneous code.
//  - We have to clone a large repo and validate that the problem isn't elsewhere.
//  - The sample is confusing or doesn't clearly demonstrate what's wrong.

❗ It is a large repo. But we have narrowed down the suspicious code paths. See the "Context" section.

Checkout https://github.com/JLHwung/babel/tree/ts-4-8-regression

Run yarn && yarn tsc --extendedDiagnostics --incremental false

🙁 Actual behavior

4.8.0-dev.20220528:

Files:                          714
Lines of Library:             27991
Lines of Definitions:         48613
Lines of TypeScript:         109741
Lines of JavaScript:              0
Lines of JSON:                 4198
Lines of Other:                   0
Nodes of Library:            119423
Nodes of Definitions:        127849
Nodes of TypeScript:         416263
Nodes of JavaScript:              0
Nodes of JSON:                11813
Nodes of Other:                   0
Identifiers:                 230685
Symbols:                    3072081
Types:                      1627331
Instantiations:            42109836
Memory used:               4019580K
Assignability cache size:    266387
Identity cache size:          16941
Subtype cache size:           99137
Strict subtype cache size:     9238
I/O Read time:                0.03s
Parse time:                   0.49s
ResolveModule time:           0.06s
ResolveTypeReference time:    0.00s
Program time:                 0.63s
Bind time:                    0.30s
Check time:                  67.33s
transformTime time:           4.21s
Source Map time:              0.12s
commentTime time:             0.14s
I/O Write time:               0.10s
printTime time:               5.26s
Emit time:                    5.26s
Total time:                  73.53s

tsc 4.7.4

Files:                          714
Lines of Library:             27958
Lines of Definitions:         48613
Lines of TypeScript:         109737
Lines of JavaScript:              0
Lines of JSON:                 4198
Lines of Other:                   0
Nodes of Library:            119371
Nodes of Definitions:        127849
Nodes of TypeScript:         416262
Nodes of JavaScript:              0
Nodes of JSON:                11813
Nodes of Other:                   0
Identifiers:                 230675
Symbols:                     959380
Types:                       380554
Instantiations:            42135598
Memory used:               1102821K
Assignability cache size:    262472
Identity cache size:          16941
Subtype cache size:          100724
Strict subtype cache size:     8524
I/O Read time:                0.02s
Parse time:                   0.49s
ResolveModule time:           0.05s
ResolveTypeReference time:    0.00s
Program time:                 0.62s
Bind time:                    0.30s
Check time:                  15.77s
transformTime time:           0.97s
Source Map time:              0.04s
commentTime time:             0.06s
I/O Write time:               0.09s
printTime time:               1.50s
Emit time:                    1.50s
Total time:                  18.18s

tsc 4.8 increases memory usage from 1GB to 4GB, thus performance is worsen from 18s to 73s. The increased memory usage fails our CI by OOM error. The "Symbols" and "Types" significantly increased.

🙂 Expected behavior

Memory usage should not be quadrupled.

👓 Context

The issue surfaced when we upgrade from 4.7.4 to 4.8.2: babel/babel#14880
Later @liuxingbaoyu managed to narrow down the root cause:

class NodePath<T extends t.Node = t.Node> {
  getScope(scope: Scope): Scope {
    return this.isScope() ? new Scope(this) : scope;
  }
}

https://github.com/babel/babel/blob/6c5ebd12fa3a3e594f5dc8dab2b4f44e153f5976/packages/babel-traverse/src/path/index.ts#L111-L113

where Scope accepts NodePath<t.Pattern | t.Scopeable>. t.Pattern | t.Scopeable is subset of t.Node, which is a union of 252 AST types. (https://github.com/babel/babel/blob/6c5ebd12fa3a3e594f5dc8dab2b4f44e153f5976/packages/babel-types/src/ast-types/generated/index.ts#L50)

The isScope is defined as

interface VirtualTypeNodePathValidators
  isScope<T extends t.Node>(
    this: NodePath<T>,
    opts?: object,
  ): this is NodePath<T & VirtualTypeAliases["Scope"]>;
}

https://github.com/babel/babel/blob/6c5ebd12fa3a3e594f5dc8dab2b4f44e153f5976/packages/babel-traverse/src/path/lib/virtual-types-validator.ts#L72

where VirtualTypeAliases["Scope"] resolves to t.Pattern | t.Scopeable.

It seems to me tsc spends quite a bit of effort to check new Scope(this) is valid behind the this.isScope check. We can workaround the issue by casting this to NodePath<t.Pattern | t.Scopeable>:

Class NodePath<T extends t.Node = t.Node> {
  getScope(scope: Scope): Scope {
+++    return this.isScope() ? new Scope(this as NodePath<t.Pattern | t.Scopeable>) : scope;
---    return this.isScope() ? new Scope(this) : scope;
  }
}

Yet I expect tsc infers that NodePath<T & VirtualTypeAliases["Scope"]> must subset NodePath<t.Pattern | t.Scopeable> and thus new Scope is valid without that much efforts.

I did a rough bisect and the issue was first introduced in 4.8.0-dev.20220528. The regression might be introduced in #49119.

@JLHwung
Copy link
Author

JLHwung commented Aug 29, 2022

Here is a smaller reproduction repo thanks to @liuxingbaoyu, though the behaviour is somewhat different, in this reproduction tsc 4.8 hangs while tsc 4.7 "works fine": https://github.com/liuxingbaoyu/babel-ts4.8-bug

@ehoogeveen-medweb
Copy link

I'm not an expert, but this sounds to me like what #50329 intends to fix.

@JLHwung
Copy link
Author

JLHwung commented Aug 29, 2022

@ehoogeveen-medweb Thanks for the head-up. I tested with "typescript": "npm:@typescript-deploys/pr-build@4.9.0-pr-50329-13", the memory usage is still around 4GB. I believe this issue is not yet covered by #50329

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Aug 29, 2022
@ehoogeveen-medweb
Copy link

@types/babel__traverse faced what looks like the same issue in #50290. They worked around it with a change to the type information in DefinitelyTyped/DefinitelyTyped#62001 based on the suggestion in #50290 (comment).

@ahejlsberg
Copy link
Member

The issue here appears to be the intersection normalization code that was added in #49119. In this particular repro we end up constructing some very expensive unions of intersections due to the strongly typed parent property in NodePath, and normalizing these types ends up taking forever. Since we really only care about normalizing intersections with {}, I have added a check for that in #50535.

@ahejlsberg ahejlsberg added Bug A bug in TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels Aug 30, 2022
@ahejlsberg ahejlsberg added this to the TypeScript 4.9.0 milestone Aug 30, 2022
@ahejlsberg ahejlsberg added the Fix Available A PR has been opened for this issue label Aug 30, 2022
@liuxingbaoyu
Copy link

@ehoogeveen-medweb @ahejlsberg Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants