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 related to require #9697

Closed
ron23 opened this issue Mar 24, 2020 · 11 comments
Closed

Memory leak related to require #9697

ron23 opened this issue Mar 24, 2020 · 11 comments

Comments

@ron23
Copy link

ron23 commented Mar 24, 2020

🐛 Bug Report

We have a decent size JS&TS project suffering from memory leak. Within a few minutes the heap grows to 2GB and testing becomes extremely slow until I get allocation error.
I've looked at all memory leak related issues but none of them seem to solve this problem.
Specifically: #8331 && #9443.
I tried the patch version in #8331 but that didn't help.
On the heap snapshot I'm seeing many occurrences of stuff that seem related to requiring module but I don't really understand it.
I tried running with --detectLeaks but it seems broken and reports a leak even for a naive test.

execution log (partial)

node --expose-gc ./node_modules/.bin/jest --runInBand --logHeapUsage

...selectors/index.spec.ts (19.693s, 256 MB heap size)
...payment.spec.js (305 MB heap size)
...actions.spec.ts (326 MB heap size)
...selectors.spec.ts (341 MB heap size)
...actions.spec.ts (360 MB heap size)
...orders/action.spec.ts (379 MB heap size)
...selectors.spec.ts (394 MB heap size)
...utils.spec.js (416 MB heap size)
...metrics.spec.ts (427 MB heap size)
...selectors.spec.js (443 MB heap size)
...NotificationCenterContainer.spec.tsx (472 MB heap size)
...actions.spec.ts (479 MB heap size)
...selectors.spec.ts (494 MB heap size)
...translators.spec.ts (512 MB heap size)
...PhoneVerificationContainer.spec.tsx (534 MB heap size)
...apiUtils.spec.js (542 MB heap size)
...Logging.spec.ts (549 MB heap size)
...baseTranslator.spec.js (556 MB heap size)
...reducers.spec.ts (583 MB heap size)
...actions.spec.ts (600 MB heap size)
...actions.spec.ts (626 MB heap size)
...FormContainer.spec.tsx (648 MB heap size)
...audience.spec.ts (657 MB heap size)
...PayModal.spec.tsx (682 MB heap size)
...present.spec.tsx (700 MB heap size)
...utils.spec.ts (719 MB heap size)
...renderers.spec.tsx (740 MB heap size)
...selectors.spec.js (755 MB heap size)
...nativePageMetricUtils.spec.ts (762 MB heap size)
...OrdersReceiptDownloadContainer.spec.js (781 MB heap size)

image

Sampling Profiles snapshots

image

image

Allocation Snapshot

image

Heap Snapshot

I didn't find clear repetition there and since it's massive didn't see the point adding it here.

Performance snapshot

Notice all the pink is requiring modules

image

Allocation timelines - Allocation view

This is just an example, many of the nodes have this nested tree structure
image


Expected behavior

Not have a leak and the heap size shouldn't grow that much.

Link to repl or repo (highly encouraged)

I wasn't able to isolate the issue.

envinfo

  System:
    OS: macOS Mojave 10.14.6
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Binaries:
    Node: 12.14.0 - ~/.nvm/versions/node/v12.14.0/bin/node
    Yarn: 1.21.1 - ~/.nvm/versions/node/v12.14.0/bin/yarn
    npm: 6.13.4 - ~/.nvm/versions/node/v12.14.0/bin/npm
  npmPackages:
    jest: ^24.9.0 => 24.9.0 
yarn list graceful-fs
├─ @jest/transform@24.9.0
│  └─ write-file-atomic@2.4.1
│     └─ graceful-fs@4.2.3
├─ cacache@13.0.1
│  └─ graceful-fs@4.2.3
├─ caching-transform@1.0.1
│  └─ graceful-fs@4.2.3
├─ configstore@3.1.2
│  └─ write-file-atomic@2.4.2
│     └─ graceful-fs@4.2.3
├─ graceful-fs@4.1.15
├─ jscodeshift@0.7.0
│  └─ graceful-fs@4.2.3
├─ readdirp@2.2.1
│  └─ graceful-fs@4.2.3
└─ write-file-atomic@2.4.3
   └─ graceful-fs@4.2.3
✨  Done in 3.88s.

Edit:

I tried running with w=1 and test was fast but with a leak. When I ran with w=4 it was MUCH slower but the leak was much smaller (if at all). Using strong 2019 Macbook.

@kirillgroshkov
Copy link

We have a similar leak / heap growth speed, very annoying:

