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

Add option to specify heap memory limit to underlying JavaScript runtime #367

Open
pranavpudasaini opened this issue May 16, 2024 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@pranavpudasaini
Copy link

Is your feature request related to a problem? Please describe.
While scanning a repo with 4000+ TypeScript files with a total of 100+ rules, it always crashes with the error: "Fatal JavaScript out of memory: Reached heap limit". I'm using a standard GitHub-hosted runner for private repos with 2 cores and 7 GiB memory to perform the test.

Describe the solution you'd like
It would be awesome if there were an option to specify the maximum heap size for the underlying JavaScript runtime. Could be something equivalent to NODE_OPTIONS=--max_old_space_size=2048.

Describe alternatives you've considered
I've tried specifying heap size by adding the command prefix NODE_OPTIONS=--max_old_space_size=2048 to the datadog-static-analyzer binary but the JS runtime doesn't seem to use it.

image

@jasonforal
Copy link
Collaborator

Thanks for the report.

We have been aware of this issue, and we are currently working on some changes that will (among other things) prevent this particular crash.

While we do not currently expose configuration options for memory usage, we may in the future--similarly to how we allow max CPU core configuration.

--

In the meantime, we have adjusted our default rulesets to temporarily disable some static analysis rules that exacerbate this issue. Hopefully that bandaid solves your particular issue, but if not, I'll update this issue when the fixes I mentioned get merged and released.

@pranavpudasaini
Copy link
Author

@jasonforal Thank you for your response!

I've tried running the scans against the same repo but I'm still getting the same issue. We will switch to a larger runner instance until the heap configuration is available.

@juli1
Copy link
Collaborator

juli1 commented May 17, 2024

@pranavpudasaini thank you for the report. Could you please run the analyzer with the option --debug yes (if you use the GitHub action, you can specify it as an input).

This will gives the output about which rule is failing and will help us finding the culprit.

@juli1 juli1 added the bug Something isn't working label May 17, 2024
@pranavpudasaini
Copy link
Author

pranavpudasaini commented May 17, 2024

Hi @juli1, Thank you for the response!

Here are the few last rules that were evaluated just before the crash, the last one being no-duplicate-imports.

typescript-code-style/ban-tslint-comment
typescript-code-style/ban-ts-comment
typescript-code-style/no-confusing-non-null-assertion
typescript-code-style/no-empty-interface
typescript-code-style/no-inferrable-types
typescript-code-style/no-useless-empty-export
typescript-code-style/class-name
typescript-code-style/function-naming
typescript-code-style/parameter-name
typescript-code-style/method-name
typescript-code-style/no-duplicate-imports

@juli1
Copy link
Collaborator

juli1 commented May 22, 2024

Hi @juli1, Thank you for the response!

Here are the few last rules that were evaluated just before the crash, the last one being no-duplicate-imports.

Any change you can share the file that causes the crash?

@SophisticaSean
Copy link

same issue is also happening to us and it's very frustrating

@juli1
Copy link
Collaborator

juli1 commented Jun 7, 2024

We are working on removing all operations that trigger these issues. We should be done by end of month.

@jasonforal
Copy link
Collaborator

Hello @pranavpudasaini and @SophisticaSean, we released a new version of the analyzer (v0.3.7) that should circumvent the initial issue reported (this update greatly reduces v8 allocations, so hitting the heap limit should "never" happen under normal usage).

However, I will be keeping this issue open because v8 will still crash if it hits the heap limit -- we will eventually be adding functionality to gracefully handle this and treat it much like we do an execution timeout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants