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

Do initial parsing in a background thread #242

Open
r4nt opened this issue Jan 18, 2013 · 23 comments
Open

Do initial parsing in a background thread #242

r4nt opened this issue Jan 18, 2013 · 23 comments

Comments

@r4nt
Copy link

r4nt commented Jan 18, 2013

I work at Google where we have an internal build system. We have a version of libclang that integrates with that build system, but it has (often) significantly higher runtimes than libclang on a small project.

It would make clang_complete much more useful to me, if it never blocked the main thread, even while opening files.

@xavierd
Copy link
Owner

xavierd commented Jan 19, 2013

Hi,

Unless I'm missing something, we do the initial parsing in a background thread :). However, quite recently, that background thread wasn't running due to a bug in vim. See: http://code.google.com/p/vim/issues/detail?can=2&start=0&num=100&q=&colspec=ID%20Type%20Status%20Priority%20Milestone%20Owner%20Summary&groupby=&sort=&id=103

Even without that, we shouldn't block the main thread and we're regularly polling vim to see if the current completion should be cancelled or not (it doesn't seems to work for me however…). Are you saying that you cannot cancel a completion and so you're forced to wait for it to finish?

@r4nt
Copy link
Author

r4nt commented Jan 19, 2013

So, to clarify: this is not about completions (completions work nicely in
the background thread for me)
This is when I load a file - it at least tries to load the command line
parameters via the libclang compilation database from the main thread, as
that blocks my complete vim every time I open a file.

Thanks,
/Manuel

On Sat, Jan 19, 2013 at 3:20 AM, Xavier Deguillard <notifications@github.com

wrote:

Hi,

Unless I'm missing something, we do the initial parsing in a background
thread :). However, quite recently, that background thread wasn't running
due to a bug in vim. See:
http://code.google.com/p/vim/issues/detail?can=2&start=0&num=100&q=&colspec=ID%20Type%20Status%20Priority%20Milestone%20Owner%20Summary&groupby=&sort=&id=103

Even without that, we shouldn't block the main thread and we're regularly
polling vim to see if the current completion should be cancelled or not (it
doesn't seems to work for me however…). Are you saying that you cannot
cancel a completion and so you're forced to wait for it to finish?


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-12449092.

@tobiasgrosser
Copy link
Collaborator

On 01/19/2013 09:47 AM, r4nt wrote:

So, to clarify: this is not about completions (completions work nicely in
the background thread for me)
This is when I load a file - it at least tries to load the command line
parameters via the libclang compilation database from the main thread, as
that blocks my complete vim every time I open a file.

Yes, the title of this bug report is misleading. As far as I understand
the problem is that within Google implementation the compilation
database is rather slow. For us getting parameters from the compilation
database has never been a performance issue, so it was not moved into a
background thread.

I don't see a problem accepting a patch that improves when using slow
compilation databases. Which call is the slow one for you?

CompilationDatabase.fromDirectory or
compilation_database.getCompileCommands()?

@r4nt
Copy link
Author

r4nt commented Jan 19, 2013

Ah, sorry for the confusion :) getCompileCommands is the slow call...
On Jan 19, 2013 12:31 PM, "Tobias Grosser" notifications@github.com wrote:

On 01/19/2013 09:47 AM, r4nt wrote:

So, to clarify: this is not about completions (completions work nicely in
the background thread for me)
This is when I load a file - it at least tries to load the command line
parameters via the libclang compilation database from the main thread, as
that blocks my complete vim every time I open a file.

Yes, the title of this bug report is misleading. As far as I understand
the problem is that within Google implementation the compilation
database is rather slow. For us getting parameters from the compilation
database has never been a performance issue, so it was not moved into a
background thread.

I don't see a problem accepting a patch that improves when using slow
compilation databases. Which call is the slow one for you?

CompilationDatabase.fromDirectory or
compilation_database.getCompileCommands()?


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-12453649.

@tobiasgrosser
Copy link
Collaborator

OK. I see. We should probably move the getCompileCommands call into the warmup thread. This requires some more changes as we can not call vim from within the thread, so we need to make sure the other parameters are properly propagated into and out of the thread.

@r4nt
Copy link
Author

r4nt commented Jan 21, 2013

Do you think there's a "quick-and-dirty" hack I can do to fix this up, or
will I run into a myriad of follow-up problems? Because solving this (if
only in a non-principled way) is in the way of rolling this out to a wider
audience here (10-20 seconds pauses when opening a file are unfortunately
not acceptable ;)

