Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Sprint 28 - Code slowdown in JavaScript #4608

Closed
cfjedimaster opened this issue Jul 30, 2013 · 40 comments
Closed

Sprint 28 - Code slowdown in JavaScript #4608

cfjedimaster opened this issue Jul 30, 2013 · 40 comments

Comments

@cfjedimaster
Copy link
Contributor

I'm not 100% convinced - but as I was using Spring 27 this morning and then switched to Sprint 28 as soon as I saw it - I think typing is slower now. In a few cases Brackets was 1 or 2 characters behind where it should be. It caught up and I'm not able to scientifically reproduce this, but it really feels a bit slower than before.

In my current project I'm working on an HTML file with a script block on top. The slowdown seems to be just in the JS area.

@njx
Copy link

njx commented Jul 30, 2013

There is likely to be some slowdown due to the workaround we had to put in to avoid a Chromium crash, which involves deoptimizing some of the parsing code we use in JS code hints. However, the slowdown should only be noticeable during reindexing on a large project (like Brackets), and should go away after a few seconds (though it will come back briefly if you switch back into the app and then switch files). Is that consistent with what you're seeing?

@cfjedimaster
Copy link
Contributor Author

Hmm. Today I've been "in and out" of Brackets so maybe I'm seeing it
because I switch back into it quite a bit. I wrote a new function that
looks like this:

function thisiS() {
var x= 1;
var y = 2;
var z = 9;
for(var i = 0; i<10; i++) {
var a = 1;
var b = 2;
var z = 3;
var
}
}

Yeah, that's garbage, but i was trying to code quick. I noticed a slowdown
at the beginning of each line. A small one - but a definite break between
when I hit 'v' and when it showed up.

I'm seeing slowdowns whenever code assist pops up. I think that is the
culprit.

I'd be happy to do a quick video recording as well if that may help.

@dangoor
Copy link
Contributor

dangoor commented Aug 1, 2013

There were two more reports of this on the mailing list. I'm going to set this to high priority for this sprint for now so that we investigate.

@dangoor
Copy link
Contributor

dangoor commented Aug 1, 2013

Here's a video of this recorded by Sathyamoorthi.

@cfjedimaster
Copy link
Contributor Author

Glad to know I'm not crazy. (At least in this regard.)

On Thu, Aug 1, 2013 at 7:41 AM, Kevin Dangoor notifications@github.comwrote:

There were two more reports of this on the mailing list. I'm going to set
this to high priority for this sprint for now so that we investigate.


Reply to this email directly or view it on GitHubhttps://github.com//issues/4608#issuecomment-21933445
.

Raymond Camden, Adobe Developer Evangelist

Email : raymondcamden@gmail.com
Blog : www.raymondcamden.com
Twitter: cfjedimaster

@ghost ghost assigned JeffryBooher Aug 1, 2013
@JeffryBooher
Copy link
Contributor

