Fix issues identified in review by gdr <https://info.ravenbrook.com/mail/2018/07/16/13-54-48/0/>

Copied from Perforce
 Change: 194674

mps/code/config.h

@@ -376,14 +376,6 @@
 #define LO_GEN_DEFAULT       0
 
 
-/* Pool MV Configuration -- see <code/poolmv.c> */
-
-#define MV_ALIGN_DEFAULT      MPS_PF_ALIGN
-#define MV_EXTEND_BY_DEFAULT  ((Size)65536)
-#define MV_AVG_SIZE_DEFAULT   ((Size)32)
-#define MV_MAX_SIZE_DEFAULT   ((Size)65536)
-
-
 /* Pool MFS Configuration -- see <code/poolmfs.c> */
 
 #define MFS_EXTEND_BY_DEFAULT ((Size)65536)

mps/code/djbench.c

@@ -232,8 +232,7 @@ static struct {
 } pools[] = {
   {"mvt",   arena_wrap, dj_reserve, mps_class_mvt},
   {"mvff",  arena_wrap, dj_reserve, mps_class_mvff},
-  {"mv",    arena_wrap, dj_alloc,   mps_class_mv},
-  {"mvb",   arena_wrap, dj_reserve, mps_class_mv}, /* mv with buffers */
+  {"mvffa", arena_wrap, dj_alloc,   mps_class_mvff}, /* mvff with alloc */
   {"an",    wrap,       dj_malloc,  dummy_class},
 };
 

mps/code/locusss.c

@@ -5,7 +5,6 @@
  */
 
 #include "mpscmvff.h"
-#include "mpscmv.h"
 #include "mpslib.h"
 #include "mpsavm.h"
 #include "testlib.h"
@@ -169,7 +168,7 @@ static void testInArena(mps_arena_t arena,
                       FALSE, FALSE, TRUE),
       "Create LO MFFV");
 
-  die(mps_pool_create_k(&temppool, arena, mps_class_mv(),
+  die(mps_pool_create_k(&temppool, arena, mps_class_mvff(),
                         mps_args_none),
       "Create TEMP");
 

mps/code/mpsicv.c

@@ -9,7 +9,7 @@
 #include "mpslib.h"
 #include "mpscamc.h"
 #include "mpsavm.h"
-#include "mpscmv.h"
+#include "mpscmvff.h"
 #include "fmthe.h"
 #include "fmtdy.h"
 #include "fmtdytst.h"
@@ -100,7 +100,7 @@ static void alignmentTest(mps_arena_t arena)
     MPS_ARGS_ADD(args, MPS_KEY_EXTEND_BY, 0x1000);
     MPS_ARGS_ADD(args, MPS_KEY_MEAN_SIZE, 1024);
     MPS_ARGS_ADD(args, MPS_KEY_MAX_SIZE, 16384);
-    die(mps_pool_create_k(&pool, arena, mps_class_mv(), args),
+    die(mps_pool_create_k(&pool, arena, mps_class_mvff(), args),
         "alignment pool create");
   } MPS_ARGS_END(args);
   
@@ -305,7 +305,7 @@ static mps_res_t root_single(mps_ss_t ss, void *p, size_t s)
  * incidentally tests:
  *   mps_alloc
  *   mps_arena_commit_limit_set
- *   mps_class_mv
+ *   mps_class_mvff
  *   mps_pool_create
  *   mps_pool_destroy
  */
@@ -323,7 +323,7 @@ static void arena_commit_test(mps_arena_t arena)
     MPS_ARGS_ADD(args, MPS_KEY_EXTEND_BY, 0x1000);
     MPS_ARGS_ADD(args, MPS_KEY_MEAN_SIZE, 1024);
     MPS_ARGS_ADD(args, MPS_KEY_MAX_SIZE, 16384);
-    die(mps_pool_create_k(&pool, arena, mps_class_mv(), args),
+    die(mps_pool_create_k(&pool, arena, mps_class_mvff(), args),
         "commit pool create");
   } MPS_ARGS_END(args);
   
@@ -384,7 +384,7 @@ static void *test(void *arg, size_t s)
     MPS_ARGS_ADD(args, MPS_KEY_EXTEND_BY, 0x10000);
     MPS_ARGS_ADD(args, MPS_KEY_MEAN_SIZE, 32);
     MPS_ARGS_ADD(args, MPS_KEY_MAX_SIZE, 0x10000);
-    die(mps_pool_create_k(&mv, arena, mps_class_mv(), args),
+    die(mps_pool_create_k(&mv, arena, mps_class_mvff(), args),
         "pool_create(mv)");
   } MPS_ARGS_END(args);
 

mps/code/poolmvff.c

@@ -440,7 +440,7 @@ static Res MVFFInit(Pool pool, Arena arena, PoolClass klass, ArgList args)
   AVER(pool != NULL);
   AVERT(Arena, arena);
   AVERT(ArgList, args);
-  UNUSED(klass); /* used for debug pools only */
+  AVERC(PoolClass, klass);
 
   /* .arg: class-specific additional arguments; see */
   /* <design/poolmvff/#method.init> */
@@ -759,25 +759,6 @@ mps_pool_class_t mps_class_mvff_debug(void)
 }
 
 
-/* Replacement of the deprecated MV pool class.
- *
- * MVFF replaces MV, but these functions are provided for backward
- * compatibility.  TODO: Remove these sometime after MPS 1.116.
- */
-
-mps_pool_class_t mps_class_mv(void)
-{
-  /* return (mps_pool_class_t)(EnsureMVPoolClass()); */
-  return mps_class_mvff();
-}
-
-mps_pool_class_t mps_class_mv_debug(void)
-{
-  /* return (mps_pool_class_t)(EnsureMVDebugPoolClass()); */
-  return mps_class_mvff_debug();
-}
-
-
 /* MVFFCheck -- check the consistency of an MVFF structure */
 
 Bool MVFFCheck(MVFF mvff)
@@ -805,6 +786,81 @@ Bool MVFFCheck(MVFF mvff)
 }
 
 
+/* Replacement of the deprecated MV pool class.
+ *
+ * MVFF replaces MV, but these functions are provided for backward
+ * compatibility.  TODO: Remove these sometime after MPS 1.117.
+ */
+
+DECLARE_CLASS(Pool, MVPool, MVFFPool);
+DECLARE_CLASS(Pool, MVDebugPool, MVPool);
+
+static Res mvInit(Pool pool, Arena arena, PoolClass klass, ArgList args)
+{
+  Index i;
+  
+  AVER(pool != NULL);
+  AVERT(Arena, arena);
+  AVERC(PoolClass, klass);
+  AVERT(ArgList, args);
+
+  /* MV allows arbitrary alignment but MVFF does not, so round up. */
+  for (i = 0; args[i].key != MPS_KEY_ARGS_END; ++i)
+    if (args[i].key == MPS_KEY_ALIGN
+        && args[i].val.align < FreelistMinimumAlignment)
+      args[i].val.align = FreelistMinimumAlignment;
+
+  return NextMethod(Pool, MVPool, init)(pool, arena, klass, args);
+}
+
+static void mvVarargs(ArgStruct args[MPS_ARGS_MAX], va_list varargs)
+{
+  args[0].key = MPS_KEY_EXTEND_BY;
+  args[0].val.size = va_arg(varargs, Size);
+  args[1].key = MPS_KEY_MEAN_SIZE;
+  args[1].val.size = va_arg(varargs, Size);
+  args[2].key = MPS_KEY_MAX_SIZE;
+  args[2].val.size = va_arg(varargs, Size);
+  args[3].key = MPS_KEY_ARGS_END;
+  AVERT(ArgList, args);
+}
+
+static void mvDebugVarargs(ArgStruct args[MPS_ARGS_MAX], va_list varargs)
+{
+  args[0].key = MPS_KEY_POOL_DEBUG_OPTIONS;
+  args[0].val.pool_debug_options = va_arg(varargs, mps_pool_debug_option_s *);
+  mvVarargs(args + 1, varargs);
+}
+
+DEFINE_CLASS(Pool, MVPool, klass)
+{
+  INHERIT_CLASS(klass, MVPool, MVFFPool);
+  klass->init = mvInit;
+  klass->varargs = mvVarargs;
+  AVERT(PoolClass, klass);
+}
+
+DEFINE_CLASS(Pool, MVDebugPool, klass)
+{
+  INHERIT_CLASS(klass, MVDebugPool, MVPool);
+  PoolClassMixInDebug(klass);
+  klass->size = sizeof(MVFFDebugStruct);
+  klass->varargs = mvDebugVarargs;
+  klass->debugMixin = MVFFDebugMixin;
+  AVERT(PoolClass, klass);
+}
+
+mps_pool_class_t mps_class_mv(void)
+{
+  return CLASS(MVPool);
+}
+
+mps_pool_class_t mps_class_mv_debug(void)
+{
+  return CLASS(MVDebugPool);
+}
+
+
 /* C. COPYRIGHT AND LICENSE
  *
  * Copyright (C) 2001-2016 Ravenbrook Limited <http://www.ravenbrook.com/>.

mps/code/qs.c

@@ -27,7 +27,7 @@
 #include "mps.h"
 #include "mpsavm.h"
 #include "mpscamc.h"
-#include "mpscmv.h"
+#include "mpscmvff.h"
 #include "mpstd.h"
 
 #include <stdio.h> /* printf */
@@ -335,13 +335,8 @@ static void *go(void *p, size_t s)
   testlib_unused(p);
   testlib_unused(s);
 
-  MPS_ARGS_BEGIN(args) {
-    MPS_ARGS_ADD(args, MPS_KEY_EXTEND_BY, 65536);
-    MPS_ARGS_ADD(args, MPS_KEY_MEAN_SIZE, sizeof(QSCellStruct) * 1000);
-    MPS_ARGS_ADD(args, MPS_KEY_MAX_SIZE, 65536);
-    die(mps_pool_create_k(&mpool, arena, mps_class_mv(), args),
-        "MVCreate");
-  } MPS_ARGS_END(args);
+  die(mps_pool_create_k(&mpool, arena, mps_class_mvff(), mps_args_none),
+      "pool create");
 
   die(mps_fmt_create_A(&format, arena, &fmt_A_s), "FormatCreate");
   die(mps_chain_create(&chain, arena, genCOUNT, testChain), "chain_create");

mps/design/guide.impl.c.format.txt

@@ -153,9 +153,7 @@ Some examples::
 
 _`.indent.goto-label`: Place each goto-label on a line of its own,
 outdented to the same level as the surrounding block. Then indent the
-non-label part of the statement normally.
-
-::
+non-label part of the statement normally. ::
 
     result foo(void)
     {
@@ -181,22 +179,22 @@ _`.indent.cont.parens`: if you break a statement inside a parameter list or
 other parenthesized expression, indent so that the continuation lines up
 just after the open parenthesis. For example::
 
-    PoolClassInit(&PoolClassMVStruct,
-                  "MV", init, finish, allocP, freeP,
-                  NULL, NULL, describe, isValid);
+    res = ChunkInit(chunk, arena, alignedBase,
+                    AddrAlignDown(limit, ArenaGrainSize(arena)),
+                    AddrOffset(base, limit), boot);
 
 _`.indent.cont.expr`: Note that when breaking an expression it is clearer
 to place the operator at the start of the continuation line::
 
-    CHECKL(AddrAdd((Addr)chunk->pageTableMapped,
-                   BTSize(chunk->pageTablePages))
-           <= AddrAdd(chunk->base, chunk->ullageSize));
+    CHECKL(AddrAdd((Addr)chunk->allocTable, BTSize(chunk->pages))
+           <= PageIndexBase(chunk, chunk->allocBase));
 
 This is particularly useful in long conditional expressions that use &&
 and ||. For example::
 
-    } while(trace->state != TraceFINISHED
-            && (trace->emergency || traceWorkClock(trace) < pollEnd));
+    if (BufferRankSet(buffer) != RankSetEMPTY
+        && (buffer->mode & BufferModeFLIPPED) == 0
+        && !BufferIsReset(buffer))
 
 _`.indent.hint`: Usually, it is possible to determine the correct
 indentation for a line by looking to see if the previous line ends with
@@ -212,21 +210,13 @@ open brace after the control word or expression, separated by a space,
 and when there is an else, places that after the close brace. For
 example::
 
-    if(isBase) {
-      new->base = limit;
-      new->limit = block->limit;
-      block->limit = base;
-      new->next = block->next;
-      block->next = new;
+    if (buffer->mode & BufferModeFLIPPED) {
+      return buffer->initAtFlip;
     } else {
-      new->base = block->base;
-      new->limit = base;
-      block->base = limit;
-      new->next = block;
-      *prev = new;
+      return buffer->ap_s.init;
     }
 
-The same applies to struct, enum, union.
+The same applies to ``struct``, ``enum``, and ``union``.
 
 _`.brace.otb.function.not`: OTB is never used for function definitions.
 
@@ -248,42 +238,34 @@ Switch statements
 
 _`.switch`: format switch statements like this::
 
-    switch (action) {
-    case WIBBLE:
-    case WOBBLE:
-      {
-        int angle;
-        err = move(plate, action, &angle);
-      }
-      break;
-
-    case QUIET:
-      drop();
-      /* fall-through */
-
-    case QUIESCENT:
-      err = 0;
-      break;
-
+    switch (SplaySplay(splay, oldKey, splay->compare)) {
     default:
       NOTREACHED;
-      break;
+      /* fall through */
+    case CompareLESS:
+      return SplayTreeRoot(splay);
+
+    case CompareGREATER:
+    case CompareEQUAL:
+      return SplayTreeSuccessor(splay);
     }
 
 The component rules that result in this style are:
 
 _`.switch.break`: The last line of every case-clause body must be an
 unconditional jump statement (usually ``break``, but may be ``goto``,
-``continue``, or ``return``), or if a fall-through is intended, the comment
-``/* fall-through */``. (Note: if the unconditional jump should never be
-taken, because of previous conditional jumps, use ``NOTREACHED`` on the
-line before it). This rule is to prevent accidental fall-throughs, even
-if someone makes a editing mistake that causes a conditional jump to be
-missed.
+``continue``, or ``return``), or if a fall-through is intended, the
+comment ``/* fall through */``. (Note: if the unconditional jump
+should never be taken, because of previous conditional jumps, use
+``NOTREACHED`` on the line before it.) This rule is to prevent
+accidental fall-throughs, even if someone makes a editing mistake that
+causes a conditional jump to be missed. This rule is automatically
+checked by GCC and Clang with the ``-Wimplicit-fallthrough`` option.
 
-_`.switch.default`: It is usually a good idea to have a default-clause,
-even if all it contains is ``NOTREACHED`` and ``break``.  Remember that
-``NOTREACHED`` doesn't stop the process in all build varieties.
+_`.switch.default`: It is usually a good idea to have a
+default-clause, even if all it contains is ``NOTREACHED`` and
+``break`` or ``/* fall through */``. Remember that ``NOTREACHED``
+doesn't stop the process in all build varieties.
 
 
 Comments
@@ -330,16 +312,22 @@ union, or enum declarations. They should start at least at column 32
 descriptive text. Abandon English sentence structure if this makes the
 comment clearer. Don't write more than one line. Here's an example::
 
-      typedef struct PoolMVStruct {
-        Pool blockPool;           /* for block descriptors */
-        Pool spanPool;            /* for span descriptors */
-        size_t extendBy;          /* size to extend pool by */
-        size_t avgSize;           /* estimate of allocation size */
-        size_t maxSize;           /* estimate of maximum size */
-        Addr space;               /* total free space in pool */
-        Addr lost;                /* lost when free can't allocate */
-        struct SpanStruct *spans; /* span chain */
-      } PoolMVStruct;
+    typedef struct MVFFStruct {     /* MVFF pool outer structure */
+      PoolStruct poolStruct;        /* generic structure */
+      LocusPrefStruct locusPrefStruct; /* the preferences for allocation */
+      Size extendBy;                /* size to extend pool by */
+      Size avgSize;                 /* client estimate of allocation size */
+      double spare;                 /* spare space fraction, see MVFFReduce */
+      MFSStruct cbsBlockPoolStruct; /* stores blocks for CBSs */
+      CBSStruct totalCBSStruct;     /* all memory allocated from the arena */
+      CBSStruct freeCBSStruct;      /* free memory (primary) */
+      FreelistStruct flStruct;      /* free memory (secondary, for emergencies) */
+      FailoverStruct foStruct;      /* free memory (fail-over mechanism) */
+      Bool firstFit;                /* as opposed to last fit */
+      Bool slotHigh;                /* prefers high part of large block */
+      Sig sig;                      /* <design/sig/> */
+    } MVFFStruct;
+
 
 Macros
 ......

mps/design/pool.txt

@@ -27,7 +27,7 @@ _`.class`: Each pool belongs to a *pool class*.
 _`.class.name`: Each pool class has a short, pithy, cryptic name for
 the pool class. It should start with ``"A"`` (for "automatic") if
 memory is managed by the garbage collector, and ``"M"`` (for "manual")
-if memory is managed by alloc/free. For example, "AMC", "MV".
+if memory is managed by alloc/free. For example, "AMC", "MVFF".
 
 _`.class.protocol`: Pool classes use the *protocol* mechanisms (see
 design.mps.protocol_) to implement class initialization and

mps/design/poolmvt.txt

@@ -28,12 +28,10 @@ _`.readership`: MM developers
 
 _`.source`: req.dylan(6), req.epcore(16), req.product(2)
 
-_`.background`: design.mps.poolmv_, design.mps.poolepdl(0),
+_`.background`: design.mps.poolmv, design.mps.poolepdl(0),
 design.product.soft.drop(0), paper.wil95(1), paper.vo96(0),
 paper.grun92(1), paper.beck82(0), `mail.ptw.1998-02-25.22-18`_.
 
-.. _design.mps.poolmv: poolmv
-
 .. _mail.ptw.1998-02-25.22-18: https://info.ravenbrook.com/project/mps/mail/1998/02/25/22-18/0.txt
 
 
@@ -956,12 +954,10 @@ _`.test.component`: Components `.impl.c.splay`_, `.impl.c.cbs`_, and
 `.impl.c.abq`_ will be subjected to individual component tests to
 verify their functionality.
 
-_`.test.regression`: All tests applied to MV (design.mps.poolmv_) and
+_`.test.regression`: All tests applied to MV (design.mps.poolmv) and
 EPDL (design.mps.poolepdl(0)) will be applied to poolmvt to ensure
 that mvt is at least as functional as the pools it is replacing.
 
-.. _design.mps.poolmv: poolmv
-
 _`.test.qa`: Once poolmvt is integrated into the MPS, the standard MPS
 QA tests will be applied to poolmvt prior to each release.
 

mps/manual/source/code-index.rst

@@ -23,7 +23,7 @@ mpscams.h    :ref:`pool-ams` pool class external interface.
 mpscawl.h    :ref:`pool-awl` pool class external interface.
 mpsclo.h     :ref:`pool-lo` pool class external interface.
 mpscmfs.h    :ref:`pool-mfs` pool class external interface.
-mpscmv.h     :ref:`pool-mv` pool class external interface.
+mpscmv.h     Deprecated MV (Manual Variable) pool class external interface.
 mpscmv2.h    Former (deprecated) :ref:`pool-mvt` pool class interface.
 mpscmvff.h   :ref:`pool-mvff` pool class external interface.
 mpscmvt.h    :ref:`pool-mvt` pool class external interface.
@@ -231,7 +231,6 @@ poolawl.c    :ref:`pool-awl` implementation.
 poollo.c     :ref:`pool-lo` implementation.
 poolmfs.c    :ref:`pool-mfs` implementation.
 poolmfs.h    :ref:`pool-mfs` internal interface.
-poolmv.c     :ref:`pool-mv` implementation.
 poolmv2.c    :ref:`pool-amc` implementation.
 poolmv2.h    :ref:`pool-mvt` internal interface.
 poolmvff.c   :ref:`pool-mvff` implementation.

mps/manual/source/release.rst

@@ -28,7 +28,7 @@ New features
 Interface changes
 .................
 
-#. The pool class :ref:`pool-mv` is now deprecated.
+#. The pool class MV (Manual Variable) is now deprecated.
 
 
 Other changes
@@ -217,7 +217,7 @@ New features
 Interface changes
 .................
 
-#. The pool class :ref:`pool-mv` is no longer deprecated.
+#. The pool class MV (Manual Variable) is no longer deprecated.
 
 #. The type of pool classes is now :c:type:`mps_pool_class_t`. The old
    name :c:type:`mps_class_t` is still available via a ``typedef``,
@@ -293,8 +293,8 @@ Other changes
    .. _job003865: https://www.ravenbrook.com/project/mps/issue/job003865/
 
 #. :c:func:`mps_arena_has_addr` now returns the correct result for
-   objects allocated from the :ref:`pool-mfs`, :ref:`pool-mv`, and
-   :ref:`pool-mvff` pools. See job003866_.
+   objects allocated from the :ref:`pool-mfs`, MV (Manual Variable),
+   and :ref:`pool-mvff` pools. See job003866_.
 
    .. _job003866: https://www.ravenbrook.com/project/mps/issue/job003866/
 
@@ -402,8 +402,9 @@ Interface changes
    (meaning that there is no dependent object).
 
 #. It is now possible to configure the alignment of objects allocated
-   in a :ref:`pool-mv` pool, by passing the :c:macro:`MPS_KEY_ALIGN`
-   keyword argument to :c:func:`mps_pool_create_k`.
+   in an MV (Manual Variable) pool, by passing the
+   :c:macro:`MPS_KEY_ALIGN` keyword argument to
+   :c:func:`mps_pool_create_k`.
 
 #. The :ref:`pool-mvff` pool class takes a new keyword argument
    :c:macro:`MPS_KEY_SPARE`. This specifies the maximum proportion of
@@ -722,7 +723,7 @@ Interface changes
    :c:func:`mps_telemetry_control`, which is now deprecated. See
    :ref:`topic-telemetry`.
 
-#. The pool classes :ref:`pool-mv` and :ref:`pool-snc` are now
+#. The pool classes MV (Manual Variable) and :ref:`pool-snc` are now
    deprecated.
 
 #. Allocation frames are now deprecated. See :ref:`topic-frame`.

mps/test/argerr/41.c

@@ -6,7 +6,7 @@ TEST_HEADER
  link = testlib.o
 OUTPUT_SPEC
  assert = true
- assertfile P= poolmv.c
+ assertfile P= poolmff.c
  assertcond = extendBy > 0
 END_HEADER
 */

mps/test/argerr/42.c

@@ -6,7 +6,7 @@ TEST_HEADER
  link = testlib.o
 OUTPUT_SPEC
  assert = true
- assertfile P= poolmv.c
+ assertfile P= poolmvff.c
  assertcond = avgSize > 0
 END_HEADER
 */

mps/test/argerr/43.c

@@ -6,7 +6,7 @@ TEST_HEADER
  link = testlib.o
 OUTPUT_SPEC
  assert = true
- assertfile P= poolmv.c
+ assertfile P= poolmvff.c
  assertcond = maxSize > 0
 END_HEADER
 */

mps/test/argerr/44.c

@@ -6,7 +6,7 @@ TEST_HEADER
  link = testlib.o
 OUTPUT_SPEC
  assert = true
- assertfile P= poolmv.c
+ assertfile P= poolmvff.c
  assertcond = extendBy <= maxSize
 END_HEADER
 */

mps/test/conerr/59.c

@@ -6,7 +6,7 @@ TEST_HEADER
  link = testlib.o
 OUTPUT_SPEC
  assert = true
- assertfile P= poolmv.c
+ assertfile P= poolmvff.c
  assertcond = unreachable code
 END_HEADER
 */