Skip to content

Repo: Our EXPERIMENTAL_useProjectService unit tests are much slower in CI #7348

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
JoshuaKGoldberg opened this issue Jul 28, 2023 · 4 comments · Fixed by #8136
Closed

Repo: Our EXPERIMENTAL_useProjectService unit tests are much slower in CI #7348

JoshuaKGoldberg opened this issue Jul 28, 2023 · 4 comments · Fixed by #8136
Labels
accepting prs Go ahead, send a pull request that resolves this issue help wanted Extra attention is needed package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin package: parser Issues related to @typescript-eslint/parser performance Issues regarding performance

Comments

@JoshuaKGoldberg
Copy link
Member

Suggestion

Taking 18ea3b1's https://github.com/typescript-eslint/typescript-eslint/actions/runs/5588135268/ as an example:

The Node version is 18 with the flag and 16 or 20 without, but that's just a quirk of the CI file. I've found it to be slower locally as well. This should be investigated.

@bradzacher and I noticed this when we first merged. cc @jakebailey - if you have the time, we'd appreciate any tips you have on it. We haven't investigated deeply yet.
@JoshuaKGoldberg JoshuaKGoldberg added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin package: parser Issues related to @typescript-eslint/parser performance Issues regarding performance labels Jul 28, 2023
@bradzacher bradzacher added help wanted Extra attention is needed accepting prs Go ahead, send a pull request that resolves this issue labels Jul 28, 2023
@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Jul 28, 2023
Open
2 tasks
@jakebailey
Copy link
Collaborator

Does each test fully spin up a new ProjectService? It's not a cheap operation; starting tsserver takes a bit to get going so if it's fresh each time I suspect that it won't be cheap.

But, if it's all in one process, I should be able to profile it pretty easily too.

@JoshuaKGoldberg
Copy link
Member Author

each test fully spin up a new ProjectService

Yeah, I believe so. We've previously tried to keep tests pretty isolated from each other. Would it be reasonable to look into speeding up tsserver creation?

@rubiesonthesky
Copy link
Contributor

Jest is not using isolated modules nor reset modules, so I think it's reusing a lot things. But because we implemented the worker idle memory limit, jest is creating new worker when going over the memory limit. This means that they probably are creating new ProjectService too. We could pump the memory limit to 500 MB instead of 300 MB and check does that help. I'm not sure what the max memory in that CI runner is. Like, you could also try 700 MB but then it may get to point, where the whole test setup crashes.

@bradzacher
Copy link
Member

typescript-eslint/packages/rule-tester/src/RuleTester.ts

Lines 100 to 109 in 67f93b1

constructor.afterAll(() => {
try {
// instead of creating a hard dependency, just use a soft require
// a bit weird, but if they're using this tooling, it'll be installed
const parser = require(TYPESCRIPT_ESLINT_PARSER) as typeof ParserType;
parser.clearCaches();
} catch {
// ignored on purpose
}
});

We are clearing the project service after each test block finishes.

@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Oct 20, 2023
Closed
3 tasks
This was referenced Dec 24, 2023
Draft
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
accepting prs Go ahead, send a pull request that resolves this issue help wanted Extra attention is needed package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin package: parser Issues related to @typescript-eslint/parser performance Issues regarding performance
Projects
None yet
Milestone
No milestone
4 participants
@rubiesonthesky @JoshuaKGoldberg @jakebailey @bradzacher