aganea marked an inline comment as done.
aganea added inline comments.
================
Comment at: clang-tools-extra/clangd/index/Background.cpp:154
assert(this->IndexStorageFactory && "Storage factory can not be null!");
- for (unsigned I = 0; I < ThreadPoolSize; ++I) {
+ for (unsigned I = 0; I < Rebuilder.TUsBeforeFirstBuild; ++I) {
ThreadPool.runAsync("background-worker-" + llvm::Twine(I + 1), [this] {
----------------
sammccall wrote:
> Hmm, I finally stumbled across this today when editing the rebuild policy.
> I do wish this hadn't been changed without understanding what this code is
> doing or getting review from an owner.
>
> After this change, modifying the background-index rebuild frequency (which
> was initially tuned to be roughly "after each thread built one file") has the
> side-effect of changing the number of threads used for background indexing!
>
> Less seriously, the use of zero to mean "all the threads" is problematic here
> because in the other thread roots in this project we use zero to mean "no
> threads, work synchronously".
>
> I'll look into reverting the clangd parts of this patch.
Sorry about that Sam. Do you think 'one' could be used in clangd instead? That
is the common value used across other parts of LLVM to signify 'no threads',
and also when `LLVM_ENABLE_THREADS` is off. 'zero' means to use the default
settings for the thread strategy. That is, `llvm::hardware_concurrency(0)`
means to use all hardware threads; or
`llvm::heavyweight_hardware_concurrency(0)` means to use all hardware cores,
but only one `std::thread` per core.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71775/new/
https://reviews.llvm.org/D71775
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
