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.

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

Use the new Async Server to replace analysis threads #3055

Closed
blink1073 opened this issue Mar 12, 2016 · 20 comments
Closed

Use the new Async Server to replace analysis threads #3055

blink1073 opened this issue Mar 12, 2016 · 20 comments

Comments

@blink1073
Copy link
Contributor

Builds on #3033 and #3049.

@blink1073
Copy link
Contributor Author

I'm concerned about having an unbounded number of processes running, more opportunities for zombie processes. What if we only created one other process for all async tasks, saved that pid, and explicitly kill it at the next startup?

That other process would have a pool of QThread workers. Then we have worst case one zombie process, that gets cleaned up at startup. We'd also give priority to the libs in the target environment, but provide the ones in the root environment as fallback.

@goanpeca, @ccordoba12, @Nodd, thoughts?

@ccordoba12
Copy link
Member

I'm +1 for that. Please go ahead with one server only :-)

@goanpeca
Copy link
Member

Sound good stevo

@blink1073
Copy link
Contributor Author

I think we need two servers: one that is running in a target environment, and one that is running in the current environment. The current environment would be used for things that don't rely on the target environment, removing the need for complicated path overrides in the target environment.

@blink1073
Copy link
Contributor Author

Also, if possible, we should avoid using Qt in the target environment, since that is a huge can of worms.

@goanpeca
Copy link
Member

we should avoid using Qt in the target environment meaning?

@blink1073
Copy link
Contributor Author

We shouldn't require it to be installed in the target project environment, and I'm not sure if we can "patch" it in, especially across python versions.

@goanpeca
Copy link
Member

Ahh ok.

I never thought we needed to do that... the idea is to have a root env for spyder and spyder plugins and a spyder infrastructure to be able to call python modules that are specific to python versions (pep, jedi, rope, ipython backend etc...). There is no Qt python stuff that we would need to have in place in the specific environment.

@blink1073
Copy link
Contributor Author

These are actually two new types of plugins: async_root, and async_env, and I think should be treated similarly to other plugins.

@blink1073
Copy link
Contributor Author

So, when we start the AsyncRoot server, it provides those plugins, and when we start the AsyncEnv server, it provides those plugins.

@blink1073
Copy link
Contributor Author

We write the two pids to spyder.lock, and clear those pids at startup.

@blink1073
Copy link
Contributor Author

  • Candidates for root: widgets/findinfiles/SearchThread, widgets/pydocgui/PydocServer, plugins/help/SphinxThread, workers/updates/WorkerUpdates, widgets/externalshell/monitor/Monitor, widgets/externalshell/introspection/NotificationThread, widgets/editor/AnalysisThread
  • Candidates for env: utils/introspection/*

@goanpeca
Copy link
Member

👍

@ccordoba12
Copy link
Member

My thoughts:

  • I would like to see widgets/editor/AnalysisThread as part of the env server, not the root one.
  • Things related to our Python consoles monitor should be left as they are. They are tricky and hard to debug. Besides, I'm planning to deprecate our Python console in the future (precisely because of this).
  • I don't see the need to touch SearchThread, PydocServer, SphinxThread and WorkerUpdates. IMHO they are fine as Qt threads :-)

@Nodd
Copy link
Contributor

Nodd commented Mar 23, 2016

@ccordoba12 By deprecating our Python console, you mean keeping ipython only ? If that's the case I'm strongly against it, I run my scripts in a new python interpreter almost exclusively.

@ccordoba12
Copy link
Member

@Nodd, I mean that I'm planning to move our Python console to be a third-party plugin, so others can help me to maintain it (if they want to keep using it) because having two consoles is too much of a burden :-)

@blink1073
Copy link
Contributor Author

I don't see myself having time to implement this for 3.0beta4.

@ccordoba12 ccordoba12 modified the milestones: v3.0beta4, v3.1 Apr 25, 2016
@blink1073 blink1073 removed their assignment Mar 11, 2017
@blink1073
Copy link
Contributor Author

Here is the API sketch I made:

class AsyncService:

    """The base async service that runs in the current env"""

    def __init__(self, executable=None, env=None):
        # set up the zmq socket
        pass

    def setup(self):
        """Set up the QProcess - this is intended to be overridden"""
        # discover base plugins, pass to the qprocess

    def start(self):
        """Start the QProcess"""
        # Set up the socket notifier and heartbeat

    def request(self, plugin, func_name, *args, **kwargs):
        """Send a request to the provider, returning the request id"""


class AsyncEnvService(AsyncService):

    def setup(self):
        pass
        # discover env plugins, add to PYTHONPATH, set up qprocess


class AsyncManager:

    def __init__(self):
        # gets the port and the list of plugins to load from argv
        pass


class AsyncPlugin:
    pass


class AsyncEnvPlugin:

    """A plugin that should run in a target environment."""
    libs = []  # Libs to load in the target environment

@ccordoba12
Copy link
Member

Thanks @blink1073!

@ccordoba12 ccordoba12 modified the milestones: v4.0beta2, todo Jun 25, 2017
@ccordoba12 ccordoba12 removed this from the v4.0beta2 milestone Aug 13, 2017
@ccordoba12
Copy link
Member

This is superseded by issue #4742.

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

No branches or pull requests

4 participants