Skip to content
This repository was archived by the owner on Sep 21, 2019. It is now read-only.

Conversation

svalaskevicius
Copy link

@svalaskevicius svalaskevicius commented Jun 6, 2018

the timeouts for polling add up to enormous waiting times, especially on pure FP files where ensime sends a lot of ScalaNote messages for libraries it parses.

this pr is not finished. please excuse the commented blocks left here etc, this is rather an attempt (successful it seems!) to make it faster by using asyncio instead of periodic polls..

any comments are welcome, before I try to clean it up and make it backwards compatible (as asyncio only exists from python 3.4 apparently, and python 3.5 added some syntax used here)..

also.. while this pr increased my python-fu considerably, it's still rather basic.. any help from ppl who want to join would be appreciated greatly!

oh and also - this pr also adds a dependency on https://websockets.readthedocs.io/en/stable/intro.html

with catch(Exception, self.log.error):
if timeout != 0 or not self.queue.empty():
if timeout == 0:
timeout = 0.01
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably reintroduce should_wait param... although... maybe another function might be better for that?

self.ws = await websockets.client.connect(self.ensime_server, **options)
True
gotws = asyncio.run_coroutine_threadsafe(connect(), self.ws_loop).result(10)
if gotws:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this gotws needed at all? if future fails it will throw exception, so should be ok w/o it and the following line shifted right into the catch block

# Queue for messages received from the ensime server.
self.queue = Queue()
# if sys.version_info >= (3, 5):
self.ws_loop = None
Copy link
Author

@svalaskevicius svalaskevicius Jun 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably move to a new class, that handles the other thread and exposes thread safe methods to send via ws and get from queue

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added benefit - diff classes can be used for diff python versions

@ches
Copy link
Contributor

ches commented Jun 11, 2018

Hi @svalaskevicius, thanks for working on this! I made an attempt at converting to asyncio awhile back (partly in a separate project that I've never gotten as far as PR for, I was going to make a pip-installable Python ENSIME client that ensime-vim + the Sublime plugin could use as a dependency potentially), but I wrestled with trying to keep Python 2 support through backport implementations.

It's more than a year later now and I think it's probably beyond the point of being worth trying to keep Python 2 compat now, so I'd be happy to see this move forward. The Trollius project is basically dead, and I think it's the norm now for Vim to build with Python 3 support by default in most desktop-oriented distributions. I know it is the case for Homebrew's macvim package on OS X, I need to specify a with-python@2 build option now to keep plugins like ensime-vim working.

By the way, I was using the same websockets library that you chose as well.

I'll try to give this some review soon if you're ready for it, sorry if it takes some time, the maintainers haven't been able to spare a lot of attention to the project lately.

@svalaskevicius
Copy link
Author

Cool! Agreed re Python 2. What about python <3.5? Do you know if it's used widely or can we use this async syntax I've used here?

As I mentioned, any review comments are welcome even if it's not finished - would help to guide the design thinking (I've added a few of my own :))

@svalaskevicius
Copy link
Author

Also, re time investment - I'm not sure how much time I'll have either, so no pressure. The basic idea for me was to get it to a usable state, but I'm still to try the LSP approach :)

@svalaskevicius
Copy link
Author

hmm, removed ticker too in svalaskevicius/ensime-vim@8387c47 - seems like it's not really required, and this way ensime feels much faster. as fast is it probably can be..

is ticker useful for anything else than lazy_display_errors? if that switched to quickfix list (separate pr discussion, see #415), it could be removed altogether... it seems - still testing/experimenting :)

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

Successfully merging this pull request may close these issues.

2 participants