-
Notifications
You must be signed in to change notification settings - Fork 357
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
High CPU usage while idle #415
Comments
Same here, CPU at ~30%, lagging intellisense, slow to load and unload the project. It has been a while since I have this problem, I thought it was due to #384, but it's not since the issue was closed and I use now the 1.1 RC2. Don't know if it's related, but I start to get problems once I add the npm package |
Trying to get to the bottom of this now. I seem to have a repro. Though it doesn't match the call stacks above, it does fully consume a thread on a quad core, meaning about 25% constant CPU. In my repro it looks like its updating the NPM catalog that's the issue. It seems the offending line of code is in the below code (see my comment annotation): NpmGetCatalogCommands.cs@100 while (module != null) {
try {
builder.Name = (string)module["name"];
if (string.IsNullOrEmpty(builder.Name)) {
continue; /**** just repeats the while loop with no change in state ****/
} It appears the data coming from NPM from the catalog isn't as expected, so trying to root cause this now... |
I have issue with npm catalog with duplicate properties in package.json. so that's could be the cause of wrong data |
The logic for the JSON schema parsing was a little off. For example, if you go to https://registry.npmjs.org/-/all/static/today.json you can see it just returns an array of package objects. The current code was parsing incorrectly and getting in an infinite loop (as outlined above). I've started a branch to fix this. Currently is has one commit to handle the schema properly (7900e27). It still needs to be updated for where it was expecting to find an Going to bed now. Will finish this up over the coming days (and try any other repros for the high CPU). |
@billti I would suggest thing about other JSON library, or take another approach. The reason for that is #106 which seems trivial to implement, but with current approach this is not possible. NTVS need more flexible JSON parser. Same with #208, too strict parser is good when developing application, but since you could not restrict NPM and your users NTVS should be more tolerable to issues in the external deps. I look at the Newtonsonft.JSON API and does not find a way how I could implement loose JSON. If you have an idea, I could try work on these kind of issues. Also, do you have any news on this issue? |
The JSON parse in this case is fine and doing exactly what is expected. It is the NTVS code which is wrong, and looping without advancing the parser - hence the infinite loop and CPU usage. When you say "Do you have any news on this issue?", you mean since 3 days ago when I last updated this? Then no. It's been a long weekend here and I managed to keep my laptop closed for most of it :-) |
You are right, it's only 3 days. I have to work less, to not push people who don't work on weekends. My apologize for being too pushy 😄 |
Per @nippur72 above, I added the Looking at the call stacks when broken, I see similar to the below quite consistently:
Looking at the diagnostics output and what it is analyzing at this point, I see the below:
Per the code at https://github.com/Microsoft/nodejstools/blob/master/Nodejs/Product/Nodejs/Microsoft.NodejsTools.targets#L12 , we should be ignoring As |
microsoft#415 High CPU usage when using core-js - we were not properly initializing _analysisIgnoredDirectories in certain scenarios. This was also preventing us from ignoring core-js by default, which was causing perf issues with some popular packages.
@kant2002 regarding your first comment about CPU usage, what's the output in NTVS diagnostic info look like? (Tools > Node.js Tools > Diagnostic Info) that'll give us an idea of which packages are failing to complete analysis. |
microsoft#415 High CPU usage when using core-js and bower_components - we were not properly initializing _analysisIgnoredDirectories in certain scenarios. This was also preventing us from ignoring certain directories by default, which was causing perf issues with some popular packages.
microsoft#415 High CPU usage when using core-js and bower_components - we were not properly initializing _analysisIgnoredDirectories in certain scenarios. This was also preventing us from ignoring certain directories by default, which was causing perf issues with some popular packages.
@mousetraps, I managed to remove NpmCache folder and update catalog and issues was gone. I also suspect that this is fix which already implemeted by Bill. If this issue happens to me I report again with diagnostics. |
@kant2002 Sounds good! The original callstack was in the analysis code, not the npm cache, so I'm skeptical it's the same issue. |
for us npm newbies, can you tell me how you remove the cache, and then update the catalog? |
Please don't confuse NPM cache, with cache of NPM packages for NTVS. |
@DrYSG To answer your second question, I just open VS install package dialog and NTVS do that for me automatically. |
Thanks @kant2002, I did suspect I was confusing the two, which is why I wrote rather than just do something stupid first. Here is what I did, Just to be safe I renamed (rather than deleted the .sqlite file and the all-packages.json file. When NodeTools downloaded it deleted everything and rebuilt the folder, and the sqlite file. It all works. |
#316, #415 - AnalysisIgnoredDirectories not working and high cpu usage when using core-js and bower_components We were not properly initializing _analysisIgnoredDirectories on project load. This was also preventing us from ignoring core-js and bower_components by default, which was causing perf issues with some popular packages. Fixes #316, related to #415
@nippur72 @kant2002 We just pushed a new release with some fixes that could resolve this. See https://github.com/Microsoft/nodejstools/releases if you'd like to check it out. Thanks! |
As |
@billti good work, I've just tried it and now the slowness is completely gone. Thank you, I've been waiting this for long! |
I open my project, working on it for some time, then during seeking and finding stuff which I need the VS start freezing and eat ~30% CPU. All CPU distributed evenly across the processors.
It taking ~20 minutes so far and I kill process.
I take two dump to take a look at what happens. Both show on the same place - Quick Intellisense source. Take a look. I was on RC1.1 Release version + VS2015.
The text was updated successfully, but these errors were encountered: