Fairness

The new stealing mechanism can work with fairness, but works less well.  It is more likely that a steal will take the fairness token, and hence we get some less good behaviour.

src/rt/sched/core.h

@@ -18,7 +18,20 @@ namespace verona::rt
     size_t affinity = 0;
     WorkStealingQueue<4> q;
     std::atomic<Core*> next{nullptr};
-    std::atomic<bool> should_steal_for_fairness{false};
+
+    std::atomic<bool> should_steal_for_fairness{true};
+
+    /**
+     * @brief Create a token work object.  It is affinitised to the `this`
+     * core, and marks that stealing is required, for fairness.
+     */
+    Work* token_work{Closure::make([this](Work* w) {
+      this->should_steal_for_fairness = true;
+      // The token work is only deallocated during the destruction of the core.
+      // The destructor will run the token work, and return true, so that the
+      // closure code will run destructors and deallocate the memory.
+      return this->token_work == nullptr;
+    })};
 
     /// Progress and synchronization between the threads.
     //  These counters represent progress on a CPU core, not necessarily on
@@ -29,24 +42,14 @@ namespace verona::rt
 
     SchedulerStats stats;
 
-    /**
-     * @brief Create a token work object.  It is affinitised to the `home`
-     * core, and marks that stealing is required, for fairness. Once completed
-     * it reschedules itself on the home core.
-     */
-    Work* create_token_work(Core* home)
-    {
-      auto w = Closure::make([home](Work* w) {
-        home->should_steal_for_fairness = true;
-        home->q.enqueue(w);
-        return false;
-      });
-      return w;
-    }
-
   public:
     Core() : q{} {}
 
-    ~Core() {}
+    ~Core()
+    {
+      auto tw = token_work;
+      token_work = nullptr;
+      tw->run();
+    }
   };
 }

src/rt/sched/schedulerthread.h

@@ -147,14 +147,22 @@ namespace verona::rt
 
       if (core->should_steal_for_fairness)
       {
-        // Can race with other threads on the same core.
-        // This is a heuristic, so we don't care.
-        core->should_steal_for_fairness = false;
-        auto work = try_steal();
-        if (work != nullptr)
+        // Check if we have some work. We should only reschedule the token
+        // if we do have some work.  Otherwise, the token will be rescheduled
+        // and we will fail to reach quicescence.
+        if (!core->q.is_empty())
         {
-          return_next_work();
-          return work;
+          auto work = try_steal();
+          // Set the flag before rescheduling the token so that we don't have
+          // a race.
+          core->should_steal_for_fairness = false;
+          // Reschedule the token.
+          core->q.enqueue(core->token_work);
+          if (work != nullptr)
+          {
+            return_next_work();
+            return work;
+          }
         }
       }
 

test/func/fair_variance/fair_variance.cc

@@ -71,7 +71,7 @@ void assert_variance()
   {
     printf("cown[%d] took %f\n", i, elapsed_secs[i]);
   }
-  if ((max - min) / max > 0.15)
+  if ((max - min) / max > 0.35)
   {
     printf("(max - min) / max = %f\n", (max - min) / max);
     printf("variance too large");