jasonplatts / nova-todo

A Panic Nova extension for todo and fixme management.
https://extensions.panic.com/extensions/jasonplatts/jasonplatts.TODO/
MIT License
29 stars 7 forks source link

Performance Issues #22

Closed mikebronner closed 3 years ago

mikebronner commented 3 years ago

Hi, unfortunately I had to turn the TODO extension off, as it was causing Extension Service to crash. I don't have any actual replication steps, as it just happened when I opened my project. After disabling the extension, everything went back to normal.

Any suggestions on steps I can take to help identify the issue? Thanks!

jasonplatts commented 3 years ago

Hi Mike. Sorry to hear you are having issues with the extension. If you don't mind me asking, are you experiencing the problem with every project you open or only one? Does it happen every time you open the project? What languages does your project use? Is it a large project?

mikebronner commented 3 years ago

@jasonplatts Thanks for the quick reply. This seems to happen on my large PHP project with LOTS of composer and node_modules packages. I will add those two folders to be excluded and see if that helps. Let me know if there is anything else I can try, happy to help get down to the core of the issue. :)

jasonplatts commented 3 years ago

Thanks @mikebronner for the details. The extension ignores the node_modules and vendor directories by default, but it is certainly possible there is another directory and/or file in your project that is large enough to cause the extension to hang. If you are to exclude all of the directories in the project root, it might be possible to isolate the problem.

Amparose commented 3 years ago

Might be connected: The periodic flashing of the TODO panel (I'm guessing a refresh) is annoying.

jasonplatts commented 3 years ago

Hi @mikebronner. Are you still having problems with the extension hanging in your project?

Yes, @Amparose. The extension refreshes whenever a change is detected in the workspace. When a workspace isn't found polling is used. Ideally, only the changed file should be updated in the treeview. This is a performance update that should be made in a future version.

mikebronner commented 3 years ago

@jasonplatts I have not had it crash things yet. fingers crossed :)

Amparose commented 3 years ago

The extension refreshes whenever a change is detected in the workspace. When a workspace isn't found polling is used.

Is a “workspace” something different to opening a folder in Nova? Anyway, I’d like it if the polling could be turned off in prefs. There’s a refresh button and I’d prefer to manually do that that the visually flub of polling.

jasonplatts commented 3 years ago

Opening a folder in Nova is considered a workspace. Opening a file or files is not considered a workspace and will trigger polling. In most cases the refreshes are a result of Nova detecting a change in your workspace.

I'll consider this an enhancement issue. Thanks for your suggestions.

mikebronner commented 3 years ago

@jasonplatts I'm not sure exactly how Nova implements these things, but would it be possible to cache the results, and only update Nova after parsing is completed, so that the list does not appear to be so laggy? Just thinking out loud, not sure if this is a feasible idea.

jasonplatts commented 3 years ago

Great suggestion @mikebronner. That could certainly help. In the few minutes I've had to do some testing, I have noticed some new issues processing larger projects. These did not seem to occur in previous testing with the same projects. I will need to do some research.

The extension uses the egrep command in a process to find files containing the keywords within a workspace. It then searches through those files known to contain keywords to extract additional information. As I mentioned, the extension is inefficient in the way that it reruns this process whenever a change is detected in the workspace and updates the entire treeview, causing the flicker. I know this is not ideal. Caching could be helpful, but even just updating the individual treeview item rather than the entrire treeview would make a big difference. It's just a matter of finding the time to make the changes.

If anyone has the time and the interest to contribute, please feel free to make a PR.

mikebronner commented 3 years ago

Today I received the following error after waking my Mac up from sleep. After that it also prompted me to restart the Extension Service, so my extension console logs were wiped.

The extension “TODO” has stopped responding.
Screen Shot 2021-04-17 at 5 02 12 PM
jasonplatts commented 3 years ago

Thanks for the update @mikebronner. I will take a look.

gitscosh commented 3 years ago

Just came here looking to see if it was logged yet. I opened a rather large project today, and TODO crashed the extension service. Any ETA?

jasonplatts commented 3 years ago

Hi @gitscosh. Sorry you are having issues with the extension crashing. Unfortunately I haven't had the chance to go back and optimize the code yet.

Thanks for letting me know you are continuing to have issues. Since the last Nova update, I am not experiencing any crashes, which makes it difficult to know the extent of the problem. I will absolutely be looking to make performance improvements on the next release.

gitscosh commented 3 years ago

Thanks, @jasonplatts! I really like this addon and hope to be able to continue using it. On the positive side, most of the repos I open aren't nearly as large, so this isn't an "always" type of thing.

ghost commented 3 years ago

I'm working on a relatively large project, and with the plugin enabled, when I save I see well over 100 egrep processes are spawned (130 just now) 😬

jasonplatts commented 3 years ago

Thanks, @jasonplatts! I really like this addon and hope to be able to continue using it. On the positive side, most of the repos I open aren't nearly as large, so this isn't an "always" type of thing.

I appreciate that!

jasonplatts commented 3 years ago

I'm working on a relatively large project, and with the plugin enabled, when I save I see well over 100 egrep processes are spawned (130 just now) 😬

Thanks for letting me know @pa-aaron.

jasonplatts commented 3 years ago

This issue should be resolved with the release of version 3.0.