kadircet added a comment. Moving forward with priorities then.
================ Comment at: clangd/index/Background.cpp:89 } - QueueCV.notify_all(); + QueueCV.notify_one(); } ---------------- sammccall wrote: > I always forget the details of how these work :-\ > Is it possible for the "one" notification to be consumed by a waiter on > blockUntilIdleForTest? > > In general I'm not sure whether the `notify_one` optimization is worth the > correctness risk as the code evolves. Ah, you are right, a thread waiting for done might catch that one as well, but I think it only applies to that one. Is there a possibility of `blockUntilIdleForTest` and `enqueue` being called from different threads? There is still the argument of code evolution, but I don't think we should ever end up in a state in which an enqueue and a wait that will not consume that enqueue should occur concurrently. ================ Comment at: clangd/index/Background.h:80 + // Must be last, spawned thread reads instance vars. + llvm::SmallVector<std::thread, 8> ThreadPool; }; ---------------- sammccall wrote: > ilya-biryukov wrote: > > Why not `std::vector`? Memory allocs won't ever be a bottleneck here. > ilya was saying nice things about `llvm::ThreadPool` recently - worth a look? going for llvm::ThreadPool Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53651 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
