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

Memory leak in jest-circus #7274

Closed
dylang opened this issue Oct 25, 2018 · 15 comments
Closed

Memory leak in jest-circus #7274

dylang opened this issue Oct 25, 2018 · 15 comments

Comments

@dylang
Copy link
Contributor

dylang commented Oct 25, 2018

🐛 Bug Report

When using jest-circus, an obvious memory leak and gradual slowdown occurs.

$ node --expose-gc ./node_modules/.bin/jest --logHeapUsage --runInBand funky
 PASS  tests/funky-36.test.js (20 MB heap size)
 PASS  tests/funky-86.test.js (22 MB heap size)
 PASS  tests/funky-28.test.js (24 MB heap size)
...
 PASS  tests/funky-56.test.js (195 MB heap size)
 PASS  tests/funky-24.test.js (197 MB heap size)
 PASS  tests/funky-30.test.js (199 MB heap size)
 PASS  tests/funky-52.test.js (201 MB heap size)

To Reproduce

100 identical tests with jest-circus/runner.

describe('funky-10.test', () => {
    test('funky-10', async () => {
        expect(1).toEqual(1);
    });
});

jest.config:

testRunner: require.resolve('jest-circus/runner')

Expected behavior

Same 100 tests without jest-circus/runner.

$ node --expose-gc ./node_modules/.bin/jest --logHeapUsage --runInBand funky
 PASS  tests/funky-36.test.js (17 MB heap size)
 PASS  tests/funky-40.test.js (17 MB heap size)
 PASS  tests/funky-28.test.js (17 MB heap size)
...
 PASS  tests/funky-18.test.js (17 MB heap size)
 PASS  tests/funky-87.test.js (17 MB heap size)
 PASS  tests/funky-89.test.js (17 MB heap size)

Link to repl or repo (highly encouraged)

https://github.com/dylang/jest-circus-memory-leak-repo

Run npx envinfo --preset jest

 System:
    OS: macOS Sierra 10.12.6
    CPU: x64 Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
  Binaries:
    Node: 10.8.0 - ~/.nvm/versions/node/v10.8.0/bin/node
    Yarn: 1.10.1 - /usr/local/bin/yarn
    npm: 6.4.1 - ~/.nvm/versions/node/v10.8.0/bin/npm
  npmPackages:
    jest: ^23.6.0 => 23.6.0
@rickhanlonii
Copy link
Member

👀

@SimenB
Copy link
Member

SimenB commented Oct 25, 2018

#6965?

@dylang
Copy link
Contributor Author

dylang commented Oct 26, 2018

I've determined this is caused by graceful-fs.

isaacs/node-graceful-fs#102

I have a fix locally and will create a PR to graceful-fs in the morning.

@dylang
Copy link
Contributor Author

dylang commented Oct 26, 2018

Here's a PR for graceful-fs:
isaacs/node-graceful-fs#137

This PR fixes the issue in my repro repo at
https://github.com/dylang/jest-circus-memory-leak-repo

HOWEVER, in my work repo that I can't share here, the graceful-fs changes weren't enough for jest-circus not to leak memory so I'm disabling it for now. I've spent more time than I intended already but I can look deeper if there are specific things I should try.

@thymikee
Copy link
Collaborator

That's awesome. I just feel this is never gonna be merged, based on the state of the repository. Also @palmerj3 seems to have some issues there as well: https://mobile.twitter.com/palmerj3/status/1055782284983496704

Maybe there's a maintained for of the project?

@SimenB
Copy link
Member

SimenB commented Nov 2, 2018

@dylang can you test with eager evaluating error stacks, ref #6965?

@dylang
Copy link
Contributor Author

dylang commented Nov 6, 2018

@thymikee - my fixes for the memory leak were merged and published!

@SimenB I upgraded graceful-fs in dylang/jest-circus-memory-leak and it no longer have the memory leak issue, so I think we can consider this issue resolved, as long as graceful-fs is ^4.1.15 everywhere it's used in this project.

@SimenB
Copy link
Member

SimenB commented Nov 6, 2018

HOWEVER, in my work repo that I can't share here, the graceful-fs changes weren't enough for jest-circus not to leak memory so I'm disabling it for now. I've spent more time than I intended already but I can look deeper if there are specific things I should try.

I'd still like to track that part. Or is it fixed? If not, could you test the stack change I linked to?

@dylang
Copy link
Contributor Author

dylang commented Nov 7, 2018

