Closed ameyp closed 11 years ago
Added a thread, but ST2 hangs soon after displaying the lookup results. No idea why.
I'm just beginning to learn about ST2 plugins, so you may have already seen this, but http://net.tutsplus.com/tutorials/python-tutorials/how-to-create-a-sublime-text-2-plugin/ walks through creating a plugin with threads. HTH?
Yep, I saw that too, but my use case is nowhere near complicated enough. That tutorial creates a class that extends the Thread class, and then creates a bunch of threads and polls them to see which ones are done. I'm just passing a function to an instance of Thread as the "target", and that function will get called when I call "start" on the thread object.
http://docs.python.org/2/library/threading.html#thread-objects
I think the problem is that it is unsafe to call any SublimeText API other than set_timeout from threads, so you have to use a worker thread to perform the search and then check on that thread from the main CscopeSublime class (non-thread) and update the view only from the non-thread class when the thread worker is done. Sorry if I'm telling you what you already know. =:) FWIW, I believe the SublimeLinter or sublimelint plugins do this via threads too?
I didn't know this, but I was afraid that it might be this. Damnit :(
Ok, doesn't seem to be hanging anymore with commit https://github.com/ameyp/CscopeSublime/commit/242aba34bc6a6e277000c10f0ad50a403bf44e9d, but I can't be sure since I'm on a different machine. Care to try it out on your 280 MB database?
Definitely, will do that later this morning! =:)
Cool, thanks :)
Looks good, Amey! This definitely looks better! I have one suggestion though.... I think it's a nice touch how the Package Control plugin gives you kind of an activity indicator in the status bar. The plugin tutorial at http://net.tutsplus.com/tutorials/python-tutorials/how-to-create-a-sublime-text-2-plugin/ has that concept too. To quote, "it is a nice user interface enhancement to provide an activity indicator to show that our plugin is still running."
The code looks pretty straightforward:
if len(threads):
# This animates a little activity indicator in the status area
before = i % 8
after = (7) - before
if not after:
dir = -1
if not before:
dir = 1
i += dir
self.view.set_status('prefixr', 'Prefixr [%s=%s]' % \
(' ' * before, ' ' * after))
sublime.set_timeout(lambda: self.handle_threads(edit, threads,
braces, offset, i, dir), 100)
return
If you don't feel like adding the activity indicator bit, I can take that on after you merge your work into master if you don't want to mess with it right now. =:)
So, yeah, I'm testing from your threading branch and it looks like the plugin is still functional (yay!), it doesn't freeze anymore (yay!), and it gives a status message that tells me that it's working (yay!).
It was hard to tell how big the changes were to introduce threading. How bad was it?
Thanks Amey!!
I think it's a nice touch how the Package Control plugin gives you kind of an activity indicator in the status bar.
Oh yeah, I love how Package Control has that activity indicator. Thanks for the pointer and the snippet, I'll take it up myself, should be fairly interesting to get it working.
It was hard to tell how big the changes were to introduce threading. How bad was it?
It actually wasn't as bad as I initially thought it would be. I just had to move a couple of functions to another class, convert sublime.
calls to arguments passed to that class's constructor, and call sublime.set_timeout
with a function pointer. That ensures that the passed function gets called in ST2's main thread. Thankfully the existing functions were already pretty modular, so separating the ST2-specific functionality from cscope-specific functionality was relatively painless. The whole thing only took about half an hour :)
Hey Amey, how's it going on the threading work? I was going to start in on the search confirmation popup but I want to wait until the threading code is in first. Need any help with the threading changes? I could pick up what you have on the threading branch and keep going if you like?
I was looking at your changes on the threading branch and I had a thought. It looked like you moved the results parsing into the worker thread. I was thinking it might be simpler to make a smaller change so that the only thing the worker thread did was to kick off the cscope shell process and build a temp buffer with the results from cscope. Then the rest of the code could remain where it was. The main class would check to see if a worker thread already existed and if it did, do nothing. It would then kick off the worker thread search and go into a callback loop, monitoring the worker thread and giving activity feedback in the status bar. As soon as the main class sees that the worker thread is done, it would retrieve the raw cscope results from the worker thread's buffer and go into the same logic as is on master today for parsing the results and displaying them. Maybe I'm telling you what you already know and are already doing? What do you think? I just think the worker thread should only be responsible for talking to the cscope shell process and buffering its output.
To start off, that's all the worker thread was doing. But then I saw that if a large number of results are fetched, ST2 freezes for around 1.5 seconds before displaying the results. So I thought I'd move some more functionality to the worker thread, but it doesn't seem to be helping. I'll merge the initial commit and the activity indicator over to main and keep tinkering around with a more responsive UI in this branch.
Ok, I've cherry-picked some of the commits from threading and merged them to main. I've moved the results-parsing logic into the worker thread so that all cscope-related code is in one class, and all ST2-related stuff is in the CscopeCommand class (with the exception of the display_results
function in the Worker class, which I'll move to CscopeCommand soon). I believe it makes more sense this way; have one class that knows how to talk to Cscope and parse its output, and have another class that knows how to talk to ST2.
Moved display_results
over to the CscopeCommand
class as well with commit https://github.com/ameyp/CscopeSublime/commit/188095fb829ab1bf858843be7ce34894abe752a8, I guess I'm done with threading. Doesn't seem to be crashing or freezing anymore, please let me know if you have any issues.
Yeah, I think you're totally right. Now that I think about it, anything that's not dealing with drawing, rendering, etc., should be in the worker thread class. I'm hoping that the "results-parsing logic" has nothing to do with Sublime's view or UI code?
Awesome, thank you Amey! I'll take a look at this today. =:)
Nope, the result-parsing code is Sublime-independent as far as I can see.
Finished with commit https://github.com/ameyp/CscopeSublime/commit/188095fb829ab1bf858843be7ce34894abe752a8.
Instead of performing Cscope queries in the main thread (thereby causing the editor to freeze while cscope is returning the results), display a status message and run cscope in a separate thread, returning the results to the editor when that thread finishes.