[PATCH] D114845: [llvm] [Debuginfod] DebuginfodCollection and DebuginfodServer for tracking local debuginfo.

[Noah Shutty via Phabricator via llvm-commits]

noajshu added inline comments.


================
Comment at: llvm/lib/Debuginfod/Debuginfod.cpp:324
+    // unbounded memory usage
+    Pool.waitQueueEmpty();
+  }
----------------
noajshu wrote:
> noajshu wrote:
> > mysterymath wrote:
> > > If I'm reading this right, wouldn't this loop dispatch one item to the queue, wait for the queue to be empty, dispatch another item, wait for the queue to be empty, etc. It seems like this disables parallelism entirely.
> > > 
> > > I'd have expected this to wait until the queue was "not full"; then items would be dispatched until max concurrency was reached, and the next item dispatched the moment a thread becomes free.
> > I don't think this should disable parallelism entirely as the queue will become empty as soon as the job starts processing in a worker thread, rather than when that job finishes. Let me double check this.
> On a closer look, you're right about it disabling parallelism. This is due to `ThreadPool`'s internals! Further modification of the `ThreadPool` is required.
> 
> Also I think it would be simple enough to parametrize as `waitQueueSize(size_t Size)`, blocking until the queue has at most Size tasks.
I corrected the implementation of `waitQueueSize`. Thank you for catching this!

Although it takes a `Size` parameter, I leave it as the default of 0 here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114845/new/

https://reviews.llvm.org/D114845