Thanks!
/Manuel

On Sat, Jan 19, 2013 at 1:52 PM, Tobias Grosser notifications@github.comwrote:

OK. I see. We should probably move the getCompileCommands call into the
warmup thread. This requires some more changes as we can not call vim from
within the thread, so we need to make sure the other parameters are
properly propagated into and out of the thread.


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-12454494.

@xavierd
Copy link
Owner

xavierd commented Jan 21, 2013

@r4nt: Let me try to quickly hack something, we'll see if that solves your problem

@xavierd
Copy link
Owner

xavierd commented Jan 21, 2013

@r4nt Could you try the code that I pushed to the branch compilation-database-background? There is probably lots of issues, but it should in theory work :).

@r4nt
Copy link
Author

r4nt commented Jan 22, 2013

Much better - very cool, thanks! I'll work with that from now on and let
you know when I find problems (currently all problems I find are in my
build system integration...)

On Mon, Jan 21, 2013 at 8:35 PM, Xavier Deguillard <notifications@github.com

wrote:

@r4nt https://github.com/r4nt Could you try the code that I pushed to
the branch compilation-database-background? There is probably lots of
issues, but it should in theory work :).


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-12513680.

@r4nt
Copy link
Author

r4nt commented Jan 22, 2013

I have one more problem:
I've added the builtin includes to the search path, but I still get the
warning about libclang not finding the builtin includes (although it finds
it). I think my user-added path is not added when checking for the builtin
includes :)

On Tue, Jan 22, 2013 at 10:42 AM, Manuel Klimek klimek@box4.net wrote:

Much better - very cool, thanks! I'll work with that from now on and let
you know when I find problems (currently all problems I find are in my
build system integration...)

On Mon, Jan 21, 2013 at 8:35 PM, Xavier Deguillard <
notifications@github.com> wrote:

@r4nt https://github.com/r4nt Could you try the code that I pushed to
the branch compilation-database-background? There is probably lots of
issues, but it should in theory work :).


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-12513680.

@r4nt
Copy link
Author

r4nt commented Jan 22, 2013

One more problem:
Exception ctypes.ArgumentError: "argument 1: <type
'exceptions.AttributeError'>: 'TranslationUnit' object has no attribute
'as_parameter'" in <bound method TranslationUnit.del of
<clang.cindex.TranslationUnit object at 0x10534d0>> ignored
Press ENTER or type command to continue

Happens sometimes now, and only goes away when restarting vim.

Cheers,
/Manuel

On Tue, Jan 22, 2013 at 10:52 AM, Manuel Klimek klimek@box4.net wrote:

I have one more problem:
I've added the builtin includes to the search path, but I still get the
warning about libclang not finding the builtin includes (although it finds
it). I think my user-added path is not added when checking for the builtin
includes :)

On Tue, Jan 22, 2013 at 10:42 AM, Manuel Klimek klimek@box4.net wrote:

Much better - very cool, thanks! I'll work with that from now on and let
you know when I find problems (currently all problems I find are in my
build system integration...)

On Mon, Jan 21, 2013 at 8:35 PM, Xavier Deguillard <
notifications@github.com> wrote:

@r4nt https://github.com/r4nt Could you try the code that I pushed to
the branch compilation-database-background? There is probably lots of
issues, but it should in theory work :).


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-12513680.

@r4nt
Copy link
Author

r4nt commented Jan 22, 2013

So, one more thing: the threaded retrieval of the command line options
works great when it's slow, but when I work on an open source project where
it's fast, it doesn't work well (see last mail, and I also managed to
completely get vim hanging during completion).

On Tue, Jan 22, 2013 at 1:55 PM, Manuel Klimek klimek@box4.net wrote:

One more problem:
Exception ctypes.ArgumentError: "argument 1: <type
'exceptions.AttributeError'>: 'TranslationUnit' object has no attribute
'as_parameter'" in <bound method TranslationUnit.del of
<clang.cindex.TranslationUnit object at 0x10534d0>> ignored
Press ENTER or type command to continue

Happens sometimes now, and only goes away when restarting vim.

Cheers,
/Manuel

On Tue, Jan 22, 2013 at 10:52 AM, Manuel Klimek klimek@box4.net wrote:

I have one more problem:
I've added the builtin includes to the search path, but I still get the
warning about libclang not finding the builtin includes (although it finds
it). I think my user-added path is not added when checking for the builtin
includes :)

On Tue, Jan 22, 2013 at 10:42 AM, Manuel Klimek klimek@box4.net wrote:

Much better - very cool, thanks! I'll work with that from now on and let
you know when I find problems (currently all problems I find are in my
build system integration...)

On Mon, Jan 21, 2013 at 8:35 PM, Xavier Deguillard <
notifications@github.com> wrote:

@r4nt https://github.com/r4nt Could you try the code that I pushed
to the branch compilation-database-background? There is probably lots of
issues, but it should in theory work :).


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-12513680.

@oblitum
Copy link
Contributor

oblitum commented Jan 22, 2013

@r4nt, this is a long time issue, and will continue to be while the time.sleep(0.1) hack is used. Python threading, by itself, for me, doesn't cause any problems, trying synchronization employing that causes. I've solved it using no time.sleep for automatic waiting of initialization but by calling a custom g:ClangBackgroundParse() function from my .vimrc at a point I know everything clang_complete will use is already initialized.

@xavierd
Copy link
Owner

xavierd commented Jan 23, 2013

@r4nt, @oblitum where are you setting g:clang_user_options? (@oblitum, the time.sleep has nothing to do with the plugin initialisation since it happens after).

@r4nt: could you get a python backtrace when the error on TranslationUnit happen?

@tobiasgrosser
Copy link
Collaborator

@r4nt. What is the path to the clang builtin includes, and what is the path to libclang itself?

@r4nt
Copy link
Author

r4nt commented Jan 30, 2013

On Wed, Jan 23, 2013 at 6:36 AM, Xavier Deguillard <notifications@github.com

wrote:

@r4nt https://github.com/r4nt, @oblitum https://github.com/oblitumwhere are you setting g:clang_user_options? (
@oblitum https://github.com/oblitum, the time.sleep has nothing to do
with the plugin initialisation since it happens after).

In ~/.vimrc; I tried before my pathogen setup and afterwards, but I still
get the error.

@r4nt https://github.com/r4nt: could you get a python backtrace when
the error on TranslationUnit happen?

I tried a while longer and cannot reproduce it ... (not sure whether that's
good or bad ;)


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-12582387.

@r4nt
Copy link
Author

r4nt commented Jan 30, 2013

On Thu, Jan 24, 2013 at 4:40 PM, Tobias Grosser notifications@github.comwrote:

@r4nt https://github.com/r4nt. What is the path to the clang builtin
includes, and what is the path to libclang itself?

I'm using special paths for both, so in my ~/.vimrc I have:
let g:clang_library_path = '/local/path/to/libclang'
let g:clang_user_options = '-I/local/path/to/libclang/include/'

@xavierd
Copy link
Owner

xavierd commented Jan 31, 2013

@r4nt: I've rebased the branch to master, and I've merged #259. So you may want to retry :)

@r4nt
Copy link
Author

r4nt commented Jan 31, 2013

I now get the error:
Error detected while processing function
57_ClangCompleteInit..57_initClangCompletePython:
line 16:
Traceback (most recent call last):
File "", line 1, in
File "/usr/local/google/home/klimek/.vim/plugin/libclang.py", line 75, in
initClangComplete
params = getCompileParams(vim.current.buffer.name)
NameError: global name 'getCompileParams' is not defined
line 17:
E121: Undefined variable: l:res
E15: Invalid expression: l:res == 0

On Thu, Jan 31, 2013 at 7:30 AM, Xavier Deguillard <notifications@github.com

wrote:

@r4nt https://github.com/r4nt: I've rebased the branch to master, and
I've merged #259 #259.
So you may want to retry :)


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-12929621.

@xavierd
Copy link
Owner

xavierd commented Feb 1, 2013

Indeed… It's fixed now.

@r4nt
Copy link
Author

r4nt commented Feb 1, 2013

Works, but now it seems like I get stuff running in the main thread again -
opening a file sometimes blocks for quite a while...

On Fri, Feb 1, 2013 at 4:55 AM, Xavier Deguillard
notifications@github.comwrote:

Indeed… It's fixed now.


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-12979702.

@ismail
Copy link

ismail commented Feb 1, 2013

FWIW this needs a vim patch applied, see ftp://ftp.vim.org/pub/vim/patches/7.3/7.3.786

@xavierd
Copy link
Owner

xavierd commented Feb 2, 2013

@r4nt: I should definitively stop pushing code when I'm tired… It should be ok now.

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

No branches or pull requests

5 participants