From c9e1706fa1f3d8a380803f49176d86ad008e3e61 Mon Sep 17 00:00:00 2001 From: Strahinja Val Markovic Date: Sun, 15 Jul 2012 20:49:56 -0700 Subject: [PATCH] ClangCompleter now async and caches Clang data First off, we don't block the GUI thread anymore for ClangCompleter (that was always temporary). Secondly, now ClangCompleter will cache the data coming from clang so that query-based filtering of members is fast. --- autoload/youcompleteme.vim | 30 +++++- cpp/ycm/ClangCompleter.cpp | 170 ++++++++++++++++++++++++++++++-- cpp/ycm/ClangCompleter.h | 46 ++++++++- cpp/ycm/Future.cpp | 58 ----------- cpp/ycm/Future.h | 48 +++++++-- cpp/ycm/IdentifierCompleter.cpp | 35 +++---- cpp/ycm/IdentifierCompleter.h | 12 ++- cpp/ycm/indexer.cpp | 14 +-- python/ycm.py | 52 ++++++---- 9 files changed, 335 insertions(+), 130 deletions(-) delete mode 100644 cpp/ycm/Future.cpp diff --git a/autoload/youcompleteme.vim b/autoload/youcompleteme.vim index 506d7bb6..c9f51dfb 100644 --- a/autoload/youcompleteme.vim +++ b/autoload/youcompleteme.vim @@ -77,6 +77,7 @@ endfunction function! s:OnCursorHold() + " TODO: make this async, it's causing lag py identcomp.AddBufferIdentifiers() endfunction @@ -178,8 +179,10 @@ EOF let l:results = [] py << EOF results = identcomp.CandidatesFromStoredRequest() +result_string = ycm.StringVectorToString( results ) + if results: - vim.command( 'let l:results = ' + str( results ) ) + vim.command( 'let l:results = ' + result_string ) EOF let s:searched_and_no_results_found = len( l:results ) == 0 @@ -196,8 +199,29 @@ endfunction function! s:ClangCompletion( query ) " TODO: don't trigger on a dot inside a string constant - py vim.command( 'let l:results = ' + - \ str( clangcomp.CandidatesForQuery( vim.eval( 'a:query' ) ) ) ) + + py clangcomp.CandidatesForQueryAsync( vim.eval('a:query') ) + + let l:results_ready = 0 + while !l:results_ready + py << EOF +results_ready = clangcomp.AsyncCandidateRequestReady() +if results_ready: + vim.command( 'let l:results_ready = 1' ) +EOF + if complete_check() + return { 'words' : [], 'refresh' : 'always'} + endif + endwhile + + let l:results = [] + py << EOF +results = clangcomp.CandidatesFromStoredRequest() +result_string = ycm.StringVectorToString( results ) + +if results: + vim.command( 'let l:results = ' + result_string ) +EOF let s:searched_and_no_results_found = len( l:results ) == 0 return { 'words' : l:results, 'refresh' : 'always' } diff --git a/cpp/ycm/ClangCompleter.cpp b/cpp/ycm/ClangCompleter.cpp index a5a988c3..381e3bb5 100644 --- a/cpp/ycm/ClangCompleter.cpp +++ b/cpp/ycm/ClangCompleter.cpp @@ -19,15 +19,29 @@ #include "Candidate.h" #include "standard.h" #include "CandidateRepository.h" +#include "ConcurrentLatestValue.h" #include +#include + +using boost::packaged_task; +using boost::bind; +using boost::unique_future; +using boost::make_shared; +using boost::shared_ptr; +using boost::bind; +using boost::thread; namespace YouCompleteMe { +extern const unsigned int MAX_ASYNC_THREADS; +extern const unsigned int MIN_ASYNC_THREADS; + namespace { + std::vector< CXUnsavedFile > ToCXUnsavedFiles( const std::vector< UnsavedFile > &unsaved_files ) { @@ -91,7 +105,9 @@ std::vector< std::string > ToStringVector( CXCodeCompleteResults *results ) ClangCompleter::ClangCompleter() - : candidate_repository_( CandidateRepository::Instance() ) + : candidate_repository_( CandidateRepository::Instance() ), + threading_enabled_( false ), + clang_data_ready_( false ) { clang_index_ = clang_createIndex( 0, 0 ); } @@ -109,6 +125,15 @@ ClangCompleter::~ClangCompleter() } +// We need this mostly so that we can not use it in tests. Apparently the +// GoogleTest framework goes apeshit on us if we enable threads by default. +void ClangCompleter::EnableThreading() +{ + threading_enabled_ = true; + InitThreads(); +} + + void ClangCompleter::SetGlobalCompileFlags( const std::vector< std::string > &flags ) { @@ -152,7 +177,6 @@ void ClangCompleter::UpdateTranslationUnit( std::vector< std::string > ClangCompleter::CandidatesForLocationInFile( - const std::string &query, const std::string &filename, int line, int column, @@ -166,7 +190,7 @@ std::vector< std::string > ClangCompleter::CandidatesForLocationInFile( // If there are unsaved files, then codeCompleteAt will parse the in-memory // file contents we are giving it. In short, it is NEVER a good idea to call // clang_reparseTranslationUnit right before a call to clang_codeCompleteAt. - // The only makes clang reparse the whole file TWICE, which has a huge impact + // This only makes clang reparse the whole file TWICE, which has a huge impact // on latency. At the time of writing, it seems that most users of libclang // in the open-source world don't realize this (I checked). Some don't even // call reparse*, but parse* which is even less efficient. @@ -180,12 +204,64 @@ std::vector< std::string > ClangCompleter::CandidatesForLocationInFile( cxunsaved_files.size(), clang_defaultCodeCompleteOptions()); - std::vector< std::string > completions = ToStringVector( results ); - if ( !query.empty() ) - completions = SortCandidatesForQuery( query, completions ); - + std::vector< std::string > candidates = ToStringVector( results ); clang_disposeCodeCompleteResults( results ); - return completions; + return candidates; +} + + +Future< AsyncResults > ClangCompleter::CandidatesForQueryAndLocationInFileAsync( + const std::string &query, + const std::string &filename, + int line, + int column, + const std::vector< UnsavedFile > &unsaved_files ) +{ + // TODO: throw exception when threading is not enabled and this is called + if ( !threading_enabled_ ) + return Future< AsyncResults >(); + + if ( query.empty() ) + { + { + boost::lock_guard< boost::mutex > lock( clang_data_ready_mutex_ ); + clang_data_ready_ = false; + } + sorting_threads_.interrupt_all(); + } + + // the sorting task needs to be set before the clang task (if any) just in + // case the clang task finishes (and therefore notifies a sorting thread to + // consume a sorting task) before the sorting task is set + shared_ptr< packaged_task< AsyncResults > > task = + make_shared< packaged_task< AsyncResults > >( + bind( ReturnValueAsShared< std::vector< std::string > >, + static_cast< FunctionReturnsStringVector >( + bind( &ClangCompleter::SortCandidatesForQuery, + boost::ref( *this ), + query, + boost::cref( latest_clang_results_ ) ) ) ) ); + + unique_future< AsyncResults > future = task->get_future(); + sorting_task_.Set( task ); + + if ( query.empty() ) + { + shared_ptr< packaged_task< AsyncResults > > task = + make_shared< packaged_task< AsyncResults > >( + bind( ReturnValueAsShared< std::vector< std::string > >, + static_cast< FunctionReturnsStringVector >( + bind( &ClangCompleter::CandidatesForLocationInFile, + boost::ref( *this ), + filename, + line, + column, + unsaved_files ) ) ) ); + + clang_task_.Set( task ); + } + + return Future< AsyncResults >( move( future ) ); } @@ -281,4 +357,82 @@ std::vector< std::string > ClangCompleter::SortCandidatesForQuery( return sorted_candidates; } + +void ClangCompleter::InitThreads() +{ + int threads_to_create = + std::max( MIN_ASYNC_THREADS, + std::min( MAX_ASYNC_THREADS, thread::hardware_concurrency() ) ); + + for ( int i = 0; i < threads_to_create; ++i ) + { + sorting_threads_.create_thread( + bind( &ClangCompleter::SortingThreadMain, + boost::ref( *this ), + boost::ref( sorting_task_ ) ) ); + } + + clang_thread_ = boost::thread( &ClangCompleter::ClangThreadMain, + boost::ref( *this ), + boost::ref( clang_task_ ) ); +} + + +void ClangCompleter::ClangThreadMain( LatestTask &clang_task ) +{ + while ( true ) + { + shared_ptr< packaged_task< AsyncResults > > task = clang_task.Get(); + ( *task )(); + unique_future< AsyncResults > future = task->get_future(); + + { + boost::unique_lock< boost::shared_mutex > writer_lock( + latest_clang_results_shared_mutex_ ); + latest_clang_results_ = *future.get(); + } + + { + boost::lock_guard< boost::mutex > lock( clang_data_ready_mutex_ ); + clang_data_ready_ = true; + } + + clang_data_ready_condition_variable_.notify_all(); + } +} + + +void ClangCompleter::SortingThreadMain( LatestTask &sorting_task ) +{ + while ( true ) + { + try + { + { + boost::unique_lock< boost::mutex > lock( clang_data_ready_mutex_ ); + + while ( !clang_data_ready_ ) + { + clang_data_ready_condition_variable_.wait( lock ); + } + } + + shared_ptr< packaged_task< AsyncResults > > task = sorting_task.Get(); + + { + boost::shared_lock< boost::shared_mutex > reader_lock( + latest_clang_results_shared_mutex_ ); + + ( *task )(); + } + } + + catch ( boost::thread_interrupted& ) + { + // Do nothing and re-enter the loop + } + } +} + + } // namespace YouCompleteMe diff --git a/cpp/ycm/ClangCompleter.h b/cpp/ycm/ClangCompleter.h index 2a5a4374..674065bb 100644 --- a/cpp/ycm/ClangCompleter.h +++ b/cpp/ycm/ClangCompleter.h @@ -18,6 +18,9 @@ #ifndef CLANGCOMPLETE_H_WLKDU0ZV #define CLANGCOMPLETE_H_WLKDU0ZV +#include "ConcurrentLatestValue.h" +#include "Future.h" + #include #include @@ -55,6 +58,7 @@ struct UnsavedFile typedef boost::unordered_map< std::string, std::vector< std::string > > FlagsForFile; + typedef boost::unordered_map< std::string, CXTranslationUnit > TranslationUnitForFilename; @@ -65,6 +69,8 @@ public: ClangCompleter(); ~ClangCompleter(); + void EnableThreading(); + void SetGlobalCompileFlags( const std::vector< std::string > &flags ); void SetFileCompileFlags( const std::string &filename, @@ -73,8 +79,13 @@ public: void UpdateTranslationUnit( const std::string &filename, const std::vector< UnsavedFile > &unsaved_files ); - // TODO: rename this std::vector< std::string > CandidatesForLocationInFile( + const std::string &filename, + int line, + int column, + const std::vector< UnsavedFile > &unsaved_files ); + + Future< AsyncResults > CandidatesForQueryAndLocationInFileAsync( const std::string &query, const std::string &filename, int line, @@ -82,6 +93,9 @@ public: const std::vector< UnsavedFile > &unsaved_files ); private: + typedef ConcurrentLatestValue< + boost::shared_ptr< + boost::packaged_task< AsyncResults > > > LatestTask; // caller takes ownership of translation unit CXTranslationUnit CreateTranslationUnit( @@ -99,17 +113,47 @@ private: const std::string &query, const std::vector< std::string > &candidates ); + void InitThreads(); + + void ClangThreadMain( LatestTask &clang_task ); + + void SortingThreadMain( LatestTask &sorting_task ); + ///////////////////////////// // PRIVATE MEMBER VARIABLES ///////////////////////////// CXIndex clang_index_; + FlagsForFile flags_for_file_; + TranslationUnitForFilename filename_to_translation_unit_; + std::vector< std::string > global_flags_; + CandidateRepository &candidate_repository_; + mutable LatestTask clang_task_; + + mutable LatestTask sorting_task_; + + bool threading_enabled_; + + // TODO: use boost.atomic for clang_data_ready_ + bool clang_data_ready_; + boost::mutex clang_data_ready_mutex_; + boost::condition_variable clang_data_ready_condition_variable_; + + std::vector< std::string > latest_clang_results_; + boost::shared_mutex latest_clang_results_shared_mutex_; + + // Unfortunately clang is not thread-safe so we can only ever use one thread + // to access it. So this one background thread will be the only thread that + // can access libclang. + boost::thread clang_thread_; + + boost::thread_group sorting_threads_; }; } // namespace YouCompleteMe diff --git a/cpp/ycm/Future.cpp b/cpp/ycm/Future.cpp deleted file mode 100644 index 6b8cbeb9..00000000 --- a/cpp/ycm/Future.cpp +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright (C) 2011, 2012 Strahinja Val Markovic -// -// This file is part of YouCompleteMe. -// -// YouCompleteMe is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// YouCompleteMe is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with YouCompleteMe. If not, see . - -#include "Future.h" -#include "standard.h" -#include "Result.h" - -namespace YouCompleteMe -{ - -Future::Future( boost::shared_future< AsyncResults > future ) - : future_( boost::move( future ) ) -{ -} - -bool Future::ResultsReady() -{ - return future_.is_ready(); -} - -Pylist Future::GetResults() -{ - Pylist candidates; - AsyncResults results; - - try - { - results = future_.get(); - } - - catch ( boost::future_uninitialized & ) - { - return candidates; - } - - foreach ( const Result& result, *results ) - { - candidates.append( *result.Text() ); - } - - return candidates; -} - -} // namespace YouCompleteMe diff --git a/cpp/ycm/Future.h b/cpp/ycm/Future.h index 3f31df7e..8c292f76 100644 --- a/cpp/ycm/Future.h +++ b/cpp/ycm/Future.h @@ -19,8 +19,10 @@ #define FUTURE_H_NR1U6MZS #include +#include #include #include +#include namespace YouCompleteMe { @@ -29,22 +31,50 @@ class Result; template< typename T > class ConcurrentLatestValue; typedef boost::python::list Pylist; -typedef boost::shared_ptr< std::vector< Result > > AsyncResults; +typedef boost::shared_ptr< std::vector< std::string > > AsyncResults; -typedef ConcurrentLatestValue< - boost::shared_ptr< - boost::packaged_task< AsyncResults > > > LatestTask; +typedef boost::function< std::vector< std::string >() > + FunctionReturnsStringVector; +template< typename T > +boost::shared_ptr< T > ReturnValueAsShared( + boost::function< T() > func ) +{ + return boost::make_shared< T >( func() ); +} + + +template< typename T > class Future { public: - Future() {} - Future( boost::shared_future< AsyncResults > future ); - bool ResultsReady(); - Pylist GetResults(); + Future() {}; + Future( boost::shared_future< T > future ) + : future_( boost::move( future ) ) {} + + bool ResultsReady() + { + return future_.is_ready(); + } + + + T GetResults() + { + try + { + return future_.get(); + } + + catch ( boost::future_uninitialized & ) + { + // Do nothing and return a T() + } + + return T(); + } private: - boost::shared_future< AsyncResults > future_; + boost::shared_future< T > future_; }; } // namespace YouCompleteMe diff --git a/cpp/ycm/IdentifierCompleter.cpp b/cpp/ycm/IdentifierCompleter.cpp index a0b44908..67a01d9f 100644 --- a/cpp/ycm/IdentifierCompleter.cpp +++ b/cpp/ycm/IdentifierCompleter.cpp @@ -36,12 +36,12 @@ using boost::thread; namespace YouCompleteMe { +extern const unsigned int MAX_ASYNC_THREADS = 4; +extern const unsigned int MIN_ASYNC_THREADS = 2; + namespace { -const unsigned int MAX_ASYNC_THREADS = 4; -const unsigned int MIN_ASYNC_THREADS = 2; - void ThreadMain( LatestTask &latest_task ) { while ( true ) @@ -135,36 +135,29 @@ std::vector< std::string > IdentifierCompleter::CandidatesForQueryAndType( } -Future IdentifierCompleter::CandidatesForQueryAndTypeAsync( +Future< AsyncResults > IdentifierCompleter::CandidatesForQueryAndTypeAsync( const std::string &query, const std::string &filetype ) const { // TODO: throw exception when threading is not enabled and this is called - if (!threading_enabled_) - return Future(); + if ( !threading_enabled_ ) + return Future< AsyncResults >(); // Try not to look at this too hard, it may burn your eyes. + // TODO: refactor this so it's more readable shared_ptr< packaged_task< AsyncResults > > task = make_shared< packaged_task< AsyncResults > >( - bind( &IdentifierCompleter::ResultsForQueryAndType, - boost::cref( *this ), - query, - filetype ) ); + bind( ReturnValueAsShared< std::vector< std::string > >, + static_cast< FunctionReturnsStringVector >( + bind( &IdentifierCompleter::CandidatesForQueryAndType, + boost::cref( *this ), + query, + filetype ) ) ) ); unique_future< AsyncResults > future = task->get_future(); latest_task_.Set( task ); - return Future( move( future ) ); -} - - -AsyncResults IdentifierCompleter::ResultsForQueryAndType( - const std::string &query, - const std::string &filetype ) const -{ - AsyncResults results = boost::make_shared< std::vector< Result > >(); - ResultsForQueryAndType( query, filetype, *results ); - return results; + return Future< AsyncResults >( move( future ) ); } diff --git a/cpp/ycm/IdentifierCompleter.h b/cpp/ycm/IdentifierCompleter.h index 7b649fa1..ee920948 100644 --- a/cpp/ycm/IdentifierCompleter.h +++ b/cpp/ycm/IdentifierCompleter.h @@ -35,6 +35,7 @@ namespace YouCompleteMe class Candidate; class CandidateRepository; +// TODO: move to private // filepath -> *( *candidate ) typedef boost::unordered_map< std::string, boost::shared_ptr< std::list< const Candidate* > > > @@ -44,6 +45,9 @@ typedef boost::unordered_map< std::string, typedef boost::unordered_map< std::string, boost::shared_ptr< FilepathToCandidates > > FiletypeMap; +typedef ConcurrentLatestValue< + boost::shared_ptr< + boost::packaged_task< AsyncResults > > > LatestTask; class IdentifierCompleter : boost::noncopyable { @@ -70,14 +74,12 @@ public: const std::string &query, const std::string &filetype ) const; - Future CandidatesForQueryAndTypeAsync( const std::string &query, - const std::string &filetype ) const; + Future< AsyncResults > CandidatesForQueryAndTypeAsync( + const std::string &query, + const std::string &filetype ) const; private: - AsyncResults ResultsForQueryAndType( const std::string &query, - const std::string &filetype ) const; - void ResultsForQueryAndType( const std::string &query, const std::string &filetype, std::vector< Result > &results ) const; diff --git a/cpp/ycm/indexer.cpp b/cpp/ycm/indexer.cpp index ad7d9810..c12fca09 100644 --- a/cpp/ycm/indexer.cpp +++ b/cpp/ycm/indexer.cpp @@ -28,12 +28,13 @@ BOOST_PYTHON_MODULE(indexer) using namespace boost::python; using namespace YouCompleteMe; - class_< std::vector< std::string > >( "StringVec" ) + class_< std::vector< std::string >, + boost::shared_ptr< std::vector< std::string > > >( "StringVec" ) .def( vector_indexing_suite< std::vector< std::string > >() ); - class_< Future >( "Future" ) - .def( "ResultsReady", &Future::ResultsReady ) - .def( "GetResults", &Future::GetResults ); + class_< Future< AsyncResults > >( "Future" ) + .def( "ResultsReady", &Future< AsyncResults >::ResultsReady ) + .def( "GetResults", &Future< AsyncResults >::GetResults ); class_< IdentifierCompleter, boost::noncopyable >( "IdentifierCompleter" ) .def( "EnableThreading", &IdentifierCompleter::EnableThreading ) @@ -61,9 +62,10 @@ BOOST_PYTHON_MODULE(indexer) .def( vector_indexing_suite< std::vector< UnsavedFile > >() ); class_< ClangCompleter, boost::noncopyable >( "ClangCompleter" ) + .def( "EnableThreading", &ClangCompleter::EnableThreading ) .def( "SetGlobalCompileFlags", &ClangCompleter::SetGlobalCompileFlags ) .def( "SetFileCompileFlags", &ClangCompleter::SetFileCompileFlags ) .def( "UpdateTranslationUnit", &ClangCompleter::UpdateTranslationUnit ) - .def( "CandidatesForLocationInFile", - &ClangCompleter::CandidatesForLocationInFile ); + .def( "CandidatesForQueryAndLocationInFileAsync", + &ClangCompleter::CandidatesForQueryAndLocationInFileAsync ); } diff --git a/python/ycm.py b/python/ycm.py index b15a84db..c1eff7a5 100644 --- a/python/ycm.py +++ b/python/ycm.py @@ -89,11 +89,15 @@ class IdentifierCompleter( Completer ): filepath, True ) + class ClangCompleter( Completer ): def __init__( self ): self.completer = indexer.ClangCompleter() + self.completer.EnableThreading() + self.contents_holder = [] + self.filename_holder = [] - def CandidatesForQuery( self, query ): + def CandidatesForQueryAsync( self, query ): # TODO: sanitize query files = indexer.UnsavedFileVec() @@ -102,31 +106,32 @@ class ClangCompleter( Completer ): # pointers to data members of python objects. We need to ensure that those # objects outlive our UnsavedFile objects. This is why we need the # contents_holder and filename_holder lists, to make sure the string objects - # are still around when we call CandidatesForLocationInFile. We do this to - # avoid an extra copy of the entire file contents. + # are still around when we call CandidatesForQueryAndLocationInFile. We do + # this to avoid an extra copy of the entire file contents. - contents_holder = [] - filename_holder = [] - for buffer in GetUnsavedBuffers(): - contents_holder.append( '\n'.join( buffer ) ) - filename_holder.append( buffer.name ) + if not query: + self.contents_holder = [] + self.filename_holder = [] + for buffer in GetUnsavedBuffers(): + self.contents_holder.append( '\n'.join( buffer ) ) + self.filename_holder.append( buffer.name ) - unsaved_file = indexer.UnsavedFile() - unsaved_file.contents_ = contents_holder[ -1 ] - unsaved_file.length_ = len( contents_holder[ -1 ] ) - unsaved_file.filename_ = filename_holder[ -1 ] + unsaved_file = indexer.UnsavedFile() + unsaved_file.contents_ = self.contents_holder[ -1 ] + unsaved_file.length_ = len( self.contents_holder[ -1 ] ) + unsaved_file.filename_ = self.filename_holder[ -1 ] - files.append( unsaved_file ) + files.append( unsaved_file ) line, _ = vim.current.window.cursor column = int( vim.eval( "s:completion_start_column" ) ) + 1 current_buffer = vim.current.buffer - results = self.completer.CandidatesForLocationInFile( query, - current_buffer.name, - line, - column, - files ) - return list( results ) + self.future = self.completer.CandidatesForQueryAndLocationInFileAsync( + query, + current_buffer.name, + line, + column, + files ) def GetUnsavedBuffers(): @@ -137,6 +142,15 @@ def GetUnsavedBuffers(): return ( x for x in vim.buffers if BufferModified( x.number ) ) +# TODO: just implement __str__/__repr__ on StringVec +def StringVectorToString( stringvec ): + result = [ "[" ] + for text in stringvec: + result.append( '"{0}",'.format( text ) ) + result.append( "]" ) + return ''.join( result ) + + def CurrentColumn(): """Do NOT access the CurrentColumn in vim.current.line. It doesn't exist yet. Only the chars before the current column exist in vim.current.line."""