[clang-tools-extra] r368498 - clangd: use -j for background index pool

[Hans Wennborg via cfe-commits]

Merged to release_90 in r368569.

On Sat, Aug 10, 2019 at 1:02 AM Sam McCall via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> Author: sammccall
> Date: Fri Aug  9 16:03:32 2019
> New Revision: 368498
>
> URL: http://llvm.org/viewvc/llvm-project?rev=368498&view=rev
> Log:
> clangd: use -j for background index pool
>
> Summary:
> clangd supports a -j option to limit the amount of threads to use for parsing
> TUs. However, when using -background-index (the default in later versions of
> clangd), the parallelism used by clangd defaults to the hardware_parallelisn,
> i.e. number of physical cores.
>
> On shared hardware environments, with large projects, this can significantly
> affect performance with no way to tune it down.
>
> This change makes the -j parameter apply equally to parsing and background
> index. It's not perfect, because the total number of threads is 2x the -j value,
> which may still be unexpected. But at least this change allows users to prevent
> clangd using all CPU cores.
>
> Reviewers: kadircet, sammccall
>
> Reviewed By: sammccall
>
> Subscribers: javed.absar, jfb, sammccall, ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D66031
>
> Modified:
>     clang-tools-extra/trunk/clangd/ClangdServer.cpp
>     clang-tools-extra/trunk/clangd/TUScheduler.cpp
>     clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=368498&r1=368497&r2=368498&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Aug  9 16:03:32 2019
> @@ -40,9 +40,11 @@
>  #include "llvm/Support/FileSystem.h"
>  #include "llvm/Support/Path.h"
>  #include "llvm/Support/raw_ostream.h"
> +#include <algorithm>
>  #include <future>
>  #include <memory>
>  #include <mutex>
> +#include <type_traits>
>
>  namespace clang {
>  namespace clangd {
> @@ -117,8 +119,7 @@ ClangdServer::ClangdServer(const GlobalC
>                       : nullptr),
>        GetClangTidyOptions(Opts.GetClangTidyOptions),
>        SuggestMissingIncludes(Opts.SuggestMissingIncludes),
> -      TweakFilter(Opts.TweakFilter),
> -      WorkspaceRoot(Opts.WorkspaceRoot),
> +      TweakFilter(Opts.TweakFilter), WorkspaceRoot(Opts.WorkspaceRoot),
>        // Pass a callback into `WorkScheduler` to extract symbols from a newly
>        // parsed file and rebuild the file index synchronously each time an AST
>        // is parsed.
> @@ -144,7 +145,8 @@ ClangdServer::ClangdServer(const GlobalC
>      BackgroundIdx = llvm::make_unique<BackgroundIndex>(
>          Context::current().clone(), FSProvider, CDB,
>          BackgroundIndexStorage::createDiskBackedStorageFactory(
> -            [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }));
> +            [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),
> +        std::max(Opts.AsyncThreadsCount, 1u));
>      AddIndex(BackgroundIdx.get());
>    }
>    if (DynamicIdx)
>
> Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=368498&r1=368497&r2=368498&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
> +++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Fri Aug  9 16:03:32 2019
> @@ -54,6 +54,7 @@
>  #include "llvm/ADT/ScopeExit.h"
>  #include "llvm/Support/Errc.h"
>  #include "llvm/Support/Path.h"
> +#include "llvm/Support/Threading.h"
>  #include <algorithm>
>  #include <memory>
>  #include <queue>
> @@ -801,10 +802,10 @@ std::string renderTUAction(const TUActio
>  } // namespace
>
>  unsigned getDefaultAsyncThreadsCount() {
> -  unsigned HardwareConcurrency = std::thread::hardware_concurrency();
> -  // C++ standard says that hardware_concurrency()
> -  // may return 0, fallback to 1 worker thread in
> -  // that case.
> +  unsigned HardwareConcurrency = llvm::heavyweight_hardware_concurrency();
> +  // heavyweight_hardware_concurrency may fall back to hardware_concurrency.
> +  // C++ standard says that hardware_concurrency() may return 0; fallback to 1
> +  // worker thread in that case.
>    if (HardwareConcurrency == 0)
>      return 1;
>    return HardwareConcurrency;
>
> Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=368498&r1=368497&r2=368498&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
> +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Fri Aug  9 16:03:32 2019
> @@ -267,7 +267,8 @@ list<std::string> TweakList{
>  opt<unsigned> WorkerThreadsCount{
>      "j",
>      cat(Misc),
> -    desc("Number of async workers used by clangd"),
> +    desc("Number of async workers used by clangd. Background index also "
> +         "uses this many workers."),
>      init(getDefaultAsyncThreadsCount()),
>  };
>
> @@ -308,7 +309,8 @@ opt<PCHStorageFlag> PCHStorage{
>  opt<bool> Sync{
>      "sync",
>      cat(Misc),
> -    desc("Parse on main thread. If set, -j is ignored"),
> +    desc("Handle client requests on main thread. Background index still uses "
> +         "its own thread."),
>      init(false),
>      Hidden,
>  };
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits