From 83fc7866c9d7ae0718409c27366f314cfb2d43cf Mon Sep 17 00:00:00 2001
From: Richard Brooksby <rb@ravenbrook.com>
Date: Sun, 13 Mar 2016 13:26:24 +0000
Subject: [PATCH] Tidying up naming of write barrier deferral.

Copied from Perforce
 Change: 189960
 ServerID: perforce.ravenbrook.com
---
 mps/code/config.h | 28 ++++++++++++++++++++--------
 mps/code/mpmst.h  |  2 +-
 mps/code/seg.c    |  2 +-
 mps/code/trace.c  | 42 ++++++++++++++++++++++++------------------
 4 files changed, 46 insertions(+), 28 deletions(-)

diff --git a/mps/code/config.h b/mps/code/config.h
index 50cb0aece82..5204171795b 100644
--- a/mps/code/config.h
+++ b/mps/code/config.h
@@ -659,20 +659,32 @@
     { 36 * 1024, 0.45 }  /* second gen, after which dynamic */ \
   }
 
-/* Experimental Scan Barrier threshold
+/* Write barrier deferral
  *
+ * Defer using the write barrier for the remembered set until a number
+ * of unnecessary scans have been performed on a segment.  Avoids
+ * memory protection costs when scanning might be cheaper.  See job003975.
+ *
+ * TODO: These settings were determined by trial and error, but should
+ * be based on measurement of the protection overhead on each
+ * platform.  We know it's extremely different between OS X and
+ * Windows, for example.
+ *
+ * TODO: Consider basing the count on the amount of time that has
+ * passed in the mutator rather than the number of scans.
  */
-/* Number of bits needed to keep the seg scan count */
-#define SEG_SCANS_BITS 10
+
+/* Number of bits needed to keep the write barrier deferral count */
+#define WB_DEFER_BITS 3
 /* The number of unecessary scans performed, before raising the write
- * barrier to remember the refset summary. */
-#define SEG_SCANS_INIT 3
+   barrier to maintian the remembered set. */
+#define WB_DEFER_INIT 3
 /* The number of unecessary scans performed, before raising the write
- * barrier to remember the refset summary, after a necessary scan */
-#define SEG_SCANS_AFTER_NEEDED_SCAN 3
+   barrier to remember the refset summary, after a necessary scan */
+#define WB_DEFER_DELAY WB_DEFER_INIT
 /* The number of unecessary scans performed, before raising the write
  * barrier to remember the refset summary, after a barrier hit */
-#define SEG_SCANS_AFTER_HIT 1
+#define WB_DEFER_AFTER_HIT 1
 
 #endif /* config_h */
 
diff --git a/mps/code/mpmst.h b/mps/code/mpmst.h
index 5636bf22b28..48f80eb2255 100644
--- a/mps/code/mpmst.h
+++ b/mps/code/mpmst.h
@@ -281,7 +281,7 @@ typedef struct SegStruct {      /* segment structure */
   TraceSet white : TraceLIMIT;  /* traces for which seg is white */
   TraceSet nailed : TraceLIMIT; /* traces for which seg has nailed objects */
   RankSet rankSet : RankLIMIT;  /* ranks of references in this seg */
-  unsigned scans : SEG_SCANS_BITS; /* use write barrier after this many scans */
+  Count defer : WB_DEFER_BITS;  /* defer write barrier for this many scans */
 } SegStruct;
 
 
diff --git a/mps/code/seg.c b/mps/code/seg.c
index 1e693d8a48d..31ef5bd8b64 100644
--- a/mps/code/seg.c
+++ b/mps/code/seg.c
@@ -160,7 +160,7 @@ static Res SegInit(Seg seg, Pool pool, Addr base, Size size,
   seg->grey = TraceSetEMPTY;
   seg->pm = AccessSetEMPTY;
   seg->sm = AccessSetEMPTY;
-  seg->scans = SEG_SCANS_INIT;
+  seg->defer = WB_DEFER_INIT;
   seg->depth = 0;
   seg->firstTract = NULL;
 
diff --git a/mps/code/trace.c b/mps/code/trace.c
index f8a9494067f..bd3175fc5aa 100644
--- a/mps/code/trace.c
+++ b/mps/code/trace.c
@@ -1089,6 +1089,7 @@ static Res traceScanSegRes(TraceSet ts, Rank rank, Arena arena, Seg seg)
   Bool wasTotal;
   ZoneSet white;
   Res res;
+  RefSet summary;
 
   /* The reason for scanning a segment is that it's grey. */
   AVER(TraceSetInter(ts, SegGrey(seg)) != TraceSetEMPTY);
@@ -1134,28 +1135,33 @@ static Res traceScanSegRes(TraceSet ts, Rank rank, Arena arena, Seg seg)
      * scan, consistent with the recorded SegSummary?
      */
     AVER(RefSetSub(ScanStateUnfixedSummary(ss), SegSummary(seg)));
+
+    /* Remembered set and write barrier */
+    /* Was the scan necessary?  Did the segment refer to the white set */
     if (ZoneSetInter(ScanStateUnfixedSummary(ss), white) == ZoneSetEMPTY) {
-      /* a scan was not necessary */
-      if (seg->scans > 0)
-        seg->scans--;
+      if (seg->defer > 0)
+        --seg->defer;
     } else {
-      if (seg->scans < SEG_SCANS_AFTER_NEEDED_SCAN)
-        seg->scans = SEG_SCANS_AFTER_NEEDED_SCAN;
+      if (seg->defer < WB_DEFER_DELAY)
+        seg->defer = WB_DEFER_DELAY;
     }
-    
-    if (seg->scans == 0) {
-      if(res != ResOK || !wasTotal) {
-        /* scan was partial, so... */
-        /* scanned summary should be ORed into segment summary. */
-        SegSetSummary(seg, RefSetUnion(SegSummary(seg), ScanStateSummary(ss)));
-      } else {
-        /* all objects on segment have been scanned, so... */
-        /* scanned summary should replace the segment summary. */
-        SegSetSummary(seg, ScanStateSummary(ss));
-      }
+
+    /* Only apply the write barrier if it is not deferred. */
+    /* TODO: This discards information we collected during
+       scanning. Consider keeping the summary but changing the
+       invariant on shielding instead. */
+    if (seg->defer == 0) {
+      /* If we scanned every reference in the segment then we have a
+         complete summary we can set. Otherwise, we just have
+         information about more zones that the segment refers to. */
+      if (res == ResOK && wasTotal)
+        summary = ScanStateSummary(ss);
+      else
+        summary = RefSetUnion(SegSummary(seg), ScanStateSummary(ss));
     } else {
-      SegSetSummary(seg, RefSetUNIV);
+      summary = RefSetUNIV;
     }
+    SegSetSummary(seg, summary);
 
     ScanStateFinish(ss);
   }
@@ -1215,7 +1221,7 @@ void TraceSegAccess(Arena arena, Seg seg, AccessSet mode)
   EVENT3(TraceAccess, arena, seg, mode);
 
   if ((mode & SegSM(seg) & AccessWRITE) != 0)     /* write barrier? */
-    seg->scans = SEG_SCANS_AFTER_HIT;
+    seg->defer = WB_DEFER_AFTER_HIT;
 
   if((mode & SegSM(seg) & AccessREAD) != 0) {   /* read barrier? */
     Trace trace;