Skip to content

Performance degradation in 8.1.0: @typescript-eslint/no-unsafe-return with high-cardinality unions #10196

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

Closed
4 tasks done
AaronMoat opened this issue Oct 22, 2024 · 7 comments · Fixed by #10203
Closed
4 tasks done
Labels
accepting prs Go ahead, send a pull request that resolves this issue locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. performance Issues regarding performance

Comments

@AaronMoat
Copy link

AaronMoat commented Oct 22, 2024

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Issue Description

I haven't exactly narrowed down why yet, but there appears to be a performance degradation in patterns like this:

type Alpha2Code =
  | "AF"
  ...
  | "XK";

type LanguageCode =
  | "aa"
  ...
  | "zu";

type Locale = `${LanguageCode}-${Alpha2Code}`;

export function getDefaultLocale() {
  return 'en-AU' as Locale;
}

In the example reproduction, it takes about 73 seconds on my machine.

Note that tsc takes 0.7s.

Reproduction Repository Link

https://github.com/AaronMoat/typescript-eslint-perf

Repro Steps

  1. clone the repo
  2. yarn install
  3. yarn lint

Versions

package version
typescript-eslint 8.11.0
TypeScript 5.6.3
ESLint 9.13.0
node v20.17.0
@AaronMoat AaronMoat added bug Something isn't working triage Waiting for team members to take a look labels Oct 22, 2024
@bradzacher
Copy link
Member

A few things you need to do:

First - you've filed an issue saying there's been a perf regression... But you've provided no point of comparison. You say it's gotten slower... But by how much? Is it slower with the exact same config? Or did your config change? You need to provide more information!

Next - you've provided a reproduction with all of the recommended rules turned on. That's a massive blast radius for performance and so any meaningful comparisons will be very difficult. You need to slim down the config so that it includes the minimum config to reproduce the issue.

Help us to help you by providing a minimal repro with all the information.

@bradzacher
Copy link
Member

On the surface I would say that this is working as expected. Type information is computed lazily - meaning it is computed only when it's accessed by the tool.

TypeScript checks completely different things to what we check (intentionally so!) meaning it requests different type info to what we do. It could just be that we're getting or iterating through more types than TS is.

We also can't access the same internals as TS - hence we cannot optimise in the same ways they can.

But as mentioned - without more information it's hard to tell much of anything.

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party performance Issues regarding performance and removed bug Something isn't working triage Waiting for team members to take a look labels Oct 22, 2024
@AaronMoat
Copy link
Author

Apologies @bradzacher, I held too much context in my head and didn't write it down!

It's near-instant (~1s) in previous versions. I extracted this out of a larger internal repository which was in the process of upgrading, so I didn't have a benchmarks for this minimal reproduction, but I've grabbed that for you below.

Apologies for not bisecting the rules - I have done so now, and found that one rule, @typescript-eslint/no-unsafe-return, is mostly the culprit. Turning it off but keeping the recommended rules, the repo I linked finishes in ~1s.

I've pushed to the reproduction repository to just run @typescript-eslint/no-unsafe-return. It takes around 56s on my machine. I have pushed a v7 branch, which downgrades to [email protected] and [email protected] (and switches back to project: "./tsconfig.json"), which completes with 1.2s on my machine. This is around a 46x increase in time spent for this rule between the two majors.

Is there anything else I can do to help narrow down the problem?

@AaronMoat AaronMoat changed the title Performance degradation in Typescript ESLint 8 with high-cardinality unions Performance degradation in @typescript-eslint/no-unsafe-return in major version 8 with high-cardinality unions Oct 22, 2024
@AaronMoat
Copy link
Author

Ah interesting; digging further, the issue is not reproducible in 8.0.1 but is reproducible in 8.1.0, so it's not the v8 upgrade.

@AaronMoat AaronMoat changed the title Performance degradation in @typescript-eslint/no-unsafe-return in major version 8 with high-cardinality unions Performance degradation in 8.1.0: @typescript-eslint/no-unsafe-return with high-cardinality unions Oct 22, 2024
@AaronMoat
Copy link
Author

Looks like #8693; most of the time is spent in tsutils.isThenableType. I don't have nearly enough context to understand if there's feasible fixes, though.

@tezf
Copy link
Contributor

tezf commented Oct 24, 2024

I spent some time looking at this with @AaronMoat and think we were able to narrow down the cause. Issue isn't currently taking PRs so I'll just link to the commit.

main...tezf:typescript-eslint:fix-accidental-n-2-iterations

@bradzacher
Copy link
Member

that fix certainly looks reasonable.
feel free to raise a PR so we can see how the CI goes on it.

@bradzacher bradzacher added accepting prs Go ahead, send a pull request that resolves this issue and removed awaiting response Issues waiting for a reply from the OP or another party labels Oct 24, 2024
@github-actions github-actions bot added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Nov 1, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. performance Issues regarding performance
Projects
None yet
3 participants