[PATCH] D114845: [llvm] [Debuginfod] DebuginfodCollection and DebuginfodServer for tracking local debuginfo.
Daniel Thornburgh via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 1 17:07:46 PST 2022
mysterymath added inline comments.
Herald added a project: All.
================
Comment at: llvm/include/llvm/Debuginfod/Debuginfod.h:85
+ ThreadPool &Pool;
+ size_t Concurrency = 1;
+
----------------
I don't think the `= 1` does anything, since providing a constructor inhibits generation of a default one.
================
Comment at: llvm/include/llvm/Debuginfod/Debuginfod.h:96
+
+class DebuginfodServer {
+public:
----------------
Since all members are public, this should be a `struct`. This also fits with how it's used, post-construction.
================
Comment at: llvm/lib/Debuginfod/Debuginfod.cpp:217-218
+ return Err;
+ outs() << "finished first update\n";
+ outs().flush();
+ std::this_thread::sleep_for(Interval);
----------------
This seems a bit odd to unconditionally put to stdout; is this necessary?
Otherwise, the whole routine just becomes
```
while (true) {
if (Error Err = update())
return Err;
std::this_thread::sleep_for(Interval);
}
return Error::success();
```
================
Comment at: llvm/lib/Debuginfod/Debuginfod.cpp:225
+ }
+ return Error::success();
+}
----------------
llvm_unreachable()?
================
Comment at: llvm/lib/Debuginfod/Debuginfod.cpp:315-319
+ // Wait for the number of concurrent jobs to go down
+ while (NumTasksRemaining > Concurrency) {
+ LLVM_DEBUG(dbgs() << NumTasksRemaining << " tasks remaining\n";);
+ std::this_thread::sleep_for(std::chrono::milliseconds(50));
+ }
----------------
Is there an advantage for manually managing the concurrency here, over passing it as an argument to ThreadPool, `std::min`-ed with the hardware concurrency?
>From a cursor look at ThreadPool's API, each async call after the thread pool is full should just more-or-less push a std::function<void()> onto a vector.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114845/new/
https://reviews.llvm.org/D114845
More information about the llvm-commits
mailing list