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

[WIP/RFC] Only use Jedi, without worker/client/server/cache #184

Closed
wants to merge 10 commits into from

Conversation

blueyed
Copy link
Collaborator

@blueyed blueyed commented Jul 8, 2018

I have tried to implement restarting in case
g:deoplete#sources#jedi#python_path gets changed, and figured that it
is way easier when using only Jedi.

There are likely previous features missing, and it needs cleaning up /
deleting still.

It uses the Jedi branch to improve handling of Environments:
davidhalter/jedi#1108

TODO:

Works quite well already, tested with:

import numpy

numpy.
2018-07-08 23:25:51,485 DEBUG    [14486] (deoplete.jedi) Line: 3, Col: 6, Filename: '…/vim/plugged/jedi-vim/pythonx/jedi/t.py'
2018-07-08 23:25:52,442 INFO     [14486] (deoplete.jedi) get_completions t = 956.456334ms, µ = 0, σ = 0)
2018-07-08 23:25:53,194 INFO     [14486] (deoplete.jedi) massage_completions t = 752.618779ms, µ = 0, σ = 0)
2018-07-08 23:25:53,195 INFO     [14486] (deoplete.jedi) gather_candidates t = 1712.006350ms, µ = 0, σ = 0)
2018-07-08 23:26:01,394 DEBUG    [14485] (deoplete.core) on_event: InsertEnter
2018-07-08 23:26:03,061 DEBUG    [14485] (deoplete.core) completion_begin: 'numpy.' (TextChangedI)
2018-07-08 23:26:12,425 DEBUG    [14485] (deoplete.core) on_event: InsertEnter
2018-07-08 23:26:12,944 DEBUG    [14485] (deoplete.core) completion_begin: '' (TextChangedI)
2018-07-08 23:26:13,003 DEBUG    [14485] (deoplete.core) completion_begin: 'n' (Async)
2018-07-08 23:26:14,475 DEBUG    [14485] (deoplete.core) completion_begin: 'numpy.' (TextChangedI)
2018-07-08 23:26:14,483 DEBUG    [14486] (deoplete.jedi) Line: 5, Col: 6, Filename: '…/vim/plugged/jedi-vim/pythonx/jedi/t.py'
2018-07-08 23:26:14,534 DEBUG    [14485] (deoplete.core) completion_begin: 'numpy.' (Async)
2018-07-08 23:26:14,702 INFO     [14486] (deoplete.jedi) get_completions t = 219.013144ms, µ = 0, σ = 0)
2018-07-08 23:26:15,270 INFO     [14486] (deoplete.jedi) massage_completions t = 567.887416ms, µ = 0, σ = 0)
2018-07-08 23:26:15,270 INFO     [14486] (deoplete.jedi) gather_candidates t = 791.059430ms, µ = 0, σ = 0)

So massage_completions takes a lot of the time by itself.

It might make certainly sense to add some caching on top, but it shouldn't be so complicated anymore.

/cc @tweekmonster

@blueyed blueyed mentioned this pull request Jul 8, 2018
# Hard coded python interpreter location
self.python_path = vars.get(
'deoplete#sources#jedi#python_path', '')
# TODO
self.extra_path = vars.get(
'deoplete#sources#jedi#extra_path', [])
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Jedi has jedi.settings.additional_dynamic_modules (used in jedi-vim).

@Shougo
Copy link
Collaborator

Shougo commented Jul 8, 2018

I think it is better version.
Because, deoplete is already parallel.

@blueyed
Copy link
Collaborator Author

blueyed commented Jul 8, 2018

Yep, I like it also so far.

@Shougo
Copy link
Collaborator

Shougo commented Aug 13, 2018

@blueyed
I want to test the branch.
Please rebase it.

if comp.module_path not in cache and comp.line and comp.line > 1 \
and os.path.exists(comp.module_path):
with open(comp.module_path, 'r') as fp:
cache[comp.module_path] = fp.readlines()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I get the slow reason.
It is very slow. Because, deoplete-jedi loads all module files.
I think it should be removed.

