From 7a73e986389525248b74a1f9bca8dc6ec9c9b4ab Mon Sep 17 00:00:00 2001 From: Richard Brooksby Date: Thu, 30 Jan 2014 18:08:10 +0000 Subject: [PATCH] Refactored implementation of preferenced allocation policy from vm arena to general purpose arena. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminating SegPrefExpress in most places, especially where there wasn’t really any preference. Eliminating special case knowledge about garbage collection from the arena. Copied from Perforce Change: 184279 ServerID: perforce.ravenbrook.com --- mps/code/arena.c | 85 ++++++++++++++++++++++++++++++++++++++++----- mps/code/arenavm.c | 32 +++++++++++++++-- mps/code/config.h | 3 +- mps/code/locus.c | 11 ++---- mps/code/mpm.h | 1 + mps/code/mpmst.h | 3 +- mps/code/mpmtypes.h | 1 - mps/code/poolmv2.c | 17 +-------- mps/code/poolmvff.c | 4 --- mps/code/ref.c | 37 ++++++++++++++++++++ 10 files changed, 151 insertions(+), 43 deletions(-) diff --git a/mps/code/arena.c b/mps/code/arena.c index 6f961164c8e..d30ed3cc41a 100644 --- a/mps/code/arena.c +++ b/mps/code/arena.c @@ -183,6 +183,7 @@ Res ArenaInit(Arena arena, ArenaClass class, Align alignment) arena->poolReady = FALSE; /* */ arena->lastTract = NULL; arena->lastTractBase = NULL; + arena->freeZones = ZoneSetUNIV; arena->primary = NULL; RingInit(&arena->chunkRing); @@ -264,7 +265,7 @@ Res ArenaCreate(Arena *arenaReturn, ArenaClass class, ArgList args) res = ResMEMORY; /* size was too small */ goto failStripeSize; } - + res = ControlInit(arena); if (res != ResOK) goto failControlInit; @@ -640,6 +641,7 @@ Res ArenaFreeCBSInsert(Arena arena, Addr base, Addr limit) * * The free CBS contains all the free address space we have in chunks, * so this is the primary method of allocation. + * FIXME: Needs to take a "high" option to use CBSFindLastInZones. */ static Res arenaAllocFromCBS(Tract *tractReturn, ZoneSet zones, @@ -677,6 +679,9 @@ static Res arenaAllocFromCBS(Tract *tractReturn, ZoneSet zones, if (res != ResOK) goto failMark; + arena->freeZones = ZoneSetDiff(arena->freeZones, + ZoneSetOfRange(arena, range.base, range.limit)); + *tractReturn = PageTract(&chunk->pageTable[baseIndex]); /* FIXME: method for this? */ return ResOK; @@ -697,11 +702,20 @@ failMark: * can be maintained and adjusted. */ +static Res arenaExtend(Arena arena, SegPref pref, Size size) +{ + UNUSED(arena); + UNUSED(pref); + UNUSED(size); + return ResUNIMPL; +} + static Res arenaAllocPolicy(Tract *tractReturn, Arena arena, SegPref pref, Size size, Pool pool) { Res res; Tract tract; + ZoneSet zones, moreZones, evenMoreZones; AVER(tractReturn != NULL); AVERT(SegPref, pref); @@ -717,18 +731,71 @@ static Res arenaAllocPolicy(Tract *tractReturn, Arena arena, SegPref pref, return ResCOMMIT_LIMIT; } } - + /* Plan A: allocate from the free CBS in the requested zones */ /* FIXME: Takes no account of other zones fields */ - res = arenaAllocFromCBS(&tract, pref->zones, size, pool); - if (res == ResOK) { - *tractReturn = tract; - return ResOK; + zones = pref->zones; + if (zones != ZoneSetEMPTY) { + res = arenaAllocFromCBS(&tract, zones, size, pool); + if (res == ResOK) + goto found; } - /* Plan B: extend the arena */ - /* FIXME: unimplemented! */ - return ResUNIMPL; + /* Plan B: add free zones that aren't blacklisted */ + /* TODO: Pools without ambiguous roots might not care about the blacklist. */ + /* TODO: zones are precious and (currently) never deallocated, so we + should consider extending the arena first if address space is plentiful */ + moreZones = ZoneSetUnion(zones, ZoneSetDiff(arena->freeZones, pref->avoid)); + if (moreZones != zones) { + res = arenaAllocFromCBS(&tract, moreZones, size, pool); + if (res == ResOK) + goto found; + } + + /* Plan C: Extend the arena, then try A and B again. */ + if (moreZones != ZoneSetEMPTY) { + res = arenaExtend(arena, pref, size); + if (res != ResOK) + return res; + zones = pref->zones; + if (zones != ZoneSetEMPTY) { + res = arenaAllocFromCBS(&tract, zones, size, pool); + if (res == ResOK) + goto found; + } + if (moreZones != zones) { + zones = ZoneSetUnion(zones, ZoneSetDiff(arena->freeZones, pref->avoid)); + res = arenaAllocFromCBS(&tract, moreZones, size, pool); + if (res == ResOK) + goto found; + } + } + + /* Plan D: add every zone that isn't blacklisted. This might mix GC'd + objects with those from other generations, causing the zone check + to give false positives and slowing down the collector. */ + /* TODO: log an event for this */ + evenMoreZones = ZoneSetUnion(moreZones, ZoneSetDiff(ZoneSetUNIV, pref->avoid)); + if (evenMoreZones != moreZones) { + res = arenaAllocFromCBS(&tract, evenMoreZones, size, pool); + if (res == ResOK) + goto found; + } + + /* Last resort: try anywhere. This might put GC'd objects in zones where + common ambiguous bit patterns pin them down, causing the zone check + to give even more false positives permanently, and possibly retaining + garbage indefinitely. */ + res = arenaAllocFromCBS(&tract, ZoneSetUNIV, size, pool); + if (res == ResOK) + goto found; + + /* Uh oh. */ + return res; + +found: + *tractReturn = tract; + return ResOK; } diff --git a/mps/code/arenavm.c b/mps/code/arenavm.c index 3e338f601b4..7c532964d57 100644 --- a/mps/code/arenavm.c +++ b/mps/code/arenavm.c @@ -884,6 +884,7 @@ static void tablePagesUnmapUnused(VMChunk vmChunk, } +#if 0 /* pagesFindFreeInArea -- find a range of free pages in a given address range * * Search for a free run of pages in the free table, between the given @@ -1020,7 +1021,6 @@ static Bool pagesFindFreeInZones(Index *baseReturn, VMChunk *chunkReturn, return FALSE; } - /* pagesFindFreeWithSegPref -- find a range of free pages with given preferences * * Note this does not create or allocate any pages. @@ -1223,6 +1223,7 @@ static Res VMNZAllocPolicy(Index *baseIndexReturn, VMChunk *chunkReturn, } return ResRESOURCE; } +#endif /* pageIsMapped -- checks whether a free page is mapped or not. */ @@ -1367,7 +1368,7 @@ static Res VMPagesMarkAllocated(Arena arena, Chunk chunk, } - +#if 0 /* vmAllocComm -- allocate a region from the arena * * Common code used by mps_arena_class_vm and @@ -1457,7 +1458,6 @@ failPagesMap: return res; } - static Res VMAlloc(Addr *baseReturn, Tract *baseTractReturn, SegPref pref, Size size, Pool pool) { @@ -1474,6 +1474,32 @@ static Res VMNZAlloc(Addr *baseReturn, Tract *baseTractReturn, VMNZAllocPolicy, pref, size, pool); } +#else + +static Res VMAlloc(Addr *baseReturn, Tract *baseTractReturn, + SegPref pref, Size size, Pool pool) +{ + UNUSED(baseReturn); + UNUSED(baseTractReturn); + UNUSED(pref); + UNUSED(size); + UNUSED(pool); + return ResUNIMPL; +} + +static Res VMNZAlloc(Addr *baseReturn, Tract *baseTractReturn, + SegPref pref, Size size, Pool pool) +{ + UNUSED(baseReturn); + UNUSED(baseTractReturn); + UNUSED(pref); + UNUSED(size); + UNUSED(pool); + return ResUNIMPL; +} + +#endif + /* spareRangesMap -- map a function over spare ranges * diff --git a/mps/code/config.h b/mps/code/config.h index 22e4bbfd4de..32a673513aa 100644 --- a/mps/code/config.h +++ b/mps/code/config.h @@ -305,6 +305,7 @@ #define ArenaDefaultZONESET (ZoneSetUNIV << (MPS_WORD_WIDTH / 2)) /* @@@@ knows the implementation of ZoneSets */ +/* FIXME: There's no particular reason to think this will avoid GC segments. */ /* .segpref.default: For EPcore, non-DL segments should be placed high */ /* to reduce fragmentation of DL pools (see request.epcore.170193_). */ @@ -313,7 +314,7 @@ SegPrefSig, /* sig */ \ TRUE, /* high */ \ ArenaDefaultZONESET, /* zoneSet */ \ - FALSE, /* isCollected */ \ + ZoneSetEMPTY, /* avoid */ \ } #define LDHistoryLENGTH ((Size)4) diff --git a/mps/code/locus.c b/mps/code/locus.c index c826cbe71d8..d8a3ca23488 100644 --- a/mps/code/locus.c +++ b/mps/code/locus.c @@ -25,7 +25,7 @@ Bool SegPrefCheck(SegPref pref) CHECKS(SegPref, pref); CHECKL(BoolCheck(pref->high)); /* zones can't be checked because it's arbitrary. */ - CHECKL(BoolCheck(pref->isCollected)); + /* avoid can't be checked because it's arbitrary. */ return TRUE; } @@ -70,11 +70,6 @@ void SegPrefExpress(SegPref pref, SegPrefKind kind, void *p) pref->zones = *(ZoneSet *)p; break; - case SegPrefCollected: - AVER(p == NULL); - pref->isCollected = TRUE; - break; - default: /* Unknown kinds are ignored for binary compatibility. */ /* See design.mps.pref. */ @@ -261,8 +256,8 @@ Res ChainAlloc(Seg *segReturn, Chain chain, Serial genNr, SegClass class, zones = arena->topGen.zones; SegPrefInit(&pref); - SegPrefExpress(&pref, SegPrefCollected, NULL); - SegPrefExpress(&pref, SegPrefZoneSet, &zones); + pref.zones = zones; + pref.avoid = ZoneSetBlacklist(arena); res = SegAlloc(&seg, class, &pref, size, pool, withReservoirPermit, args); if (res != ResOK) return res; diff --git a/mps/code/mpm.h b/mps/code/mpm.h index 3e5d1809a4f..f4f2c5cd5c6 100644 --- a/mps/code/mpm.h +++ b/mps/code/mpm.h @@ -862,6 +862,7 @@ extern ZoneSet ZoneSetOfSeg(Arena arena, Seg seg); extern Bool RangeInZoneSet(Addr *baseReturn, Addr *limitReturn, Addr base, Addr limit, Arena arena, ZoneSet zoneSet, Size size); +extern ZoneSet ZoneSetBlacklist(Arena arena); /* Shield Interface -- see */ diff --git a/mps/code/mpmst.h b/mps/code/mpmst.h index 4dca49b9873..9290d0f7308 100644 --- a/mps/code/mpmst.h +++ b/mps/code/mpmst.h @@ -314,7 +314,7 @@ typedef struct SegPrefStruct { /* segment placement preferences */ Sig sig; /* */ Bool high; /* high or low */ ZoneSet zones; /* preferred zones */ - Bool isCollected; /* whether segment will be collected */ + ZoneSet avoid; /* zones to avoid */ } SegPrefStruct; @@ -659,6 +659,7 @@ typedef struct mps_arena_s { ChunkCacheEntryStruct chunkCache; /* just one entry */ CBSStruct freeCBS; /* CBS of free address space */ + ZoneSet freeZones; /* zones not yet allocated */ /* locus fields () */ GenDescStruct topGen; /* generation descriptor for dynamic gen */ diff --git a/mps/code/mpmtypes.h b/mps/code/mpmtypes.h index 471b1a53e4b..67d0e2b3763 100644 --- a/mps/code/mpmtypes.h +++ b/mps/code/mpmtypes.h @@ -316,7 +316,6 @@ enum { SegPrefHigh = 1, SegPrefLow, SegPrefZoneSet, - SegPrefCollected, SegPrefLIMIT }; diff --git a/mps/code/poolmv2.c b/mps/code/poolmv2.c index e72f3189ed2..69fc149cb79 100644 --- a/mps/code/poolmv2.c +++ b/mps/code/poolmv2.c @@ -53,7 +53,6 @@ static Bool MVTCheckFit(Addr base, Addr limit, Size min, Arena arena); static ABQ MVTABQ(MVT mvt); static CBS MVTCBS(MVT mvt); static Freelist MVTFreelist(MVT mvt); -static SegPref MVTSegPref(MVT mvt); /* Types */ @@ -182,12 +181,6 @@ static Freelist MVTFreelist(MVT mvt) } -static SegPref MVTSegPref(MVT mvt) -{ - return &mvt->segPrefStruct; -} - - /* Methods */ @@ -287,14 +280,6 @@ static Res MVTInit(Pool pool, ArgList args) FreelistInit(MVTFreelist(mvt), align); - { - ZoneSet zones; - /* --- Loci needed here, what should the pref be? */ - SegPrefInit(MVTSegPref(mvt)); - zones = ZoneSetComp(ArenaDefaultZONESET); - SegPrefExpress(MVTSegPref(mvt), SegPrefZoneSet, (void *)&zones); - } - pool->alignment = align; mvt->reuseSize = reuseSize; mvt->fillSize = fillSize; @@ -1202,7 +1187,7 @@ static Res MVTSegAlloc(Seg *segReturn, MVT mvt, Size size, /* Can't use plain old SegClass here because we need to call * SegBuffer() in MVTFree(). */ Res res = SegAlloc(segReturn, GCSegClassGet(), - MVTSegPref(mvt), size, MVT2Pool(mvt), withReservoirPermit, + SegPrefDefault(), size, MVT2Pool(mvt), withReservoirPermit, argsNone); if (res == ResOK) { diff --git a/mps/code/poolmvff.c b/mps/code/poolmvff.c index d2c2997e725..8ed836ea247 100644 --- a/mps/code/poolmvff.c +++ b/mps/code/poolmvff.c @@ -539,7 +539,6 @@ static Res MVFFInit(Pool pool, ArgList args) Arena arena; Res res; void *p; - ZoneSet zones; ArgStruct arg; AVERT(Pool, pool); @@ -595,9 +594,6 @@ static Res MVFFInit(Pool pool, ArgList args) mvff->segPref = (SegPref)p; SegPrefInit(mvff->segPref); SegPrefExpress(mvff->segPref, arenaHigh ? SegPrefHigh : SegPrefLow, NULL); - /* If using zoneset placement, just put it apart from the others. */ - zones = ZoneSetComp(ArenaDefaultZONESET); - SegPrefExpress(mvff->segPref, SegPrefZoneSet, (void *)&zones); mvff->total = 0; mvff->free = 0; diff --git a/mps/code/ref.c b/mps/code/ref.c index 313f3c75a03..93f6e78df2c 100644 --- a/mps/code/ref.c +++ b/mps/code/ref.c @@ -169,6 +169,43 @@ Bool RangeInZoneSet(Addr *baseReturn, Addr *limitReturn, } +/* ZoneSetBlacklist() -- calculate a zone set of likely false positives + * + * We blacklist the zones that could be referenced by values likely to be + * found in ambiguous roots (such as the stack) and misinterpreted as + * references, in order to avoid nailing down objects. This isn't a + * perfect simulation, but it should catch the common cases. + */ + +ZoneSet ZoneSetBlacklist(Arena arena) +{ + ZoneSet blacklist; + union { + mps_word_t word; + mps_addr_t addr; + int i; + long l; + } nono; + + AVERT(Arena, arena); + + blacklist = ZoneSetEMPTY; + nono.word = 0; + nono.i = 1; + blacklist = ZoneSetAdd(arena, blacklist, nono.addr); + nono.i = -1; + blacklist = ZoneSetAdd(arena, blacklist, nono.addr); + nono.l = 1; + blacklist = ZoneSetAdd(arena, blacklist, nono.addr); + nono.l = -1; + blacklist = ZoneSetAdd(arena, blacklist, nono.addr); + + return blacklist; +} + + + + /* C. COPYRIGHT AND LICENSE