TZ=UTC DEBUG_COLORS=1 NODE_ICU_DATA=./node_modules/full-icu JEST_SILENT=1 APP_ENV=test jest --logHeapUsage --passWithNoTests --maxWorkers=1 --silent
 PASS  src/test/api/endpoint.user.security.test.ts (6.378s, 136 MB heap size)
 PASS  src/session/handlers/session.login.handler.test.ts (137 MB heap size)
 PASS  src/session/handlers/session.auto.handler.test.ts (160 MB heap size)
 PASS  src/session/handlers/session.consumeSessionToken.handler.test.ts (188 MB heap size)
 PASS  src/session/handlers/session.createToken.handler.test.ts (220 MB heap size)
 PASS  src/session/handlers/session.logout.handler.test.ts (230 MB heap size)
 PASS  src/account/handlers/account.changePassword.handler.test.ts (275 MB heap size)
 PASS  src/qa/handlers/qa.commonality.handler.test.ts (272 MB heap size)
 PASS  src/account/handlers/account.patch.handler.test.ts (321 MB heap size)
 PASS  src/qa/persona.service.test.ts (352 MB heap size)
 PASS  src/mailchimp/mailchimp.sync.service.test.ts (384 MB heap size)
 PASS  scripts/util/kubernetes.util.test.ts (369 MB heap size)
 PASS  src/account/account.service.test.ts (400 MB heap size)
 PASS  src/account/handlers/account.complete.handler.test.ts (440 MB heap size)
 PASS  src/account/handlers/account.downloadData.handler.test.ts (451 MB heap size)
 PASS  src/product/product.service.test.ts (487 MB heap size)
 PASS  src/account/handlers/account.verifyEmail.handler.test.ts (524 MB heap size)
 PASS  src/account/handlers/account.changeEmail.handler.test.ts (579 MB heap size)
 PASS  src/account/handlers/account.demoModeEnter.handler.test.ts (612 MB heap size)
 PASS  src/account/handlers/account.emailVerification.handler.test.ts (653 MB heap size)
 PASS  src/account/handlers/account.demoModeExit.handler.test.ts (695 MB heap size)
 PASS  src/account/handlers/account.changeUnits.handler.test.ts (738 MB heap size)
 PASS  src/account/handlers/account.checkEmail.handler.test.ts (775 MB heap size)
 PASS  src/account/handlers/accountRegister.test.ts (816 MB heap size)
 PASS  src/order/order.service.test.ts (846 MB heap size)
 PASS  src/payment/payment.service.test.ts (885 MB heap size)
 PASS  src/account/handlers/account.sendVerificationEmail.handler.test.ts (922 MB heap size)
 PASS  src/cron/medical/pregFollowUp.service.test.ts (948 MB heap size)
 PASS  src/eta/eta.service.test.ts (987 MB heap size)
 PASS  src/app/handlers/appInit.test.ts (1014 MB heap size)

@ron23
Copy link
Author

ron23 commented Mar 25, 2020

I added "Allocation timelines - Allocation view" screenshot to the original ticket. I don't know if that's normal but seems suspicious to me. There are many nodes with the same long nested recursion.

@SimenB
Copy link
Member

SimenB commented Apr 29, 2020

@ron23 could you test with 25.5? In addition to the graceful-fs fix it's more aggressively clearing require caches after test runs rather than relying on GC

@ron23
Copy link
Author

ron23 commented May 1, 2020

@SimenB I've been following #9443 :)

I tried it when it got merged I also updated JSDOM to jest-environment-jsdom-sixteen (so I can use detect-leaks flag. I'm on node 12) and with both fixes in it seems like all leaks were gone but only when running with -logHeapUsage. Probably due the enforced cal to gc()?
If I run it without the flag I don't know if it leaks it's just super slow.
If I run with it, it finishes in 500-600 seconds (which is a lot but I think that might be another issue)

I still get leaks reported if I run with --detect-leaks but I'm not sure if that false positive or not.

@SimenB
Copy link
Member

SimenB commented May 1, 2020

detect-leaks should work in all versions... Do you still see the require stuff you talked about in the OP though? Might be that's been fixed but there's something else going on. Manual calls to gc() shouldn't be needed...

@ron23
Copy link
Author

ron23 commented May 1, 2020

detect-leaks should work in all versions...

I had the same issue as in #9507

I'll rerun it with inspect to check the require part

@SimenB
Copy link
Member

SimenB commented May 1, 2020

That should be fixed in jsdom@16.2.2

@ron23
Copy link
Author

ron23 commented May 1, 2020 via email

@SimenB
Copy link
Member

SimenB commented May 1, 2020

Oh right, you meant that you need it for detect-leaks to pass, I thought you meant the command failed or something.

I do find it weird a manual GC makes a difference, if anything it should be slower as GC is quite slow, so normally it's better to just let V8 run it when it feels like it

@SimenB
Copy link
Member

SimenB commented Mar 3, 2022

It's been a couple of years, so I'll close this. From what I can tell, this was never tested after the JSDOM fix? If it's still a problem, please open up a new issue with reproduction steps.

@SimenB SimenB closed this as completed Mar 3, 2022
@github-actions
Copy link

github-actions bot commented Apr 3, 2022

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 Apr 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants