Separating flag for segment in shield cache from depth, clarifying code and optimising segfree somewhat.

Copied from Perforce
 Change: 190194
 ServerID: perforce.ravenbrook.com
This commit is contained in:
Richard Brooksby 2016-03-19 13:42:09 +00:00
parent 2c69ff0ce5
commit ff69c9af04
6 changed files with 116 additions and 85 deletions

View file

@ -39,6 +39,15 @@
#include "mpslib.h"
/* FIXME: to config.h? */
#if defined(MPS_BUILD_LL) || defined(MPS_BUILD_GC)
#define UNEXPECTED(expr) __builtin_expect((expr) != 0, TRUE)
#else
#define UNEXPECTED(expr) (expr)
#endif
/* ASSERT -- basic assertion
*
* The ASSERT macro is equivalent to the ISO C assert() except that it is
@ -51,7 +60,7 @@
#define ASSERT(cond, condstring) \
BEGIN \
if (cond) NOOP; else \
if (UNEXPECTED(cond)) NOOP; else \
mps_lib_assert_fail(MPS_FILE, __LINE__, (condstring)); \
END

View file

@ -175,15 +175,13 @@ Bool GlobalsCheck(Globals arenaGlobals)
/* This is too expensive to check all the time since we have an
expanding shield cache that often has 16K elements instead of
16. */
#ifdef AVER_AND_CHECK_ALL
#if defined(AVER_AND_CHECK_ALL)
{
Count depth = 0;
for (i = 0; i < arena->shCacheLimit; ++i) {
Seg seg = arena->shCache[i];
if (seg != NULL) {
CHECKD(Seg, seg);
depth += SegDepth(seg);
}
CHECKD(Seg, seg);
depth += SegDepth(seg);
}
CHECKL(depth <= arena->shDepth);
}

View file

@ -253,7 +253,8 @@ typedef struct SegStruct { /* segment structure */
Tract firstTract; /* first tract of segment */
RingStruct poolRing; /* link in list of segs in pool */
Addr limit; /* limit of segment */
unsigned depth : ShieldDepthWIDTH; /* see <code/shield.c#def.depth> */
unsigned depth : ShieldDepthWIDTH; /* see design.mps.shield.def.depth */
BOOLFIELD(cached); /* in shield cache? */
AccessSet pm : AccessLIMIT; /* protection mode, <code/shield.c> */
AccessSet sm : AccessLIMIT; /* shield mode, <code/shield.c> */
TraceSet grey : TraceLIMIT; /* traces for which seg is grey */

View file

@ -150,6 +150,7 @@ static Res SegInit(Seg seg, Pool pool, Addr base, Size size, ArgList args)
seg->pm = AccessSetEMPTY;
seg->sm = AccessSetEMPTY;
seg->depth = 0;
seg->cached = FALSE;
seg->firstTract = NULL;
seg->sig = SegSig; /* set sig now so tract checks will see it */
@ -220,8 +221,10 @@ static void SegFinish(Seg seg)
seg->rankSet = RankSetEMPTY;
/* See <code/shield.c#shield.flush> */
if (seg->depth > 0)
AVER(seg->depth == 0);
if (seg->cached)
ShieldFlush(PoolArena(SegPool(seg)));
AVER(seg->cached == FALSE);
limit = SegLimit(seg);
@ -713,17 +716,9 @@ Bool SegCheck(Seg seg)
(design.mps.shield.inv.prot.shield). */
CHECKL(BS_DIFF(seg->pm, seg->sm) == 0);
/* An exposed segment is not protected
(design.mps.shield.inv.expose.prot). FIXME: Discovered to be
FALSE when raising the read barrier on a write-protected
segment. RB 2016-03-19 */
/* CHECKL(seg->depth == 0 || seg->pm == 0); */
/* FIXME: Wouldn't it be better to have a flag for "in cache" so
that depth > 0 => exposed? */
/* All unsynced segments have positive depth
/* All unsynced segments have positive depth or are in the cache
(design.mps.shield.inv.unsynced.depth). */
CHECKL(seg->sm == seg->pm || seg->depth > 0);
CHECKL(seg->sm == seg->pm || seg->depth > 0 || seg->cached);
CHECKL(RankSetCheck(seg->rankSet));
if (seg->rankSet == RankSetEMPTY) {

View file

@ -25,6 +25,12 @@ static Bool shieldSegIsSynced(Seg seg)
return SegSM(seg) == SegPM(seg);
}
static Bool SegIsExposed(Seg seg)
{
AVER_CRITICAL(TESTT(Seg, seg));
return seg->depth > 0;
}
/* shieldSync -- synchronize a segment's protection
*
@ -77,6 +83,7 @@ void (ShieldResume)(Arena arena)
AVER(arena->insideShield);
AVER(arena->suspended);
/* It is only correct to actually resume the mutator here if shDepth is 0 */
/* TODO: Consider actually doing that. */
}
@ -89,34 +96,35 @@ static void protLower(Arena arena, Seg seg, AccessSet mode)
AVER_CRITICAL(TESTT(Seg, seg));
AVERT_CRITICAL(AccessSet, mode);
if (SegPM(seg) & mode) {
SegSetPM(seg, SegPM(seg) & ~mode);
if (BS_INTER(SegPM(seg), mode) != 0) {
SegSetPM(seg, BS_DIFF(SegPM(seg), mode));
ProtSet(SegBase(seg), SegLimit(seg), SegPM(seg));
}
}
static void shieldFlushEntry(Arena arena, Size i)
/* shieldFlushEntry -- flush a single entry from the cache */
static void shieldFlushEntry(Arena arena, Index i)
{
Seg seg;
AVERT(Arena, arena);
AVER(i < arena->shCacheLimit);
AVERT(Arena, arena);
seg = arena->shCache[i];
AVERT(Seg, seg);
AVER(arena->shDepth > 0);
AVER(SegDepth(seg) > 0);
--arena->shDepth;
SegSetDepth(seg, SegDepth(seg) - 1);
AVER(i < arena->shCacheLimit);
AVER(seg->cached);
if (SegDepth(seg) == 0)
if (!SegIsExposed(seg))
shieldSync(arena, seg);
arena->shCache[i] = NULL; /* just to make sure it gets overwritten */
arena->shCache[i] = NULL; /* just to make sure it can't be reused */
}
/* shieldCacheEntryCompare -- comparison for cache sorting */
static Compare shieldCacheEntryCompare(void *left, void *right, void *closure)
{
Seg segA = left, segB = right;
@ -162,32 +170,25 @@ static void shieldFlushEntries(Arena arena)
{
Addr base = NULL, limit = NULL;
AccessSet mode;
Seg seg;
Size i;
Index i;
if (arena->shDepth == 0) {
AVER(arena->shCacheLimit == 0);
if (arena->shCacheLength == 0) {
AVER(arena->shCache == NULL);
return;
}
AVER(arena->shCache != NULL);
AVER(arena->shCacheLength > 0);
QuickSort((void *)arena->shCache, arena->shCacheLimit,
shieldCacheEntryCompare, UNUSED_POINTER);
mode = AccessSetEMPTY;
for (i = 0; i < arena->shCacheLimit; ++i) {
AVER(arena->shDepth > 0);
seg = arena->shCache[i];
arena->shCache[i] = NULL; /* ensure it can't be reused */
Seg seg = arena->shCache[i];
AVERT(Seg, seg);
AVER(SegDepth(seg) > 0);
--arena->shDepth;
SegSetDepth(seg, SegDepth(seg) - 1);
AVER(seg->cached);
seg->cached = FALSE;
arena->shCache[i] = NULL; /* ensure it can't be reused */
if (!shieldSegIsSynced(seg)) {
AVER(SegSM(seg) != AccessSetEMPTY); /* can't match first iter */
@ -216,8 +217,8 @@ static void shieldFlushEntries(Arena arena)
/* shieldCache -- consider adding a segment to the cache
*
* If the segment is out of sync, either sync it, or ensure depth > 0,
* and the arena is suspended.
* If the segment is out of sync, either sync it, or ensure it is
* cached and the arena is suspended.
*/
static void shieldCache(Arena arena, Seg seg)
@ -227,10 +228,10 @@ static void shieldCache(Arena arena, Seg seg)
/* Can't fully check seg while we're enforcing its invariants. */
AVER_CRITICAL(TESTT(Seg, seg));
if (shieldSegIsSynced(seg))
if (shieldSegIsSynced(seg) || seg->cached)
return;
if (SegDepth(seg) > 0) { /* already in the cache or exposed? */
if (SegIsExposed(seg)) {
/* This can occur if the mutator isn't suspended, we expose a
segment, then raise the shield on it. In this case, the
mutator isn't allowed to see the segment, but we don't need to
@ -279,11 +280,6 @@ static void shieldCache(Arena arena, Seg seg)
return;
}
SegSetDepth(seg, SegDepth(seg) + 1);
AVER_CRITICAL(SegDepth(seg) > 0); /* overflow */
++arena->shDepth;
AVER_CRITICAL(arena->shDepth > 0); /* overflow */
AVER_CRITICAL(arena->shCacheLimit <= arena->shCacheLength);
AVER_CRITICAL(arena->shCacheI <= arena->shCacheLimit);
@ -304,13 +300,14 @@ static void shieldCache(Arena arena, Seg seg)
arena->shCache[arena->shCacheI] = seg;
++arena->shCacheI;
seg->cached = TRUE;
if (arena->shCacheI >= arena->shCacheLimit)
arena->shCacheLimit = arena->shCacheI;
}
void (ShieldRaise) (Arena arena, Seg seg, AccessSet mode)
void (ShieldRaise)(Arena arena, Seg seg, AccessSet mode)
{
/* .seg.broken: Seg's shield invariants may not be true at */
/* this point (this function is called to enforce them) so we */
@ -327,6 +324,7 @@ void (ShieldRaise) (Arena arena, Seg seg, AccessSet mode)
/* ensure .inv.unsynced.suspended and .inv.unsynced.depth */
shieldCache(arena, seg);
/* Check cache and segment consistency. */
AVERT(Arena, arena);
AVERT(Seg, seg);
}
@ -334,13 +332,17 @@ void (ShieldRaise) (Arena arena, Seg seg, AccessSet mode)
void (ShieldLower)(Arena arena, Seg seg, AccessSet mode)
{
/* Don't check seg or arena, see .seg.broken */
AVERT(Arena, arena);
AVER(TESTT(Seg, seg));
AVERT(AccessSet, mode);
AVER((SegSM(seg) & mode) == mode);
AVER(BS_INTER(SegSM(seg), mode) == mode);
/* synced(seg) is not changed by the following preserving
inv.unsynced.suspended Also inv.prot.shield preserved */
SegSetSM(seg, SegSM(seg) & ~mode);
SegSetSM(seg, BS_DIFF(SegSM(seg), mode));
protLower(arena, seg, mode);
/* Check cache and segment consistency. */
AVERT(Arena, arena);
AVERT(Seg, seg);
}
@ -352,8 +354,6 @@ void (ShieldEnter)(Arena arena)
AVER(!arena->insideShield);
AVER(arena->shDepth == 0);
AVER(!arena->suspended);
AVER(arena->shCacheLimit <= arena->shCacheLength);
AVER(arena->shCacheI <= arena->shCacheLimit);
shieldCacheReset(arena);
@ -386,6 +386,7 @@ void (ShieldLeave)(Arena arena)
AVER(arena->insideShield);
ShieldFlush(arena);
/* Cache is empty so .inv.outside.depth holds */
AVER(arena->shDepth == 0);
@ -396,6 +397,20 @@ void (ShieldLeave)(Arena arena)
arena->suspended = FALSE;
}
arena->insideShield = FALSE;
#ifdef SHIELD_DEBUG
{
Seg seg;
if (SegFirst(&seg, arena))
do {
AVER(!seg->cached);
AVER(shieldSegIsSynced(seg));
/* You can directly set protections here to see if it makes a
difference. */
ProtSet(SegBase(seg), SegLimit(seg), SegPM(seg));
} while(SegNext(&seg, arena, seg));
}
#endif
}
@ -434,31 +449,33 @@ void (ShieldExpose)(Arena arena, Seg seg)
AVER_CRITICAL(arena->insideShield);
SegSetDepth(seg, SegDepth(seg) + 1);
AVER_CRITICAL(SegDepth(seg) > 0); /* overflow */
++arena->shDepth;
/* <design/trace/#fix.noaver> */
AVER_CRITICAL(arena->shDepth > 0);
AVER_CRITICAL(SegDepth(seg) > 0);
if (SegPM(seg) & mode)
AVER_CRITICAL(arena->shDepth > 0); /* overflow */
if (BS_INTER(SegPM(seg), mode))
ShieldSuspend(arena);
/* This ensures inv.expose.prot */
/* Ensure design.mps.shield.inv.expose.prot. */
protLower(arena, seg, mode);
}
/* ShieldCover -- declare MPS no longer needs access to seg */
void (ShieldCover)(Arena arena, Seg seg)
{
/* <design/trace/#fix.noaver> */
AVERT_CRITICAL(Arena, arena);
AVERT_CRITICAL(Seg, seg);
AVER_CRITICAL(SegPM(seg) == AccessSetEMPTY);
AVER_CRITICAL(arena->shDepth > 0);
AVER_CRITICAL(SegDepth(seg) > 0);
SegSetDepth(seg, SegDepth(seg) - 1);
AVER_CRITICAL(arena->shDepth > 0);
--arena->shDepth;
/* ensure inv.unsynced.depth */
/* Ensure design.mps.shield.inv.unsynced.depth. */
shieldCache(arena, seg);
}

View file

@ -115,13 +115,9 @@ segment should be unprotected, and the Shield could re-instate
hardware protection.
However, as a performance-improving hysteresis, the Shield defers
re-protection, maintaining a cache reasons that a segment no longer
had a reason to be collector-accessible. Presence in the cache counts
as a reason: segments in the cache have ``seg->depth`` increased by
one. As segments get pushed out of the cache, or at ``ShieldLeave()``,
this artificial reason is decremented from ``seg->depth``, and (if
``seg->depth`` is now zero) the deferred reinstatement of hardware
protection happens.
re-protection, maintaining a cache of segments that no longer have a
reason to be collector-accessible. While a segment is in the cache,
it has ``seg->cached`` set to TRUE.
This hysteresis allows the MPS to proceed with garbage collection
during a pause without actually setting hardware protection until it
@ -154,20 +150,22 @@ same, and unsynced otherwise.
.def.depth: The depth of a segment is defined as:
| depth ≔ #exposes #covers + #(in cache), where
| depth ≔ #exposes #covers, where
| #exposes = the total number of times the seg has been exposed
| #covers = the total number of times the seg has been covered
| #(in cache) = the number of times the seg appears in the cache
The cache is initially empty and Cover should not be called without a
matching Expose, so this figure should always be non-negative.
The cache is initially empty and ``ShieldCover`` should not be called
without a matching ``ShieldExpose``, so this figure should always be
non-negative.
.def.total.depth: The total depth is the sum of the depth over
all segments.
.def.total.depth: The total depth is the sum of the depth over all
segments.
.def.outside: Being outside the shield is being between calls to leave
and enter, and similarly .def.inside: being inside the shield is being
between calls to enter and leave.
.def.outside: Being outside the shield is being between calls to
``ShieldLeave`` and ``ShieldEnter``, and similarly .def.inside: being
inside the shield is being between calls to ``ShieldEnter`` and
``ShieldLeave``. [In a multi-threaded MPS this would be per-thread.
RB 2016-03-18]
.def.suspended: Suspended is true iff the mutator is suspended.
@ -196,13 +194,15 @@ shield.
.inv.unsynced.suspended: If any segment is not synced, the mutator is
suspended.
.inv.unsynced.depth: All unsynced segments have positive depth.
.inv.unsynced.depth: All unsynced segments have positive depth or are
in the cache.
.inv.outside.depth: The total depth is zero while outside the shield.
.inv.prot.shield: The prot mode is never more than the shield mode.
.inv.expose.prot: An exposed seg is not protected.
.inv.expose.prot: An exposed seg is not protected in the mode it was
exposed with.
Proof Hints
@ -236,6 +236,17 @@ _`.ideas`: There never was an initial design document, but
[RB_1995-11-29]_ and [RB_1995-11-30]_ contain some initial ideas.
Improvement Ideas
-----------------
.improv.mass-expose: If protection calls have a high overhead it might
be good to pre-emptively unprotect large ranges of memory when we
expose one segment. With the current design this would mean
discovering adjacent shielded segments and adding them to the cache.
The collector should take advantage of this by preferentially scanning
exposed segments during a pause.
References
----------