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

Perf regression in object destructuring with default values referencing other destructured members #48902

Closed
damienmortini opened this issue Apr 20, 2022 · 6 comments · Fixed by #48941
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros

Comments

@damienmortini
Copy link

VS Code Version:

  • Version: 1.66.2 (user setup)
  • Version: 1.67.0-insider (user setup)

OS:

  • Windows_NT x64 10.0.22000

Steps to Reproduce:

  1. Open this code
class GLTexture {
  constructor({
    gl,
    width,
    height,
    target = gl.TEXTURE_2D,
    internalFormat = gl.RGBA8 || gl.RGBA,
    format = gl.RGBA,
    type = gl.UNSIGNED_BYTE,
    autoGenerateMipmap = false,
    minFilter = autoGenerateMipmap ? gl.LINEAR_MIPMAP_LINEAR : gl.LINEAR,
    magFilter = gl.LINEAR,
    wrapS = gl.REPEAT,
    wrapT = gl.REPEAT,
  }) {
  }
}
  1. See that after switching tabs or hovering on a property that IntelliSense isn't working and Loading IntelliSense Status in status bar starts spinning forever

Code_cb8oof7A1o
Code_k3e8RGnq39

I tried to isolate the issue that seems to be related to operators in constructor, but not always. Sometimes, it appears by commenting/uncommenting other lines.

@damienmortini
Copy link
Author

damienmortini commented Apr 20, 2022

Small Update -

I found a better isolation of the issue, it seems there is a hierarchy loop issue, the more properties depending on value1 you add, the slower it'll get until eventually it never loads.
If a default value is assigned to value1 everything works as expected.

class MyClass {
  constructor({
    value1,
    test1 = value1.test1,
    test2 = value1.test2,
    test3 = value1.test3,
    test4 = value1.test4,
    test5 = value1.test5,
    test6 = value1.test6,
    test7 = value1.test7,
    test8 = value1.test8,
    // ...
  }) {}
}

@mjbvz mjbvz transferred this issue from microsoft/vscode Apr 30, 2022
@mjbvz mjbvz removed their assignment Apr 30, 2022
@mjbvz
Copy link
Contributor

mjbvz commented Apr 30, 2022

Thanks for the self-contained example! Was able to confirm using 4.7.0-dev.20220430

@andrewbranch andrewbranch added Bug A bug in TypeScript Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output labels May 2, 2022
@typescript-bot typescript-bot added the Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros label May 2, 2022
@typescript-bot
Copy link
Collaborator

typescript-bot commented May 2, 2022

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of this repro running against the nightly TypeScript.


Comment by @damienmortini

❌ Failed: -

  • 'value1' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.
  • Binding element 'value1' implicitly has an 'any' type.

⚠️ Way slower than historical runs

Historical Information
Version Reproduction Outputs Time
4.6.2

❌ Failed: -

  • 'value1' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.
  • Binding element 'value1' implicitly has an 'any' type.

⚠️ Way slower
4.2.2, 4.3.2, 4.4.2, 4.5.2

❌ Failed: -

  • 'value1' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.
  • Binding element 'value1' implicitly has an 'any' type.

@typescript-bot
Copy link
Collaborator

The change between 4.5.2 and 4.6.2 occurred at 831b770.

@andrewbranch andrewbranch removed the Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output label May 2, 2022
@andrewbranch andrewbranch added this to the TypeScript 4.7.1 milestone May 2, 2022
@andrewbranch
Copy link
Member

Repro executes in ~1 second in 4.5 and earlier, ~120 seconds in 4.6 and later 😅

Caused by #46266

@ahejlsberg
Copy link
Member

Doesn't appear to have anything to do with class constructors. This suffers from the same issue:

function foo({
    value1,
    test1 = value1.test1,
    test2 = value1.test2,
    test3 = value1.test3,
    test4 = value1.test4,
    test5 = value1.test5,
    test6 = value1.test6,
    test7 = value1.test7,
    test8 = value1.test8,
    // ...
}) {}

Something appears to have exponentially rising cost. I'll research...

@andrewbranch andrewbranch changed the title This class constructor code breaks IntelliSense making it load infinitely Perf regression in object destructuring with default values referencing other destructured members May 3, 2022
@typescript-bot typescript-bot added Fix Available A PR has been opened for this issue labels May 3, 2022
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 Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants