Huge memory usage regression: 580 MB in 0.43 → 6300 MB in 0.44

[andersk]

Using Node.js 14.17.5 with NPM 6.14.14 for this reproduction recipe (npx works slightly differently in NPM 7).

$ git clone -b v5.8.1 https://github.com/zulip/zulip-desktop.git
$ cd zulip-desktop
$ npm i
$ rm -rf node_modules/.cache
$ npx -p xo@0.43.0 time xo
…
  110 errors
Command exited with non-zero status 1
45.72user 0.89system 0:28.89elapsed 161%CPU (0avgtext+0avgdata 581408maxresident)k
0inputs+0outputs (0major+155120minor)pagefaults 0swaps
$ rm -rf node_modules/.cache
$ npx -p xo@0.44.0 time xo
…
FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
…
Command terminated by signal 6
200.27user 7.35system 1:29.69elapsed 231%CPU (0avgtext+0avgdata 2274772maxresident)k
0inputs+0outputs (0major+717566minor)pagefaults 0swaps
$ rm -rf node_modules/.cache
$ NODE_OPTIONS=--max-old-space-size=8192 npx -p xo@0.44.0 time xo
…
  148 errors
Command exited with non-zero status 1
182.59user 5.34system 1:54.32elapsed 164%CPU (0avgtext+0avgdata 6284536maxresident)k
2808inputs+0outputs (2major+1631648minor)pagefaults 0swaps

I bisected this memory usage regression to e2e715d “Simplify lintFiles (#583)”. Cc @fisker.

[XhmikosR]

@fisker any chance you could have a look at this please? The slowness is quite noticeable. Thanks!

[fisker]

I'm sorry for the regression.

Before, we search config files before lint, this cause problem here, it is also very slow when I want lint a single file, xo look for all config files(include package.json) , even very deep/unrelated config files.

Now we search config file one by one when actually linting. This solves the problem I described above. But I guess this is also the reason causes huge memory usage.

I'm not sure what to do.

[fisker]

Maybe queue the lint jobs can solve it?

[XhmikosR]

I'm not familiar with the codebase to be able to help. Maybe @sindresorhus has a suggestion.

[andersk]

@devinrhode2 Not sure why you think that. There are no relevant commits between 0.44.0 and 0.45.0, and the problem still reproduces with the reproduction instructions from my original report.

[andersk]

Note that, like I alluded to in the report, npx in the NPM 7 distributed with Node.js 16 has a bug where it incorrectly prefers the installed version over the version explicitly provided on the command line (npm/cli#3210).

But, if you account for that (and don’t forget to delete node_modules/.cache), this reproduces equally with Node.js 16.

[vadimdemedes]

Latest xo consistently crashes with out-of-memory error (I have 8GB of RAM) for me too. Would it be possible to revert e2e715d until this issue is resolved?

[sindresorhus]

I tried limiting the concurrency and it didn't seem to help: 4f05195 So the issue might be somewhere else.

[sindresorhus]

I also tried upgrading to ESLint 8, which also didn't help.

[sindresorhus]

@fisker Using the dev tools memory inspector might reveal where the memory leak is.

[sindresorhus]

I took a quick snapshot in Chrome and it seems like most of the memory is spent on TypeScript. My guess is that since we're now creating a new instance of new ESLint() for each file, something regarding ESLint plugins are not cleaned up and we keep initiating new TypeScript parser instances (which are huge) without releasing them.

[sekhavati]

We use GitHub Actions and after upgrading to 0.44 our CI started failing as the hosted runners were running out of memory (they have 7GB of RAM according to the docs)

For now we've downgraded and are sticking with 0.43 until this issue is resolved. This was on a 6 package monorepo FYI, and the individual packages aren't crazy large or anything.

Thought I'd mention it in case others are quietly hitting the same issue.

[andersk]

@devinrhode2 If you take a look at my original test case, you’ll see that it’s using TypeScript 4.3.5. You’ll also see that I already did the work of bisecting the problem to a specific xo commit. It was not introduced by a TypeScript upgrade or your other suggestions. If you are seeing a problem related to those things, I suggest filing a separate issue with a detailed test case.

[andersk]

@devinrhode2 I’m glad you’re enthusiastic about finding generic ways to improve XO’s memory usage. However, my concern is that you’re making this report harder to follow by presenting generic ideas that aren’t related to the specific regression introduced by a specific XO commit that I reported here. Would you be willing to move your ideas to a new issue? Thanks.