-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Increased memory usage when creating a watch program that includes local JavaScript files in node_modules #58011
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
Comments
@andrewbranch Any ideas why 385db44 might have triggered this issue? |
Because weβre loading more of your own JS files instead of giving you implicit |
I understand that more files need to be loaded and that will lead to more memory usage, but 6,000 MB more memory? A 300% increase? That seems excessive to say the least.
We have a mix of TypeScript and JavaScript in the monorepo. The TypeScript is the application code, while the JavaScript is used for tooling (like ESLint rules, Storybook configurations, build scripts, etc). The reason why we use JavaScript for those types of projects is so that we don't need to compile them before we can use them. For example, we can just run ESLint and it uses our custom rules instead of us first having to compile our rules from TypeScript into JavaScript. It just makes life easier. The TypeScript and JavaScript projects have separate configuration files ( Having this combined configuration ensures that VS Code (via the ESLint plugin) will check any file in the workspace because the files from all projects are in the TypeScript project that ESLint is using. |
Your |
Unfortunately, the extended diagnostics from
I'll try to work out how to get the same information from the watch host compiler. |
@andrewbranch Here are the extended diagnostics from the watch host compiler for 5.3.3 and 5.4.5:
|
Something is definitely weird. All the correlates of memory usage increased modestly, as expected, while actual memory usage shot up. And all the times increased somewhat within the expected range too, except for program time, which shot up inexplicably. If you look at your smaller repro, the same pattern doesnβt existβprogram time increased only slightlyβso Iβm not sure itβs a good representation of whatβs going on here. Edit: yeah, in the small repro, the time/memory increase can be explained by the newly included JS pulling in all the Storybook type definitions. It looks like those are fairly expensive to process, judging by the type instantiation diff. Something different is happening in the private repo that you havenβt captured in the small version. |
@sheetalkamat can you think of a way that #56946 could be having an unexpected interaction with watch programs specifically thatβs creating a huge memory / program construction time increase? |
I am not sure whats going on. |
Apologies for the slow response. I've discovered an important piece of information. This only happens on Windows. I was able to set up a WSL environment with the same projects and test script. I tried copying the TypeScript code into I also tried running the test script against the projects and Back on Windows, here's the extended diagnostics from
I'll work on getting the |
Hm, it's a long shot, but if this happens on Windows but not on Linux, can you disable |
That does indeed seem to be causing the problem. With
|
Hm, that time is still 88 seconds, no? Aren't you expecting 62s? |
88 seconds is about on par with a regular |
But, the key is that memory usage is lower? Not totally seeing the change or memory increase from the above, now that I look. |
Yeah, when |
This looks very similar to typescript-eslint/typescript-eslint#1192 then. But that's utterly confusing because this issue is new, while the eslint one is not... Probably not technically similar given the ESLint thing seems to be a diagnostic leak of some sort, but maybe something there... |
@reduckted When you get a chance, can you try using |
Unfortunately, I think that made it worse. π¬ Extended Diagnostics from @typescript-deploys/[email protected]
|
That's even more confusing. It's a shame this repo is closed, as heap snapshots would be pretty helpful to know what's leaking... |
@jakebailey i dont think that fix is right. The issue doesnt repro with Update: Also another option to try is to use |
Ah, I forgot that this was about the public API; too many examples that mention But |
Can you try this build? #58352 (comment) |
So i looked into the repro you shared a little bit and initially i was suprised as to why there is extra memory compared to just using plain program API and then I realised that you have not set What i noticed in your said example is that
Later on you have also reported that the issue repros only if you set Also i have a theory that i am going to work out and create a PR but having a concrete repro really helps speedup investigation rather than us trying things based on some theories. |
No improvement with that one either.
I've actually managed to anonymize the repository, so I will be able to share it soon. I just need to verify that everything has been anonymized. It's a large repo, so it's taking some time to review. Thanks for the patience with this. π |
Really appreciate the efforts for repro .. it will help with the right fix . Thanks |
Can you please try #58398 (comment) to see if this fixes your issue. |
I've finally been able to get the reproduction in a state where I can share it. https://github.com/reduckted/repro-typescript-memory-usage-anonymized All of the TypeScript files just contain imports and exports - there's no other code, so the overall memory usage is quite a bit lower than what the private repository has, but you can still see a very significant increase in memory. What I've discovered while putting this reproduction together is that the memory usage is low if the path of the TypeScript configuration file exactly matches the path on disk. However, if you specify a path that is a different case (which is what I've included the reproduction steps in the repository, but I'll post them here as well. Just a reminder that this only occurs on Windows. cd Workspace
npm install
npm start This will convert the full path of the TypeScript configuration file to lowercase - note that the To run the same code using the real file path without changing the case, run: npm run real-path This is what I am seeing as the results:
|
That has made a huge improvement! There's still a slight increase in memory between using an exact file path and a lowercase file path, but it's much, much closer than it was.
vs
|
π Search Terms
Reached heap limit Allocation failed out of memory createWatchCompilerHost createWatchProgram
π Version & Regression Information
allowJs
, process JS files found searching node_modules when they have a realpath outside node_modulesΒ #56946β― Playground Link
https://github.com/reduckted/repro-typescript-memory-usage
π» Code
This cannot be reproduced with a few lines of code. It needs a large TypeScript project. Use these steps to see the increased memory usage:
git clone https://github.com/reduckted/repro-typescript-memory-usage
npm i
node run.js
and see the memory usage (this will be for TypeScript 5.4.3).npm i [email protected] -D
node run.js
and see the memory usage.For the linked repository, I am seeing 525 MB used in 5.3.3 and 775 MB used in 5.4.3. The larger the TypeScript project, the worse the memory usage is.
π Actual behavior
The memory consumed by TypeScript 5.4 is much larger compared to 5.3. The larger the TypeScript project, the worse the memory usage is. Large projects can cause Node.js to run out of memory.
π Expected behavior
TypeScript should have equivalent memory usage to v5.3.
Additional information about the issue
What I've been able to determine is that when you have a JavaScript project in the repository and install it as a node module, creating a "watch" program that includes files that
require
files from that project will consume significantly more memory in 5.4.3 compared to 5.3.3.I encountered this problem when running ESLint with a config that uses the
@typescript-eslint/*
plugins. Using TypeScript 5.3.3, ESLint would run fine. After upgrading to 5.4.3, ESLint would crash with an out of memory error:The repository that this was occurring on is an internal, closed-source company project, so I can't share it and signing an NDA will not be possible. However, I have been able to see an increased memory usage with a similar project setup in other projects. The repository I have linked above is a clone of ionic-team/ionic-framework with a few additions to it.
This is roughly the setup in the closed-source repository:
source/
directory.@my/project
(for example) rather than use relative paths..storybook/
folder at the root. The files in that folderrequire
some of the JavaScript projects..storybook/
directory, as well as a few other files that aren't relevant to this issue.I've created a similar setup in the linked repository.
.storybook/
folder require the JavaScript projects: https://github.com/reduckted/repro-typescript-memory-usage/blob/main/.storybook/main.js.storybook/
directory: https://github.com/reduckted/repro-typescript-memory-usage/blob/main/tsconfig.memory.jsonI've created a script called
run.js
that does a simplified version of what the@typescript-eslint
plugin does. It creates a "watch" compiler host, then creates a "watch" program:Using the linked repository, I'm seeing this when using TypeScript 5.3.3:
And this for TypeScript 5.4.3:
An extra 250 MB of memory used. If you remove
".storybook/**/*.js"
from the TypeScript configuration, the amount of memory used by TypeScript 5.4.3 is about 580 MB, so still a little bit higher that 5.3.3, but significantly less that 5.4.3.The larger the project, the worse the memory usage is.
In the closed-source repository, if I remove about half the code, I get these results:
If I include all of the code and increase the heap size, I get these results:
As you can see, that's an extra 6 GB of memory compared to 5.3.3! π€―
Here's a summary of the increased memory usage:
The text was updated successfully, but these errors were encountered: