adobe / brackets

An open source code editor for the web, written in JavaScript, HTML and CSS.
http://brackets.io
MIT License
33.27k stars 7.63k forks source link

Version 1.8: Constant high CPU usage (JS files only) #13109

Open eKoopmans opened 7 years ago

eKoopmans commented 7 years ago

Prerequisites

Description

Version 1.8: Constant high CPU usage (JS files only) I discovered high CPU usage (50%+ on single-core PC, 25-30% on dual-core laptop) after upgrading to Brackets 1.8. Some details:

After some digging, I tracked the problem down to one specific file, with peculiar behaviour. The file is a JS package for PouchDB: pouchdb-quick-search. Specifically the minified version - I have tried replicating with the non-min version, without the same results.

The problem occurs only when there are two or more copies of the file in the project. The files can be in different folders, or at the same level with different names. If you open a project with two copies of that file in it, you're going to have a bad time.

For now, I've just reverted to Brackets 1.7 as a temporary solution.

Steps to Reproduce

Download this ZIP of a Certified Buggy Project™, or:

  1. Download pouchdb-quick-search.min.js.
  2. Create a new project folder, and put 2 copies of the file in (rename one, or put it in a subdirectory).
  3. Open the project in Brackets 1.8 and view either file, or create a new .js file. Viewing any JS file will cause persistent high CPU usage.
  4. As a comparison, create/view an HTML file. CPU usage should be normal.

Expected behaviour: Normal CPU behaviour (spike on loading, return to low/0).

Actual behaviour: CPU usage climbs to one full thread and persists indefinitely.

Versions

Windows 10, Brackets Release 1.8 build 1.8.0-17108 Behaviour was not present on version 1.7.

swmitra commented 7 years ago

@eKoopmans Thanks for raising the issue with critical details and detailed analysis from your side 👍. After looking at your analysis, to me it looks like a looping issue in Tern inference engine. But this is just an assumption. Can you please let me know the following details ?

eKoopmans commented 7 years ago

Hi @swmitra:

  1. The CPU spike is in the Brackets background process (it also climbs from ~50MB to ~120MB memory usage in my test case and stays there).
  2. Yes, the issue exists even if both min files are at a nested level and I open a JS file at the root.
zaggino commented 7 years ago

@swmitra moving tern analysis to node might help this, but I'd propose doing something that I've already implemented in Brackets-Electron, which is to separate node domains into separate processes instead of running them under single process as Brackets does it now. It's a bit of work but it's definitely worth the performance gain.

eKoopmans commented 7 years ago

@zaggino Unless I'm missing something, this isn't an issue of performance gain - there is some logic error that is causing Brackets to get stuck continually computing. Are you suggesting that having separate processes would prevent the infinite loop? Have you tested whether the bug is present in Brackets-Electron?

zaggino commented 7 years ago

@eKoopmans I'd bet with about 99% certainty that the JS code analysis (which is currently done in browser process) is blocking everything else, hence freezing (this happens in Electron too). It is probably not looping indefinitely, but parts of it are synchronous in nature and that's why we'd want to push it to node (so it doesn't block browser process) and even better, push it to separate node process (so it doesn't block other node stuff).

zaggino commented 7 years ago

Even if it's looping indefinitely, it's better to have "code hints stopped working but everything else is fine" bug instead of "everything crashed" bug.

eKoopmans commented 7 years ago

Okay, well I agree that separating the processes would be better behind the scenes. I should point out though that Brackets is not freezing, the browser process isn't being blocked, everything is still responsive. I just have constant high CPU usage in the background.

So in that sense it doesn't sound like separating the processes would be a solution to the current problem, which is that something behind the scenes (i.e. the JS code analysis) is getting confused and eating up as much CPU as it can get its hands on. But yes, if you were then monitoring that process and killing it if it was misbehaving, then that sounds like a solution.

ficristo commented 7 years ago

Could you also try the tern demo and see if there is a CPU hit there too?

swmitra commented 7 years ago

Good idea @ficristo :+1. But if this was a problem with the tern version that we are using and later got fixed in latest tern then it might not be reproduced in the demo. But still it's worth trying.

ficristo commented 7 years ago

I just noticed there is a new release of tern https://discuss.ternjs.net/t/tern-0-21-0-released/110

eKoopmans commented 7 years ago

Thanks for the suggestion @ficristo! I tried the tern demo, created two "new files" with the contents of the problem file, and... nothing. No performance hit. As @swmitra mentioned, it might just be the different version of tern...

swmitra commented 7 years ago

@ficristo @zaggino As part of porting JS code hints to node, shall we upgrade Tern and Acorn? Looks like there are quite a few fixes which has gone in Tern.

zaggino commented 7 years ago

@swmitra yes, we I believe that we should use the latest version of both before we merge that PR to avoid going through the upgrade process soon

eKoopmans commented 7 years ago

Hi, has anyone managed to replicate (or not replicate) this problem?

zaggino commented 7 years ago

I can only replicate code hints crashing my entire Brackets installation, but have removed them since then. (I couldn't replicate it with some steps, happens seemingly randomly for me)

eKoopmans commented 7 years ago

Cool, thanks @zaggino. Didn't want to be causing problems if I was the only one with the issue.

zaggino commented 7 years ago

Opened https://github.com/adobe/brackets/pull/13136 which will allow disabling default extensions like JS Code Hints

redmunds commented 7 years ago

FYI, there are already preferences for disabling all code hints (showCodeHints) or individual types of code hints (e.g. codehint.JSHints).