Common: Blocking Loop (extracted from Fifo.cpp)

[degasus]

This PR tries to extract the synchronized main loop of our fifo as shared file within Common.

The new Common Class represent a way to implement a "busy loop" which sleeps on no work. It is optimized for a high frequent pushing of new data while the loop is already running.

[degasus]

degasus> so from a testing point of view, PR 2470 is fine?
JMC47> yes
JMC47> no change in functionality at all

[comex]

Okay. I think I've convinced myself that the atomic logic is correct, so sorry for doubting you @degasus :)

Proof that Wait() does not return early:
    - Flag's operations are done with std::memory_order_seq_cst,
      so the following linear idea of time is correct:
    - once Set ensures that m_is_running is true, there must be
      moments when (a) running is clear and (b) pending is clear,
      in that order, for a subsequent Wait to return
        - m_is_running is only cleared by Run while m_is_pending
          is true, so m_is_pending must subsequently be cleared
          after that clear (which itself is after Set started, so
          the work is already reflected in the other atomics)
        - m_is_pending will not be cleared until after the /next/
          payload invocation
    - (alternately, m_running could become false, but this is
      documented to cause Wait to return)

Proof that Wait() does return:
    - The only ways for the clear-running -> payload ->
      clear-pending -> set-done-event path in Run to not
      eventually make both remain false are if:
        - m_is_running is constantly set by another Set on a
          different thread - that could cause problems in a
          multiple-producer scenario, but with Dolphin, Wait/Set
          are only called from the CPU thread - in any case, it
          will at least return once activity subsides
        - m_shutdown is set.  In this case, m_running is cleared
          before m_done_event is set, so Wait will not block.
    - Since m_done_event is always set at the end of this path,
      Wait cannot miss the timing.

Suggestions:
    - There is a 100ms timeout in Run's wait that isn't obviously
      necessary.  I guess this is needed by the GPU thread for
      some event loop nonsense, but it should probably be
      customizable in this class.
    - The names of the variables are confusing and not really
      accurate - e.g. m_is_running can be false while payload is
      running.  (I partially blame this for my confusion, but of
      course atomics are hard and I'm dumb :)
    - Do we care about the multiple-producer issue?

[degasus]

@comex Thanks for your huge efforts, I've tried to implement your suggestions.
My two atomic variables are now within one atomic which is controlled by an enum. I hope the new name is better.
The timeout is now optional.
I've added some more comments to describe what I'm doing.

Missing:

• I didn't care about the multiple-producer issue. I think it's possible to solve this, but I'm not sure if it's worth.
• I'm not sure if it's worth to add a new "state" for sleeping. So we could skip the event notification while the worker thread is in the busy loop. Do you think this is worth? I've just done it. It looks nicer than I thought it will be.

[degasus]

JMC47> seems exactly the same

[dolphin-emu-bot]

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

• mii-channel on dx-win-nv: diff

_{^{automated-fifoci-reporter}}

[comex]

Haven't thought about the new version as thoroughly as the last one, but it seems reasonable. Thanks!