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 when running with Jest #376

Closed
kirillgroshkov opened this issue Mar 30, 2019 · 4 comments
Closed

Memory leak when running with Jest #376

kirillgroshkov opened this issue Mar 30, 2019 · 4 comments
Assignees
Labels
api: datastore Issues related to the googleapis/nodejs-datastore API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@kirillgroshkov
Copy link

kirillgroshkov commented Mar 30, 2019

We use @google-cloud/datastore in our unit tests with Jest.

We discovered that just importing @google-cloud/datastore (as const { Datastore } = require('@google-cloud/datastore')) leaks memory when running in Jest tests.

I've created a minimal repro here to showcase the issue: https://github.com/kirillgroshkov/datastore-jest-leak-repro

Good to note that Jest has memory leak issues with many libraries, not just this one. Some popular libraries, e.g graceful-fs were "fixed" to not leak memory with Jest (given that Jest itself is also a "popular library", number 1 in "StateOfJS 2018" to be precise).

I will open and link a symmetric issue in Jest repo to track, cause I'm expecting some back-and-forth between this repo and Jest. (UPD: jestjs/jest#8247)

Why this issue is important - it blocks us to run our test suite in our CI environment (CircleCI), which has 4Gb memory constraint. It runs out-of-memory already with ~150 test files that we have in our project. See steps to reproduce further.

Some related issues in Jest:
jestjs/jest#6814
jestjs/jest#6738
jestjs/jest#7311
jestjs/jest#6399

Environment details

  • OS: MacOS
  • Node.js version: v10.15.0
  • npm version: 6.9.0
  • @google-cloud/datastore version: 3.1.2

Steps to reproduce

Use minimal repro repository: https://github.com/kirillgroshkov/datastore-jest-leak-repro
Run npm run test-leaking or npm run test-leaking-detectLeaks to see the issue.

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Mar 31, 2019
@bcoe bcoe added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Apr 1, 2019
@bcoe
Copy link
Contributor

bcoe commented Apr 1, 2019

@kirillgroshkov thanks for the bug report and reproduction 👍

With the other memory leaks you've read about, e.g., graceful-fs, is it that Jest surfaced legitimate memory leaks in the constructor, or that Jest runs libraries in a unorthodox manner?

@yoshi-automation yoshi-automation removed the triage me I really want to be triaged. label Apr 1, 2019
@kirillgroshkov
Copy link
Author

Thanks to @scotthovestadt, he found the root cause of memory leak: jestjs/jest#8247 (comment)

So, what can we do here in @google-cloud/datastore to fix it? Or should someone reach to https://www.npmjs.com/package/long ?

@kirillgroshkov
Copy link
Author

kirillgroshkov commented Apr 2, 2019

With the other memory leaks you've read about, e.g., graceful-fs, is it that Jest surfaced legitimate memory leaks in the constructor, or that Jest runs libraries in a unorthodox manner?

Sorry, I don't really understand what you mean here.

At this point I can't tell if it's something special with Jest or not, or if same memory leak is reproducible with other test runners (Mocha, etc). Haven't tried that. We're using Jest exclusively.

But please look at my comment above.

@bcoe
Copy link
Contributor

bcoe commented Apr 2, 2019

@kirillgroshkov thanks for all the great info 👍 and @scotthovestadt thanks for digging into the issue, and finding a reproduction.

I've created a tracking issue on long and am closing this issue, as the underlying issue seems to be more related to the interaction between Jest and WASM, vs., anything specific to this library.

@bcoe bcoe closed this as completed Apr 2, 2019
@google-cloud-label-sync google-cloud-label-sync bot added the api: datastore Issues related to the googleapis/nodejs-datastore API. label Jan 31, 2020
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the googleapis/nodejs-datastore API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

3 participants