so just a quick update on this @sathyamoorthi removed his JavaScriptCodeHints extension (you'll have to do this by removing the folder from your install as this is a built-in extension that extension manager cannot remove) and that fixed the performance issue.

We'll be looking at why this is a problem but most likely it is due to the deoptimization that @njx described above which will hopefully be fixed soon.

@ghost ghost assigned RaymondLim Aug 2, 2013
@njx
Copy link

njx commented Aug 2, 2013

To @RaymondLim to work with the reporters to figure out:

  • Is this happening in all JS files, or just JS in script blocks as in the video?
  • Does it happen only during indexing, or does it seem to happen all the time?
  • If you just keep typing through the code hint popup, does it actually stutter, or is it just that the popup takes awhile to come up if you're waiting for it but it doesn't slow down typing?

@RaymondLim
Copy link
Contributor

@sathyamoorthi and @cfjedimaster

Can you answer the questions that @njx listed above? None of us here can reproduce the issue recorded in the video. So we are wondering the issue is only in the JavaScript blocks of an HTML file or can you also reproduce it in any .js file. If you can reproduce it in a .js file, can you reproduce it all the time as opposed to the time you just started Brackets or just switched to a different project folder?

@sathyamoorthi
Copy link
Contributor

@RaymondLim

_Is this happening in all JS files, or just JS in script blocks as in the video?_

Just noticed. Yes. It is not happening in all JS files. It is happening only in script blocks within HTML files. For a test, i opened and tried with entire brackets source, there i am not seeing any slow down.

_For other two_

Image

Hope. This may help you.

@RaymondLim
Copy link
Contributor

@sathyamoorthi
Thanks for your confirmation of slowness in HTML files only. But I still cannot reproduce it with the default "Getting Started" project file. I wonder it has to do with the specific js files in your project. So can you help us to narrow down the minimum context that is required to reproduce the issue? If you can reproduce the issue with just a single HTML file, then can you share your file (or just the JavaScript blocks of the file ) with us?

@cfjedimaster
Copy link
Contributor Author

I have only seen it in JS blocks inside HTML files - but not 100% of the
time. The file I'm looking at now seems sluggish versus last Sprint, but
not as bad as my first encounter with this bug.

On Mon, Aug 5, 2013 at 5:17 PM, Raymond Lim notifications@github.comwrote:

@sathyamoorthi https://github.com/sathyamoorthi

Thanks for your confirmation of slowness in HTML files only. But I still
cannot reproduce it with the default "Getting Started" project file. I
wonder it has to do with the specific js files in your project. So can you
help us to narrow down the minimum context that is required to reproduce
the issue? If you can reproduce the issue with just a single HTML file,
then can you share your file (or just the JavaScript blocks of the file )
with us?


Reply to this email directly or view it on GitHubhttps://github.com//issues/4608#issuecomment-22145159
.

Raymond Camden, Adobe Developer Evangelist

Email : raymondcamden@gmail.com
Blog : www.raymondcamden.com
Twitter: cfjedimaster

@sathyamoorthi
Copy link
Contributor

@RaymondLim I have included bootstrap, jquery, fullcalendarjs and bootstrap-context-menu into my project.

I have included external source files like this.

<link href='../lib/client/fullcalendar/fullcalendar.css' rel='stylesheet' />
<link href="../lib/client/bootstrap/css/bootstrap.min.css" rel="stylesheet" media="screen">

<script src='../lib/client/jquery-1.9.1.min.js'></script>
<script src='../lib/client/jquery-ui-1.10.3.custom.min.js'></script>
<script src='../lib/client/fullcalendar/fullcalendar.min.js'></script>
<script src="../lib/client/bootstrap/js/bootstrap.js"></script>
<script src="../lib/client/bootstrap/js/bootstrap-contextmenu.js"></script>

apart from this, my html file is having only few lines of code.

@RaymondLim
Copy link
Contributor

Thanks @cfjedimaster and @sathyamoorthi

So the issue seems to be related to the number of JS files in your project and/or probably also depends on the specific JS files you have.

@sathyamoorthi

Since you still can reproduce it, can you try to use Timeline in Chrome Developer Tools to record your typing? Maybe we can inspect the recorded timeline to find out what is causing the lag.

@sathyamoorthi
Copy link
Contributor

@RaymondLim Recorded timeline. download link. Big file actually !!!!

I recorded another one, which has less data and more meaningful infromation.

This seems suspicious to me.

Image

@RaymondLim
Copy link
Contributor

@sathyamoorthi
Nice recording! I reviewed the second one and noticed that there are a bunch of wide gaps between GC Events. (You can see it for yourself by clicking on Frames and extending the range at the top line.) I believe those wide gaps between GC events indicate that CPU is occupied by the worker thread during those intervals. So we can confirm it by also recording Timeline for the worker thread. You can do it by clicking on the link available under Workers pane in Sources tab.

@sathyamoorthi
Copy link
Contributor

@RaymondLim

I recorded tern worker. Once i do timeline record for worker and stop, brackets crashes. So i didn't do much recording. I am not great dude with chrome dev tools. So, with that, some of my analysis for your consideration.

According to me, if you load and see my recorded 'Tern Worker' timeline data, for each event, it is taking less than 20ms. So i don't suspect this for slowness.

As i have shown you in my previous comment with this timeline record, there are lot of "Recalculate styles" and "Layouts" in frames. Below is another screen shot, which shows them.

Image

I am matching this situation with Chrome DEV Tools Time Line Demo.

I suspect hinting popup. Because it needs to calculate lot of things to position into view-port. Somewhere, that should trigger forced layout. There is huge possibility to hit any one of this property as listed in this post.

May be, you may have different story.

@dangoor
Copy link
Contributor

dangoor commented Aug 8, 2013

I'll just mention here that early in sprint 30, we'll be integrating an updated CEF and undoing the parsing workaround. Given @sathyamoorthi's recent comment, though, removing that workaround may not actually fix this problem.

@dangoor
Copy link
Contributor

dangoor commented Aug 8, 2013

We're at the end of the sprint, so I'm moving the milestone to sprint 30 to continue the investigation.

Given the nature of this problem (not consistently reproducible and only in certain contexts), I'm setting this as a medium priority issue.

@sathyamoorthi
Copy link
Contributor

I recorded another one using sprint 29. If you notice, I used javascript file not <script>. But i reported slow down happens only inside <script>. But i can sense slow down in js file also. If you notice that video, when i type text inside js file ~25% CPU consumed by brackets.exe. When i switch to php file and type, it is only 4%.

@dangoor
Copy link
Contributor

dangoor commented Aug 16, 2013

@sathyamoorthi I would expect CPU usage for the code hinter, but that should be on a background thread and not in the path that slows down typing.

In Sprint 30, we have an updated shell and have rolled back my workaround for the earlier v8 crash. It will be worth seeing if things are still slow for you or if there has been an improvement.

@sathyamoorthi
Copy link
Contributor

@dangoor Ok. let we check with sprint 30.

@RaymondLim
Copy link
Contributor

@sathyamoorthi
Do you have a chance to test with sprint 30? If not yet, you can get it from https://github.com/adobe/brackets/releases/tag/0.30.0-8785.

Let us know whether you still see the issue with sprint 30.

@sathyamoorthi
Copy link
Contributor

@RaymondLim
I recorded video with sprint 30. I feel the same.

@njx
Copy link

njx commented Aug 29, 2013

Nominating for sprint 31.

@ghost ghost assigned redmunds Sep 10, 2013
@redmunds
Copy link
Contributor

A lag seems to have been introduced in CodeHintList._calcHintListLocation(). In Sprint 27 it takes ~7ms. In the current code (Sprint 31) it's taking ~49ms.

UPDATE: I think my previous observation is a red herring. Looking further up the profile call tree, CodeHintManager.handleChange() takes ~101ms in Sprint 27 and ~99ms in Sprint 31. The difference must be that code is called in a slight different order, and the first code to get the size of hint window (which causes caches to get updated) is now in a different place (i.e. basically same code being called in a different order).

@redmunds
Copy link
Contributor

@sathyamoorthi Thanks for the in-depth analysis! I took a look at the video you posted Aug. 21 (20 days ago) and compared those scenarios to lastest code in master on Windows 7 and MacOS 10.8.

In Scenario 1, for the "slow" case you use objects "brackets" and "windows". These are "generic" objects. Notice how all of the items displayed in the list are in italics. Brackets doesn't know anything about these objects, so it displays a fairly long list of common strings that are sometimes helpful. You will get same results as with typing something random like "xyz".

For the "fast" case you use "ProjectManager". This is a known object, so it displays a list of known (text is not italicized) methods and properties. This displays faster because the list is generally much shorter.

Note that for the "slow" case, you used "windows" (with an 's') and not "window", which is a known object. Remove the 's' and list is displayed faster.

I think it's a bug that "brackets" is considered a generic object. We're currently looking at that.

In scenario 2, you are typing random objects, so it falls into the slower, generic case.

We could take a look at making generic objects displaying their list faster, but I am not seeing any noticeable difference between Sprint 27 and the latest code. Also, seems like typing should not be affected when display a "slow to build" list, so maybe there's something we could do about that.

One other thing that I thought that could be causing a difference is that we now use a transition when opening the hint list so it is smoother. But, this was introduced in Sprint 30 and is only 67ms, so it doesn't seem like that could be the difference.

Did I miss any cases?

@redmunds
Copy link
Contributor

@cfjedimaster I also ran your original case (tying "var" at start of <script> block) through the Dev Tools Profiler on Windows 7 on both Sprint 27 and latest master and could find no difference. Let me know if you're still seeing a difference. If so, what is you OS?

@njx
Copy link

njx commented Sep 10, 2013

@redmunds - did you take a look to see if the performance was any different between <script> blocks and standalone js files? It might be that it didn't get any worse between Sprint 27 and later sprints, but that it was always slower in <script> blocks.

@cfjedimaster
Copy link
Contributor Author

I'm not seeing any slow down - haven't for a while.

On Tue, Sep 10, 2013 at 4:23 PM, Narciso Jaramillo <notifications@github.com

wrote:

@redmunds https://github.com/redmunds - did you take a look to see if
the performance was any different between <script> blocks and standalone
js files? It might be that it didn't get any worse between Sprint 27 and
later sprints, but that it was always slower in <script> blocks.


Reply to this email directly or view it on GitHubhttps://github.com//issues/4608#issuecomment-24195734
.

Raymond Camden, Adobe Developer Evangelist

Email : raymondcamden@gmail.com
Blog : www.raymondcamden.com
Twitter: cfjedimaster

@redmunds
Copy link
Contributor

@njx I compared <script> blocks vs .js files, for first hint and subsequent hint, between Sprint 27 and Sprint 31.

Executive Summary:

  • first hint of <script> blocks slowed down significantly (from 678ms to 1544ms) between Sprint 27 and Sprint 31
  • first hint of <script> block are much slower than first hint of .js file. Even worse, every time you switch to a different file and then switch back to <script> block, speed returns to first hint times !
  • subsequent hint for <script> blocks is probably faster than .js file because block is much smaller than .js file

Details:

Measurements are for CodeHintList.open()

Sprint 27

External .js file

  • first hint: 123ms
  • subsequent hint: 110ms

HTML <script> block

  • first hint: 678ms
  • subsequent hint: 84ms

Sprint 31

External .js file

  • first hint: 166ms
  • subsequent hint: 161ms

HTML <script> block

  • first hint: 1544ms
  • subsequent hint: 117ms

@sathyamoorthi
Copy link
Contributor

@redmunds Thanks for checking them. I have a thought. Why should popup show long list all the time? I feel this as major problem. Can't we limit it to top 10 or 20. So when user types again and again, it will have always top 20. Something like incremental search.

@redmunds
Copy link
Contributor

@sathyamoorthi In general, I agree, but I'm not exactly sure about the strategy for populating that list. The other issue is that building the list should be done in the background (i.e.not freeze typing).

@redmunds
Copy link
Contributor

The extra time is with parsing page using CodeMirror tokenizer to extract JS from page. I'm not sure why it's taking longer: maybe added functionality to tokenizer? Is it worth trying to isolate that?

A possible solution would be to extract JS using our own regexps to avoid CM tokenizer.

The more important issue seems to be that info is not cached, so every time you leave page and come back you see "initial loading" hit again.

@njx
Copy link

njx commented Sep 16, 2013

@redmunds - @iwehrman had exactly the same thought (he also did a similar thing to what HTMLUtils.findBlocks() is doing when he implemented his hinter and had exactly the same problem) - regexps might be a faster solution. It's a little tricky because you have to make sure the <script> you find is actually valid (not in a comment, is a JS script, etc.). So maybe a solution would be to combine the two: use a regexp to find all the instances of the string <script>, then use CM to look up the state in the current editor at that location to make sure it's valid. Looking up the state in the current editor should be fast since CM caches the tokenization state.

It sounds like findBlocks() is called by the CSS hinter too...so we should decide whether we want to fix both, or just create a separate one for script blocks.

Keeping open to this sprint for now.

@redmunds
Copy link
Contributor

Moved to Sprint 32.

@redmunds
Copy link
Contributor

Using git bisect I isolated the problem to this commit: update CodeMirror submodule SHA in Sprint 29. Note that there was another update CM submodule the same day.

@njx
Copy link

njx commented Sep 26, 2013

Cool. That suggests you could bisect within the CodeMirror submodule between that SHA and whatever the previous submodule SHA was in order to figure out which CM commit was the culprit.

@redmunds
Copy link
Contributor

Problem was introduced with this CodeMirror commit: codemirror/codemirror5@ba21985. I opened an issue in CodeMirror repo to ask Marijn about this.

@redmunds
Copy link
Contributor

redmunds commented Oct 2, 2013

Removed CodeMirror tag as the issue was fixed with getTokenAt() precise parameter.

@redmunds
Copy link
Contributor

redmunds commented Oct 3, 2013

I am closing this issue. There were quite a few things discussed in this issue, changes with #5395 include:

  • Initial JS (and possible CSS) hint list in HTML page <script> (or <style>) block should be faster on large/complicated pages
  • I have not quantified it, but typing in JS should be improved (hopefully enough to notice).

Let us know if you notice any difference, good or bad.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants