[llvm] r256087 - ThreadPool unittest: reimplement concurrency test, deterministically this time.

[Vedant Kumar via llvm-commits]

>> On Dec 18, 2015, at 8:30 PM, Vedant Kumar <vsk at apple.com> wrote:
>> 
>> Thanks :).
>> 
>>> +
>>> +  /// Make sure this thread not progress faster than the main thread.
>>> +  void waitForMainThread() {
>>> +    while (!MainThreadReady) {
>>> +      std::unique_lock<std::mutex> LockGuard(WaitMainThreadMutex);
>>> +      WaitMainThread.wait(LockGuard, [&] { return MainThreadReady; });
>> 
>> Nit: wait(CV, Predicate) is guaranteed to not wake up spuriously. The surrounding while loop here is redundant.
>> 
>> 
>>> +    }
>>> +  }
>>> +  std::condition_variable WaitMainThread;
>>> +  std::mutex WaitMainThreadMutex;
>>> +  bool MainThreadReady;
>>> +
>>> };
>>> 
>>> #define CHECK_UNSUPPORTED() \
>>> @@ -68,12 +80,17 @@ TEST_F(ThreadPoolTest, AsyncBarrier) {
>>> 
>>> std::atomic_int checked_in{0};
>>> 
>>> +  MainThreadReady = false;
>>> ThreadPool Pool;
>>> for (size_t i = 0; i < 5; ++i) {
>>> -    Pool.async([&checked_in, i] {
>>> +    Pool.async([this, &checked_in, i] {
>>> +      waitForMainThread();
>>>     ++checked_in;
>>>   });
>>> }
>>> +  ASSERT_EQ(0, checked_in);
>>> +  MainThreadReady = true;
>> 
>> Paranoid nit: Can this assignment be reordered before the assert? If so, the main thread could context-switch to an async worker that's awake.
> 
> It's even worse: you're supposed to get the mutex before notifying a condition variable. I always forget that, Teresa already caught this on the original patch.
> 
>> Later, the assert will fail. Wdyt of changing this to --
>> 
>> void setMainThreadReadyState(bool Ready) {
>>   std::unique_lock<std::mutex> LockGuard(WaitMainThreadMutex);
>>   MainThreadReady = Ready;
>> }
>> 
>> setMainThreadReadyState(true);
>> 
>> That should still test the async behavior.
> 
> Would need the notify_all while holding the mutex. Otherwise LGTM.

Done, r256096.

vedant