ilya-biryukov added a comment. It's fine to spend one thread spinning on background tasks, but if we're going to do a threadpool, we should be more careful to not hurt the performance of foreground tasks. To do that, we should at least:
- share the semaphore for the number of actively running tasks between TUScheduler and BackgroundIndex. - prioritize foreground tasks over background tasks. Ideally, probably outside the scope of this change, we should make the background tasks cancellable (or find another way to keep them from interfering with the foreground tasks) in addition to that and make sure the scheduling is aware of distinction between foreground and background tasks. (@sammcall suggested simply using thread priorities for that). Another high-level comment is that we should probably use llvm::ThreadPool here, this would have give code reuse and would make `BackgroundIndex` more focused on the actual indexing part. ================ Comment at: clangd/index/Background.h:80 + // Must be last, spawned thread reads instance vars. + llvm::SmallVector<std::thread, 8> ThreadPool; }; ---------------- Why not `std::vector`? Memory allocs won't ever be a bottleneck here. 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
