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.

Sign up for GitHub

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

Jump to bottom

[CLOSED] Navigating between large, code folded files causes performance problems #11255

Open
core-ai-bot opened this issue Aug 30, 2021 · 12 comments
Open

[CLOSED] Navigating between large, code folded files causes performance problems #11255

core-ai-bot opened this issue Aug 30, 2021 · 12 comments

Comments

@core-ai-bot
Copy link
Member

Issue by lkcampbell
Friday Apr 07, 2017 at 13:21 GMT
Originally opened as adobe/brackets#13282

OS: MacS 10.12.3 Sierra and Windows 10

Brackets Version: Release 1.9 build 1.9.0-17312 (release 189f6d39a)

Repro Steps:

  1. Make sure code-folding.enabled preference is set to true.
  2. Make sure code-folding.saveFoldStates preference is set to true.
  3. Set code-folding.maxFoldLevel preference to a high number like 100. Collapse all should really be collapsing all folds.
  4. Create two decent sized xml test files. I'm using this file for both of my test files.
  5. Open both files so they are in the Working Files section of the Project Sidebar.
  6. Collapse All both files so there is plenty of fold state data.
  7. Navigate back and forth between the two files continuously, showing one, then the other, in the editor.

Observed Results:
After three or four loops of navigating to each file, the time it takes for the contents of each file to show up in the editor will get longer and longer, roughly doubling in time for each loop. Eventually, the delay is so long the files can no longer be navigated in a reasonable amount of time.

Expected Results:
The time to navigate to each test file should be about the same regardless of how many time you loop between them.

Workaround:
Make sure code-folding.saveFoldStates preference is set to false. You will need PR adobe/brackets#13269 before you can use this workaround.

Other Notes:
The problem occurs with all extensions disabled.

Since this problem goes away when fold states are turned off, and the time seems to roughly be doubling each time, maybe new fold states are being appended to old fold states instead of replacing them. Feels like there might be some redundant, duplicate data being saved somewhere, just a theory, haven't tested it.
@core-ai-bot
Copy link
Member Author

Comment by lkcampbell
Sunday Apr 09, 2017 at 18:41 GMT

Pinging@thehogfather, wanted to make sure you saw this issue, Patrick.

@core-ai-bot
Copy link
Member Author

Comment by thehogfather
Sunday Apr 09, 2017 at 18:44 GMT

@lkcampbell got it 👍 .

@core-ai-bot
Copy link
Member Author

Comment by lkcampbell
Sunday Apr 09, 2017 at 18:49 GMT

@thehogfather let me know if you need help. I am curious if my theory is correct. Either way, let me know.

@core-ai-bot
Copy link
Member Author

Comment by thehogfather
Sunday Apr 09, 2017 at 21:39 GMT

@lkcampbell your theory is in the right direction - but we could nail it down better. The problem is not actually with the routine for saving fold states. The issue occurs when we attempt to restore the saved fold states for the editor gaining focus.

Here is a quick overview of what I have found so far.
When the active editor changes, the fold states for the editor losing focus are saved while the fold states for the editor gaining focus are restored. Saving is not costly. Restoration however involves several calls to codeMirror functions for rendering fold marks on the gutter and creating folded marks in the document. We need to perform this operation at least once - although there is no need to restore the folds for a document already in the working set.

The reason why setting saveFoldStates: false gets rid of the problem is because there is nothing to restore when the saveFoldStates is set to false. Hence the restoreLineFold function has an early exit.
Mind you I am still not sure why calling the function to fold some code and generate the html markup becomes less and less performant. We might need to do some further profiling.

So a quick experiment to address this issue is to prevent the redundant call to restore line folds for files already in the working set. I am currently not aware of any API available in brackets to check if a file is in the working set, (but for now a workaround is to check if the editor object has editor._codeMirror._lineFolds). Do you know of an existing API that doe the same thing?

Here are a couple of charts from a profiling experiment

Buggy

screenshot_09_04_2017__21_59

Patched

Ignore the idle time at the start of this profile. It is clear that the first two calls to switching between the files are still slow - and we can try to figure out how to improve that still. But subsequent calls are close to instantaneous.
screenshot_09_04_2017__22_10

@core-ai-bot
Copy link
Member Author

Comment by lkcampbell
Tuesday Apr 11, 2017 at 23:30 GMT

@thehogfather that's an impressive improvement with your patch. I will probably have more time to look into this Friday and this weekend. I will see if I can find an answer to your Brackets API question then as well.

@core-ai-bot
Copy link
Member Author

Comment by lkcampbell
Sunday Apr 16, 2017 at 12:25 GMT

@thehogfather, I think MainViewManager.findInWorkingSet() is the Brackets API you want to use.

@core-ai-bot
Copy link
Member Author

Comment by thehogfather
Sunday Apr 16, 2017 at 13:10 GMT

oh awesome I'll have a play with that.

@core-ai-bot
Copy link
Member Author

Comment by thehogfather
Sunday Apr 16, 2017 at 23:27 GMT

@lkcampbell After tinkering with MainViewManager.findInAllWorkingSets I decided it was not quite right for this use case. For instance when Brackets is opened, it may already contain a list of files in the working set from a prior session (even before any user interaction). Hence presence in the working set does not necessarily mean the fold gutters and states have been initialised/restored for a given editor. See 5ae77a4 for details of my fix if you would like to test.

@core-ai-bot
Copy link
Member Author

Comment by lkcampbell
Tuesday Apr 18, 2017 at 10:55 GMT

@thehogfather, okay, it looked like the right API from the documentation. There has to be a way to tell if a file is in the current working set somewhere, but let's not let it block getting this bug fixed. I will take a look at your fix soon, thanks.

@core-ai-bot
Copy link
Member Author

Comment by thehogfather
Tuesday Apr 18, 2017 at 12:57 GMT

@lkcampbell the initial fault in framing the problem is mine as I assumed that the presence of a file in the working set implies that the Editor fold gutters would have been initialised. This is not always the case. Specifically when Brackets is closed and re-launched, files are added to the working set based on settings saved from the previous session. I believe this is largely because code folding is still loaded as an extension and not initialised as part of the Editor constructor. I think this is fine since initialising code folding could be expensive and it is best loaded on demand.

So to be clear, the API you suggested does exactly what it says on the tin - unfortunately that functionality is not what we need to solve the problem here.

@core-ai-bot
Copy link
Member Author

Comment by lkcampbell
Tuesday Apr 25, 2017 at 13:16 GMT

@thehogfather, your change already got merged before I had a chance to look at it :). I will test it later today and update this issue appropriately.

@core-ai-bot
Copy link
Member Author

Comment by lkcampbell
Wednesday Apr 26, 2017 at 00:20 GMT

Closing as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@core-ai-bot