Reduce amount of memory required for npm run lint

[ThatOneBro]

As the codebase grows, it seems that our npm run lint is running out of memory at higher and higher memory usage.

There are a few things that may be worth trying:

1. Linting only certain parts of the codebase at a time. I tried this before and I couldn't get it to stop it from going OOM.
2. Diagnosing which rules are actually taking up so much time. It seems like some rules must be doing some deeply recursive calls and keeping track of a lot of things without allowing GC to run. It may be worth investigating if people have found workaround for making these rules use less memory.

Just seems like the constant increase in memory requirement to run lint is not sustainable, so we might want to keep a look out for solutions here

[codyebberson]

Yeah.... 😢

I've noticed that running eslint on a single file can be big and slow, which I think indicates that some of our plugins are trying to read in the entire universe before doing anything.

On one hand, as you say, this feels slightly unsustainable.

On the other hand, given that a single file is slow, some of these performance concerns may be O(1)? 🤷

As for possible actionable solutions:

1. We could take stock of our existing plugins. There might be some we can simply eliminate.
2. We could consider a "fast" eslint config for dev time, and a "slow" eslint config for CI/CD (and manual runs if you want)
3. We could consider alternate solutions like Biome...

Unfortunately I don't think Biome is ready for prime time quite yet. In particular, there is no equivalent for no-floating-promises. See: biomejs/biome#3

[ThatOneBro]

Yeah I'm mostly concerned about it in CI, since I had to bump --max-old-space-size to 6GB for build.sh since you can't run npm run lint with 5GB of memory anymore 🙃

This doesn't get better if you lint individual packages at a time (still run OOM consistently); maybe linting individual files one at a time will work but that's definitely going to increase the time to lint significantly in CI or if you want to lint the whole codebase locally.

I think we have to figure out what is using so much memory, 6GB is quite ludicrous imo 😅

[codyebberson]

Using TIMING=1 trick:

Rule                                          | Time (ms) | Relative
:---------------------------------------------|----------:|--------:
@typescript-eslint/no-floating-promises       | 11471.968 |    43.1%
@typescript-eslint/prefer-optional-chain      |  1171.389 |     4.4%
@typescript-eslint/no-base-to-string          |   935.673 |     3.5%
@typescript-eslint/no-unused-vars             |   890.750 |     3.3%
@typescript-eslint/no-unsafe-enum-comparison  |   868.960 |     3.3%
jsdoc/check-tag-names                         |   807.519 |     3.0%
jsdoc/valid-types                             |   628.463 |     2.4%
jsdoc/check-types                             |   519.871 |     2.0%
jsdoc/require-hyphen-before-param-description |   455.748 |     1.7%
jsdoc/require-property-name                   |   442.038 |     1.7%

I know your main concern is memory usage, but I assume time is roughly correlated with memory usage.

I wonder if we could sponsor someone to port the no-floating-promises to Biome 🤔

[codyebberson]

Using NODE_OPTIONS="--max-old-space-size=8192"

Current settings on main, max memory usage is: 7.0 GB

Disabling no-floating-promises, max memory usage is: 6.7 GB

Disabling all @typescript-eslint rules, max memory usage is: 4.9 GB

Removing parser: '@typescript-eslint/parser',: doesn't work

Removing '@typescript-eslint' from plugins: 4.9 GB

This is not a perfect test, because Node.js will change it's GC behavior depending on --max-old-space-size... The real test would be to systematically try reducing --max-old-space-size and find the breaking point.

[codyebberson]

The implementation of no-floating-promises relies heavily on the TypeScript compiler: https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/rules/no-floating-promises.ts

Of the Biome TypeScript ESLint rules, I don't see any that come close to this level of semantic analysis. Most of them are simple checks that can be performed by just looking at the tokens.

To implement no-floating-promises would require some pretty heavy duty parsing. Consider:

import { foo } from 'bar';

foo();

There's no shortcuts here 🫤

[ThatOneBro]

Currently on my branch npm run lint crashes with SIGABORT at --max-old-space-size=5120, so that already means that minimum to even run (not even at a decent speed) is over 5GB

[codyebberson]

Attaching --prof to eslint + --prof-process, most of the time is just in the TypeScript compiler

 [JavaScript]:
   ticks  total  nonlib   name
    131    1.6%    2.7%  JS: *scan C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:10259:18
     64    0.8%    1.3%  JS: *sourceFileNotUptoDate C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:119082:35
     63    0.8%    1.3%  JS: **:not(Program) C:\Users\cody\dev\medplum\node_modules\eslint-plugin-jsdoc\dist\iterateJsdoc.cjs:1925:25
     62    0.8%    1.3%  JS: *bind C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:43681:18
     56    0.7%    1.2%  RegExp: import|require {1}
     55    0.7%    1.1%  JS: *<anonymous> C:\Users\cody\dev\medplum\node_modules\eslint\lib\linter\timing.js:139:24
     45    0.6%    0.9%  JS: *enterNode C:\Users\cody\dev\medplum\node_modules\eslint\lib\linter\code-path-analysis\code-path-analyzer.js:788:14
     44    0.5%    0.9%  JS: *getTokenBefore C:\Users\cody\dev\medplum\node_modules\eslint\lib\source-code\token-store\index.js:291:19
     42    0.5%    0.9%  JS: *getLocFor C:\Users\cody\dev\medplum\node_modules\@typescript-eslint\typescript-estree\dist\node-utils.js:222:19
     32    0.4%    0.7%  JS: *BackwardTokenCommentCursor C:\Users\cody\dev\medplum\node_modules\eslint\lib\source-code\token-store\backward-token-comment-cursor.js:31:16
     30    0.4%    0.6%  JS: *bindChildren C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:42410:26
     27    0.3%    0.6%  JS: *canHaveJSDoc C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:15038:24
     27    0.3%    0.6%  JS: *bindWorker C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:43744:24
     25    0.3%    0.5%  JS: *parseMemberExpressionRest C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:32716:43
     22    0.3%    0.5%  JS: *parseDelimitedList C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:30447:36
     21    0.3%    0.4%  JS: *createNodeArray C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:29804:33
     20    0.2%    0.4%  JS: *parseMemberExpressionOrHigher C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:32312:47
     20    0.2%    0.4%  JS: *getReferencedFileLocation C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:119033:37
     19    0.2%    0.4%  JS: *createIdentifier C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:29855:34
     18    0.2%    0.4%  JS: *getTransformFlagsSubtreeExclusions C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:24500:46
     17    0.2%    0.4%  JS: *doJSDocScan C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:34812:33
     16    0.2%    0.3%  JS: *isProgramUptoDate C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:119062:29
     16    0.2%    0.3%  JS: *get node:internal/bootstrap/node:371:8
     16    0.2%    0.3%  JS: *bindEach C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:42401:22
     15    0.2%    0.3%  JS: *parseAssignmentExpressionOrHigher C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:31785:51
     14    0.2%    0.3%  RegExp: import|require
     14    0.2%    0.3%  JS: *scanString C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:9890:24
     14    0.2%    0.3%  JS: *parseUpdateExpression C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:32273:39
     14    0.2%    0.3%  JS: *checkTypeRelatedTo C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:62714:32
     13    0.2%    0.3%  JS: *resolveNameHelper C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:47308:31
     12    0.1%    0.2%  JS: *parseTagComments C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:35065:38
     12    0.1%    0.2%  JS: *bindCondition C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:42711:27
     12    0.1%    0.2%  JS: *<anonymous> C:\Users\cody\dev\medplum\node_modules\esquery\dist\esquery.min.js:1:29413
     11    0.1%    0.2%  JS: *parsePrimaryExpression C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:32855:40
     11    0.1%    0.2%  JS: *iterateCommentRanges C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:9410:32
     11    0.1%    0.2%  JS: *getReducedASTNode C:\Users\cody\dev\medplum\node_modules\@es-joy\jsdoccomment\dist\index.cjs.cjs:838:36
     11    0.1%    0.2%  JS: *getPropertyOfType C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:57450:31
     11    0.1%    0.2%  JS: *<anonymous> C:\Users\cody\dev\medplum\node_modules\esquery\dist\esquery.min.js:1:29011
     11    0.1%    0.2%  JS: *<anonymous> C:\Users\cody\dev\medplum\node_modules\eslint\lib\linter\linter.js:1003:42
     10    0.1%    0.2%  JS: *parseCallExpressionRest C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:32780:41
     10    0.1%    0.2%  JS: *isDeprecatedSymbol C:\Users\cody\dev\medplum\node_modules\typescript\lib\typescript.js:46800:32

So clearly it's due to ESLint + TypeScript.

Given that tsc by itself doesn't cause memory issues, it could be related to our ESLint TypeScript config ??

When you ran project-by-project, did you do that from the root directory or from within the individual package directory?

---

Zooming out... I'm a little skeptical that there's any low hanging fruit here. Running one project at a time may help, but as you noted, it's not a silver bullet.

I believe that the @typescript-eslint rules are pretty important / non-negotiable.

I'm open to suggestions, but I don't see any easy solutions here 😞 Lotta lead bullets, no silver bullets.

[codyebberson]

https://news.ycombinator.com/item?id=38912104

[codyebberson]

Using eslint --cache is crazy fast, but apparently it is not recommended when using typescript-eslint: typescript-eslint/typescript-eslint#4694

[ThatOneBro]

Yeah maybe no real solution for now other than scaling everything up... definitely should keep a passive look out for potential solutions, especially around Biome and the like.