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

fix(perf): add cache for fs calls when using language service (#908) #940

Closed

Conversation

IOuser
Copy link
Contributor

@IOuser IOuser commented Jan 8, 2019

Recently, I encountered the slowdown of tests with disabled jest cache.
As @Farbdose noted in #908 , the reason for the slowdown is intensive work with the file system.
So I tried to implement a simple cache for fs calls in TS language service.

And here's results for my codebase:
Before adding fs cache: ~280s (3 runs avg.)
After adding fs cache: ~91s (3 runs avg.)

Hope it helps.

@coveralls
Copy link

coveralls commented Jan 8, 2019

Pull Request Test Coverage Report for Build 2443

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 91.974%

Totals Coverage Status
Change from base Build 2442: 0.04%
Covered Lines: 920
Relevant Lines: 965

💛 - Coveralls

@IOuser
Copy link
Contributor Author

IOuser commented Jan 9, 2019

@kulshekhar, would you please provide your feedback? Thanks

@Farbdose
Copy link

Farbdose commented Jan 9, 2019

@IOuser I wanted to test your fix with my demo repository but for some reason I can't get ts-jest to build. It always fails with src/cli/index.ts:14:33 - error TS2314: Generic type 'Arguments' requires 1 type argument(s). but the CI server doesn't have any problems with it... The tests fail too if I try it, looks like I'm using the wrong version of something fundamental.

Can't I build ts-jest with:

node@8.11.2
npm@5.6.0
typescript@3.1.6

?

Do I need to do something specific after cloning the repository (except npm install && npm run build)?

@IOuser
Copy link
Contributor Author

IOuser commented Jan 9, 2019

@Farbdose, I ran into a similar problem, npm just updates all dependencies as they are not fixed in package.json.
Try to remove node_modules, rollback package-lock.json (if it was updated by npm install) and use npm ci instead npm install.

@Farbdose
Copy link

Farbdose commented Jan 9, 2019

@IOuser many thanks. That looks much cleaner than my own fix:

find . -type f -print0 | xargs -0 sed -i "s/\(argv\|args\|_\).\sArguments\([^<a-zA-Z]\)/\1: Arguments<any>\2/g"

@IOuser
Copy link
Contributor Author

IOuser commented Jan 9, 2019

@Farbdose, did you checkout to add-simple-cache-for-fs-calls branch in ts-jest submodule? I see that npm run compileTsJest produced dist without changes from this PR

@Farbdose
Copy link

Farbdose commented Jan 9, 2019

@IOuser Just realized that too, I was only searching for the word memoize and it was in dist do I didn't look further

Regarding the submodule checkout ... I ... did ... and I may have accidentially added a -b to the checkout command... fixed

Now we are getting somewhere: I have a clear speedup from 45s to 36s in the gitlab CI and 33s on my local SSD.

Branch: add-simple-cache-for-fs-calls
Commit: c3bddefb9725f6ca6164998722f6d9b20abeb3c4
CI Run: JobID:143574563

Thats more or less what I got too with my early hacks in the compiler.js but thats still way to long for an empty test suite. Even if I have some heavy modules.

@Farbdose
Copy link

Farbdose commented Jan 9, 2019

@IOuser new cpu-profile and ts-jest log

I think we are still lossing some time on parsing the contents of the cached package.json files over and over.

@IOuser
Copy link
Contributor Author

IOuser commented Jan 9, 2019

@Farbdose, now I see. In 23.10.0-beta.2 language service was enabled by default (for type checking, const enums, etc..). That's why compile time dramatically increased.
There is option to use old behavior.

@IOuser IOuser changed the title fix(perf): add cache for fs calls (#908) fix(perf): add cache for fs calls when using language service (#908) Jan 9, 2019
@GeeWee
Copy link
Collaborator

GeeWee commented Mar 1, 2019

I'd love to merge this in. We already have lodash.memoize as a dependency, can we use that instead of implementing our own function?

@4kochi
Copy link
Contributor

4kochi commented Jul 6, 2019

Hi there, thanks for your great work maintaining this repo. I just want to know what the state of this PR is.
I recently update to the latest version of jest-preset-angular v7, coming from v6. I noticed that the performance is slower then before for non cached tests.

@kulshekhar
Copy link
Owner

addressed in #1049

@kulshekhar kulshekhar closed this Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants