[WIP] document new plan with fences

rayon-core/src/sleep/README.md

@@ -10,7 +10,7 @@ extracted from the RFC and meant to be kept up to date.
 [Rayon RFC #5]: https://github.com/rayon-rs/rfcs/pull/5
 [video walkthrough]: https://youtu.be/HvmQsE5M4cY
 
-## The `Sleep` struct
+# The `Sleep` struct
 
 The `Sleep` struct is embedded into each registry. It performs several functions:
 
@@ -21,7 +21,7 @@ The `Sleep` struct is embedded into each registry. It performs several functions
   events occur, and it will go and wake the appropriate threads if
   they are sleeping.
 
-## Thread states
+# Thread states
 
 There are three main thread states:
 
@@ -35,7 +35,7 @@ We sometimes refer to the final two states collectively as **inactive**.
 Threads begin as idle but transition to idle and finally sleeping when
 they're unable to find work to do.
 
-### Sleepy threads
+## Sleepy threads
 
 There is one other special state worth mentioning. During the idle state,
 threads can get **sleepy**. A sleepy thread is still idle, in that it is still
@@ -48,7 +48,7 @@ not guaranteed) see that the counter has changed and elect not to sleep, but
 instead to search again. See the section on the **jobs event counter** for more
 details.
 
-## The counters
+# The counters
 
 One of the key structs in the sleep module is `AtomicCounters`, found in
 `counters.rs`. It packs three counters into one atomically managed value:
@@ -57,7 +57,7 @@ One of the key structs in the sleep module is `AtomicCounters`, found in
 * The **jobs event counter**, which is used to signal when new work is available.
   It (sort of) tracks the number of jobs posted, but not quite, and it can rollover.
 
-### Thread counters
+## Thread counters
 
 There are two thread counters, one that tracks **inactive** threads and one that
 tracks **sleeping** threads. From this, one can deduce the number of threads
@@ -81,36 +81,77 @@ These counters are adjusted as follows:
 * When a thread finds work, exiting the idle state: decrement the inactive
   thread counter.
 
-### Jobs event counter
+## Jobs event counter
 
 The final counter is the **jobs event counter**. The role of this counter is to
-help sleepy threads detect when new work is posted in a lightweight fashion. The
-counter is incremented every time a new job is posted -- naturally, it can  also
-rollover if there have been enough jobs posted.
-
-The counter is used as follows:
-
-* When a thread gets **sleepy**, it reads the current value of the counter.
-* Later, before it goes to sleep, it checks if the counter has changed.
-  If it has, that indicates that work was posted but we somehow missed it
-  while searching. We'll go back and search again.
-
-Assuming no rollover, this protocol serves to prevent a race condition
-like so:
-
-* Thread A gets sleepy.
-* Thread A searches for work, finds nothing, and decides to go to sleep.
-* Thread B posts work, but sees no sleeping threads, and hence no one to wake up.
-* Thread A goes to sleep, incrementing the sleeping thread counter.
-
-However, because of rollover, the race condition cannot be completely thwarted.
-It is possible, if exceedingly unlikely, that Thread A will get sleepy and read
-a value of the JEC. And then, in between, there will be *just enough* activity
-from other threads to roll the JEC back over to precisely that old value. We
-have an extra check in the protocol to prevent deadlock in that (rather
-unlikely) case.
-
-## Protocol for a worker thread to fall asleep
+help sleepy threads detect when new work is posted in a lightweight fashion. In
+its simplest form, we would simply have a counter that gets incremented each
+time a new job is posted. This way, when a thread gets sleepy, it could read the
+counter, and then compare to see if the value has changed before it actually
+goes to sleep. But this [turns out to be too expensive] in practice, so we use a
+somewhat more complex scheme.
+
+[turns out to be too expensive]: https://github.com/rayon-rs/rayon/pull/746#issuecomment-624802747
+
+The idea is that the counter toggles between two states, depending on whether
+its value is even or odd (or, equivalently, on the value of its low bit):
+
+* Even -- If the low bit is zero, then it means that there has been no new work
+  since the last thread got sleepy.
+* Odd -- If the low bit is one, then it means that new work was posted since
+  the last thread got sleepy.
+
+### New work is posted
+
+When new work is posted, we check the value of the counter: if it is even,
+then we increment it by one, so that it becomes odd. In pseudocode:
+
+```rust
+fn new_work_posted() {
+    Fence(SeqCst); // See discussion later for the role of this fence
+    loop {
+        let value = Load(JEC, Relaxed);
+        if value is even {
+            if (Exchange(JEC, value, value+1, Relaxed)) {
+                break;
+            }
+        } else {
+            break;
+        }
+    }
+    // Post-condition: V is odd.
+}
+```
+
+### Worker thread gets sleepy
+
+When a worker thread gets sleepy, it will read the value of the counter. If the
+counter is odd, it will increment the counter so that it is even. Either way,
+it remembers the final value of the counter. This serves as
+a signal for anyone posting new work that they now must increment the counter.
+In pseudocode:
+
+```rust
+fn get_sleepy() {
+    let final_value = loop {
+        let value = Load(JEC, Relaxed);
+        if value is odd {
+            if (Exchange(JEC, value, value+1, Relaxed)) {
+                break value + 1;
+            }
+        } else {
+            break value;
+        }
+    };
+}
+```
+
+The final value `final_value` will be used later, when the thread is going to
+sleep. If at that time the counter is still equal to `final_value`, then we can
+assume no new jobs have been posted (though note the remote possibility of
+rollover, discussed in detail below).
+
+# Protocol for a worker thread to fall asleep
 
 The full protocol for a thread to fall asleep is as follows:
 
@@ -119,19 +160,20 @@ The full protocol for a thread to fall asleep is as follows:
   it searches all other work threads' queues, plus the 'injector queue' for
   work injected from the outside. If work is found in this search, the thread
   becomes active again and hence restarts this protocol from the top.
-* After a certain number of rounds, the thread "gets sleepy" and reads the JEC.
-  It does one more search for work.
+* After a certain number of rounds, the thread "gets sleepy" and executes `get_sleepy`
+  above, remembering the `final_value` of the JEC. It does one more search for work.
 * If no work is found, the thread atomically:
-  * Checks the JEC to see that it hasn't changed.
-    * If it has, then the thread returns to *just before* the "sleepy state" to
-      search again (i.e., it won't search for a full set of rounds, just a few
-      more times).
+  * Checks the JEC to see that it has not changed from `final_value`.
+    * If it has, then the thread goes back to searchin for work. We reset to
+      just before we got sleepy, so that we will do one more search
+      before attending to sleep again (rather than searching for many rounds).
   * Increments the number of sleeping threads by 1.
+* The thread then executes a seq-cst fence operation.
 * The thread then does one final check for injected jobs (see below). If any
   are available, it returns to the 'pre-sleepy' state as if the JEC had changed.
 * The thread waits to be signaled. Once signaled, it returns to the idle state.
 
-### The jobs event counter and deadlock
+## The jobs event counter and deadlock
 
 As described in the section on the JEC, the main concern around going to sleep
 is avoiding a race condition wherein:
@@ -156,23 +198,47 @@ asleep. Note that this final check occurs **after** the number of sleeping
 threads has been incremented. We are not concerned therefore with races against
 injections that occur after that increment, only before.
 
+Unfortunately, there is one rather subtle point concerning this final check:
+we wish to avoid the possibility that:
+
+* work is pushed into the injection queue by an outside thread X,
+* the sleepy thread S sees the JEC but it has rolled over and is equal
+* the sleepy thread S reads the injection queue but does not see the work posted by X.
+
+### The role of the seq-cst fences
+
+This is possible because the C++ memory model typically offers guarantees of the
+form "if you see the access A, then you must see those other accesses" -- but it
+doesn't guarantee that you will see the access A (i.e., if you think of
+processors with independent caches, you may be operating on very out of date
+cache state). 
+
+To overcome this problem, we have inserted two sequentially
+consistent fence operations into our algorithms above:
+
+* One fence occurs after work is posted into the injection queue, but before the
+  counters are read (including the number of sleeping threads).
+* One fence occurs after the number of sleeping threads is incremented, but
+  before the injection queue is read.
+
+### Proof sketch
+
 What follows is a "proof sketch" that the protocol is deadlock free. We model
 two relevant bits of memory, the job injector queue J and the atomic counters C.
 
 Consider the actions of the injecting thread:
 
 * PushJob: Job is injected, which can be modeled as an atomic write to J with release semantics.
-* IncJec: The JEC is incremented, which can be modeled as an atomic exchange to C with acquire-release semantics.
+* PushFence: A sequentially consistent fence is executed.
+* ReadSleepers: The counters C are read (they may also be incremented, but we just consider the read that comes first).
 
 Meanwhile, the sleepy thread does the following:
 
-* IncSleepers: The number of sleeping threads is incremented, which is atomic exchange to C with acquire-release semantics.
+* IncSleepers: The number of sleeping threads is incremented, which is atomic exchange to C.
+* SleepFence: A sequentially consistent fence is executed.
 * ReadJob: We look to see if the queue is empty, which is a read of J with acquire semantics.
 
-Both IncJec and IncSleepers modify the same memory location, and hence they must be fully ordered.
+Either PushFence or SleepFence must come first:
 
-* If IncSleepers came first, there is no problem, because the injecting thread
-  knows that everyone is asleep and it will wake up a thread.
-* If IncJec came first, then it "synchronizes with" IncSleepers.
-  * Therefore, PushJob "happens before" ReadJob, and so the write will be visible during
-    this final check, and the thread will not go to sleep.
+* If PushFence comes first, then PushJob must be visible to ReadJob.
+* If SleepFence comes first, then IncSleepers is visible to ReadSleepers.