Very non-scientific. We have 5800 tests and I don't have time right now to do a full run several times over, so this is just the first 15 tests - which are not necessarily the same 15 tests. Filenames have been removed because I don't know if I can share them.

Jest 23.6.0, default runner

node --expose-gc ./node_modules/.bin/jest --logHeapUsage --runInBand --config=./jest.config.js

 PASS  -filename removed- (35.116s, 165 MB heap size)
 PASS  -filename removed- (193 MB heap size)
 PASS  -filename removed- (222 MB heap size)
 PASS  -filename removed- (251 MB heap size)
 PASS  -filename removed- (251 MB heap size)
 PASS  -filename removed- (66.108s, 192 MB heap size)
 PASS  -filename removed- (221 MB heap size)
 PASS  -filename removed- (10.591s, 214 MB heap size)
 PASS  -filename removed- (246 MB heap size)
 PASS  -filename removed- (258 MB heap size)
 PASS  -filename removed- (272 MB heap size)
 PASS  -filename removed- (275 MB heap size)
 PASS  -filename removed- (276 MB heap size)
 PASS  -filename removed- (246 MB heap size)
 PASS  -filename removed- (251 MB heap size)

Jest 23.6.0, Jest Circus 23.6.0

node --expose-gc ./node_modules/.bin/jest --logHeapUsage --runInBand --config=./jest.config.js

 PASS  -filename removed- (32.311s, 234 MB heap size)
 PASS  -filename removed- (262 MB heap size)
 PASS  -filename removed- (290 MB heap size)
 PASS  -filename removed- (319 MB heap size)
 PASS  -filename removed- (349 MB heap size)
 PASS  -filename removed- (100.506s, 379 MB heap size)
 PASS  -filename removed- (6.497s, 407 MB heap size)
 PASS  -filename removed- (16.084s, 422 MB heap size)
 PASS  -filename removed- (454 MB heap size)
 PASS  -filename removed- (466 MB heap size)
 PASS  -filename removed- (481 MB heap size)
 PASS  -filename removed- (497 MB heap size)
 PASS  -filename removed- (511 MB heap size)
 PASS  -filename removed- (525 MB heap size)
 PASS  -filename removed- (531 MB heap size)

Jest v24.0.0-alpha.4, Jest Circus v24.0.0-alpha.4

Not sure best way to test #6965, so I picked the latest alpha release.

node --expose-gc ./node_modules/.bin/jest --logHeapUsage --runInBand --config=./jest.config.js

 PASS  -filename removed- (73 MB heap size)
 PASS  -filename removed- (79 MB heap size)
 PASS  -filename removed- (85 MB heap size)
 PASS  -filename removed- (95 MB heap size)
 PASS  -filename removed- (13.113s, 212 MB heap size)
 PASS  -filename removed- (215 MB heap size)
 PASS  -filename removed- (224 MB heap size)
 PASS  -filename removed- (47.523s, 284 MB heap size)
 PASS  -filename removed- (285 MB heap size)
 PASS  -filename removed- (298 MB heap size)
 PASS  -filename removed- (304 MB heap size)
 PASS  -filename removed- (8.881s, 350 MB heap size)
 PASS  -filename removed- (354 MB heap size)
 PASS  -filename removed- (369 MB heap size)
 PASS  -filename removed- (375 MB heap size)

If running it for the full set of test would be more useful let me know specifically what would be helpful and I'll find time.

@SimenB
Copy link
Member

SimenB commented Nov 7, 2018

The subset is fine 🙂 #6965 just landed for jasmine, not circus, I was wondering if you could eager evaluate all stacks and see if it helped

@dylang
Copy link
Contributor Author

dylang commented Nov 9, 2018

hi @SimenB - I'm not sure what you mean by eager evaluate all stacks.

@SimenB
Copy link
Member

SimenB commented Nov 9, 2018

See the description in #6965. But essentially, every place we do new Error in jest-circus, add a line below it that does error.stack = error.stack. This changes the getter into just a string, thus it can free the references it's keeping

@dylang
Copy link
Contributor Author

dylang commented May 5, 2019

For what it's worth, I've updated jest-circus-memory-leak-repo for 24.8.0 and I'm still detecting a memory leak when using JEST_CIRCUS=1.

https://github.com/dylang/jest-circus-memory-leak-repo

@dylang
Copy link
Contributor Author

dylang commented Aug 30, 2021

This seems to be fixed, thank you!

@dylang dylang closed this as completed Aug 30, 2021
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants