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 clang lib directly with deoplete source for better performance #464

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

DarkDefender
Copy link

Before I implemented these changes, the clang_complete deoplete source would cause neovim to freeze while it were gathering completion results.

I think this is because previously the deoplete thread would call the vim script portion of clang_complete to get completion candidates. And then vim would freeze till that call was completed.

Because this kinda defeates the async nature of deoplete, I decided that I would try to move out the completion candidates search to the deoplete thread.

This code requires that https://pypi.python.org/pypi/libclang-py3 is installed (deoplete is python3)

I basically just copy pasted the python part of clang_complete and converted it to python3.

I'm not sure everything is working correctly though. But it seems to me like I get the same completion results as before at least.

The completion results are still generated quite slowly though. But now it doesn't cause neovim to freeze while it's going it :)

This could be improved further by eliminating the remaining "vim.eval" calls. I guess you can't eliminate all of them, but only calling them when absolutely need could speed things up further.

Even if this might not be merged I would really like some input and feedback on it.

@DarkDefender
Copy link
Author

Maybe @expipiplus1 would be interested in testing this out a bit?

@expipiplus1
Copy link

I've noticed this behavior with other plugins too such as ghc-mod, it would be great to see this fixed.
I'm not an expert in either code base, but this looks pretty reasonable to me.

@tony
Copy link
Contributor

tony commented Jan 8, 2016

@DarkDefender I'm trying out your branch and it works nicely.

Nice work. Why don't you finish the improvements you were suggesting to speed things up a bit more?

tony added a commit to tony/vim-config-framework that referenced this pull request Jan 8, 2016
tony added a commit to tony/vim-config-framework that referenced this pull request Jan 8, 2016
@DarkDefender
Copy link
Author

@tony thanks for using my branch. I'll try to improve it a bit more when I have time. That probably won't be till the end of january though

@expipiplus1
Copy link

@DarkDefender
For the most part this seems to be working really nicely.
I'm not able to complete symbols from other header files for some reason though.

To reproduce:

Create b.h containing

struct B{int bar;};

Create a.cpp containing

#include "b.h"
struct A{int arr;};
int foo(){
  A a;
  B b;
  b.<Complete here>

When the cursor reaches <Complete here> I call deoplete#mappings#manual_complete(). If the line reads a. I get appropriate completions suggested, if the line reads b. I do not.

With this setup I am able to complete the members of a but not of b. It is very strange, hopefully you can reproduce this and it's not me going mad.

It's probably worth noting that I get that behaviour also with system includes such as

@DarkDefender
Copy link
Author

@expipiplus1 Yes, I get that too. I didn't notice it before because it seems like deoplete completes the ".bar" for me anyways, but you are right that it doesn't work at first.

If you write "b.bar" then remove some of bar clang complete manages to find it... Same thing with the vector include

@xaizek
Copy link
Collaborator

xaizek commented Jan 18, 2016

@DarkDefender, the description is similar to the issue addressed by #468.

@expipiplus1
Copy link

In my fiddling I set min_pattern_length to 0, however this seemed to lead to the popup menu being open constantly, something quite annoying.

Although I'm unable to get completions for members of variables of type std::vector<int> I am able to get completion suggestions for members of variables of type std::array<int,1>. What's up with that!

I'm not able to get completions for bar without typing in at least part of the name of the member.

I do however get completion suggestions from the [M] provider for h (what's that?) and bar after I've typed it once. Does anyone know what the [M] provider is?

@DarkDefender
Copy link
Author

@expipiplus1 the "[M]" provider is from the built in deoplete sources. Check out the member.py file if you want to take a look at it.

@DarkDefender
Copy link
Author

@expipiplus1 My latest commit should fix it. I also created a check so it doesn't try to complete empty lines now.

BTW, is it save to assume that all empy lines begin with '\n'?

EDIT: Nvm changed the newline check to "isspace()" instead

@expipiplus1
Copy link

That works a little better for suggesting completions after typing a period, however completions are still suggested after typing other characters such as semicolon or parentheses.

It doesn't seem to do anything to fix the failed completion for std::vector<int> however (but I don't think that was the intention of those changes).

I have a feeling that this inconsistency is due to a bug further up in clang_complete or in clang itself.

@DarkDefender
Copy link
Author

Your vector example works for me now. So it should be fixed. I guess that you didn't forget to include it correctly?

library_path + "/", # Google
"/usr/lib64/clang", # x86_64 (openSUSE, Fedora)
"/usr/lib/clang"
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Brew installs llvm stuff into:

    LDFLAGS:  -L/usr/local/opt/llvm/lib
    CPPFLAGS: -I/usr/local/opt/llvm/include

Can we add that?

@tony
Copy link
Contributor

tony commented Jan 19, 2016

Can you rebase?

@tony
Copy link
Contributor

tony commented Jan 19, 2016

On OS X:

[deoplete] Loading libclang failed, completion won't be available. Are you sure '/Library/Developer/CommandLineTools/usr/lib' contains libclang?dlopen(libclang.so, 6): image not found. To provide a path to libclang use Config.set_library_path() or Config.set_library_file(

Re: Config.set_library_path() or Config.set_library_file(, is there a way to programmatically add paths to check for clang?

I seem to have the file there though, but OS X will do .dylib, not .so:

❯ ls -l /Library/Developer/CommandLineTools/usr/lib | grep clang
drwxr-xr-x  3 root  admin       102 Dec  9 17:01 clang
-rwxr-xr-x  1 root  admin  15632704 Nov 18 01:15 libclang.dylib

brew install clang as a .dylib on OS X to /usr/local/opt/llvm/lib.

❯ brew info llvm
llvm: stable 3.6.2 (bottled), HEAD [keg-only]
Next-gen compiler infrastructure
http://llvm.org/
/usr/local/Cellar/llvm/3.6.2 (1,891 files, 572M)
  Built from source with: --with-clang --with-clang-extra-tools --with-python --with-lldb --with-rtti --with-compiler-rt
From: https://github.com/Homebrew/homebrew/blob/master/Library/Formula/llvm.rb
==> Dependencies
Build: xz ✔, cmake ✔
8< 8< 8< Snip
LDFLAGS:  -L/usr/local/opt/llvm/lib
CPPFLAGS: -I/usr/local/opt/llvm/include

@DarkDefender
Copy link
Author

@tony You should be able to manually specify where the clang lib is with "g:clang_library_path" .

However I guess that the dylib loading problem is from the libclang-py3 lib and not my code. I see that in the libclang that clang_complete comes with it specifically looks for .dylib. But in libclang it is some automagic stuff instead:

        if name == 'Windows':
            filename = 'libclang.dll'
        else:
            # Does the right thing on Linux and MacOS X
            filename = ctypes.util.find_library ('clang')

            # On Ubuntu, find_library fails and returns None
            # this will break loading below so replace with libclang.so
            if filename is None:
                return 'libclang.so'

And this is what is does in the one bundled in clang_complete:

        if name == 'Darwin':
            file = 'libclang.dylib'
        elif name == 'Windows':
            file = 'libclang.dll'
        else:
            file = find_library("clang") or 'libclang.so'

Yes, I can rebase my code. But I guess it might be better to do that later when all quirks has been worked out. Or do you need me to rebase for any specific reason?

@expipiplus1
Copy link

Is this currently going anywhere? It would be a shame for it to become lost.

@DarkDefender
Copy link
Author

@expipiplus1 I'm using https://github.com/zchee/deoplete-clang instead.

@expipiplus1
Copy link

Gotcha, thanks @DarkDefender

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

Successfully merging this pull request may close these issues.

4 participants