It is faster and simpler.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, likely only needed for some old and hopefully fixed misbehavior with regard to @property decorators.

@Shougo
Copy link
Collaborator

Shougo commented Aug 13, 2018

So massage_completions takes a lot of the time by itself.

I get the reason. Please see the comment.
I think the cache feature is not needed.

blueyed added 10 commits August 14, 2018 04:11
I have tried to implement restarting in case
`g:deoplete#sources#jedi#python_path` gets changed, and figured that it
is way easier when using only Jedi.

There are likely previous features missing, and it needs cleaning up /
deleting still.

It uses the Jedi branch to improve handling of Environments:
davidhalter/jedi#1108
…06a0

    * rplugin/python3/deoplete/vendored/jedi f6bc166...d6306a0 (48):
      > With the recent changes one performance optimization got lost
      > FunctionExecutionContext should use the parent if possible
      > Use anonymous instance arguments in a different way
      > Move some anonymous instance function execution stuff
      > Get rid of InstanceFunctionExecution, because it's really not needed
      > Use the InstanceArguments for super as well
      > Use InstanceArguments directly and not via InstanceFunctionExecution
      > Remove old garbage code
      > Don't use arguments that are not needed
      > Also move the remaining get_params to get_executed_params
      > get_params -> get_executed_params where possible
      > Subprocess error reporting improvements
      > Fix a recursion issue about compiled objects
      > Use a CompiledInstanceNameFilter that wraps the class name as well
      > Prefer Python 3 import over 2
      > Now it's actually possible to specify a pytest environment for the same Python version
      > Note that Python 3.3 support was dropped in Changelog
      > Drop support for EOL Python 3.3 (#1019)
      > Rewrite the pyc test
      > Fix an issue with stderr debugging of subprocesses
      > stderr of the child processes should be printed in debug output
      > Use close_fds for posix.
      > Remove some redundant code
      > Use names of classes to infer names of instances
      > Don't have execute and execute_evaluated on name
      > Fix an issue where __ prefixed variables where not hidden when accessed from a class
      > Bound methods are now working correctly in all Python versions. Therefore a test was wrong.
      > Remove a print in tests
      > BoundMethods now have access to the function that they are using
      > Remove another usage of is_class where it's not needed
      > FunctionContext should be created from a unified interface
      > Don't create a FunctionExecutionContext if it's not used.
      > Use TreeContext in a good way
      > Fix broken link in documentation
      > The implicit namespace package test from 4b276bae87a3170672f7ddb3e00f5851fe24d562 can only be used for Python 3.4+
      > The import resolution for namespace packages was wrong
      > Add a way to use the interpreter environment for tests
      > MergedExecutedParams -> DynamicExecutedParams
      > Fix a recursion error, fixes #1173
      > Remove two recursion tests again that will belong into a commit at a point where it is not failing anymore
      > Don't merge params if it's just one param
      > Add a repr for AnonymousArguments
      > Fix a doctest
      > Some minor flake8 fixes
      > Script.__repr__: include environment
      > Add params to CallSignature.__repr__
      > Environments are now always created on request
      > Improve Environment
…f1582

    * rplugin/python3/deoplete/vendored/parso 7064ecf...96f1582 (1):
      > Update usage.rst
@blueyed
Copy link
Collaborator Author

blueyed commented Aug 14, 2018

@Shougo
You can pick it up, and either rebase it only locally first, or also push here.

But "Allow edits from maintainers." might not be enough.. :/

Ping @zchee.

Rebased it now, updated the submodules, but no other changes.

@blueyed
Copy link
Collaborator Author

blueyed commented Aug 14, 2018

@Shougo
Maybe you want to create a new PR then - I won't have time for it currently.

@Shougo
Copy link
Collaborator

Shougo commented Aug 16, 2018

Maybe you want to create a new PR then - I won't have time for it currently.

OK. I will do it.

@Shougo
Copy link
Collaborator

Shougo commented Aug 18, 2018

I have created #192.

@blueyed
Copy link
Collaborator Author

blueyed commented Sep 13, 2018

Closing in favor of #192.

@blueyed blueyed closed this Sep 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants