From f31fc5325ac40853449d6d2dc138348dbc87c77c Mon Sep 17 00:00:00 2001 From: Richard Brooksby Date: Mon, 24 Feb 2014 23:04:31 +0000 Subject: [PATCH 1/2] Instead of aggressively returning every page it can, mvff takes a parameter for the proportion of spare space to hold in its free lists before attempting to return space to the arena. Copied from Perforce Change: 184498 ServerID: perforce.ravenbrook.com --- mps/code/config.h | 1 + mps/code/mps.h | 3 ++ mps/code/pool.c | 1 + mps/code/poolmvff.c | 126 +++++++++++++++++++++++++++++--------------- 4 files changed, 88 insertions(+), 43 deletions(-) diff --git a/mps/code/config.h b/mps/code/config.h index 4ad6ea51f15..da4bf132400 100644 --- a/mps/code/config.h +++ b/mps/code/config.h @@ -277,6 +277,7 @@ #define MVFF_SLOT_HIGH_DEFAULT FALSE #define MVFF_ARENA_HIGH_DEFAULT FALSE #define MVFF_FIRST_FIT_DEFAULT TRUE +#define MVFF_SPARE_DEFAULT 0.75 /* Pool MVT Configuration -- see */ diff --git a/mps/code/mps.h b/mps/code/mps.h index f8751d01372..7e3e734d336 100644 --- a/mps/code/mps.h +++ b/mps/code/mps.h @@ -184,6 +184,9 @@ extern const struct mps_key_s _mps_key_max_size; extern const struct mps_key_s _mps_key_align; #define MPS_KEY_ALIGN (&_mps_key_align) #define MPS_KEY_ALIGN_FIELD align +extern const struct mps_key_s _mps_key_spare; +#define MPS_KEY_SPARE (&_mps_key_spare) +#define MPS_KEY_SPARE_FIELD double extern const struct mps_key_s _mps_key_cbs_extend_by; #define MPS_KEY_CBS_EXTEND_BY (&_mps_key_cbs_extend_by) #define MPS_KEY_CBS_EXTEND_BY_FIELD size diff --git a/mps/code/pool.c b/mps/code/pool.c index df6b0fa9032..eaf751b25d8 100644 --- a/mps/code/pool.c +++ b/mps/code/pool.c @@ -118,6 +118,7 @@ ARG_DEFINE_KEY(min_size, Size); ARG_DEFINE_KEY(mean_size, Size); ARG_DEFINE_KEY(max_size, Size); ARG_DEFINE_KEY(align, Align); +ARG_DEFINE_KEY(spare, double); /* PoolInit -- initialize a pool diff --git a/mps/code/poolmvff.c b/mps/code/poolmvff.c index d26011856b7..07adb19ad4d 100644 --- a/mps/code/poolmvff.c +++ b/mps/code/poolmvff.c @@ -49,6 +49,7 @@ typedef struct MVFFStruct { /* MVFF pool outer structure */ Size avgSize; /* client estimate of allocation size */ Size total; /* total bytes in pool */ Size free; /* total free bytes in pool */ + double spare; /* spare space fraction, see MVFFReduce */ MFSStruct cbsBlockPoolStruct; /* stores blocks for CBSs */ CBSStruct totalCBSStruct; /* all memory allocated from the arena */ CBSStruct freeCBSStruct; /* free list */ @@ -156,55 +157,85 @@ static void MVFFDeleteFromFree(MVFF mvff, Range range) /* MVFFReduce -- free segments from given range * - * Given a free range, attempts to find entire tracts within - * it, and returns them to the arena, updating total size counter. + * Consider reducing the total size of the pool by returning memory + * to the arena. * * This is usually called immediately after MVFFAddToFreeList. * It is not combined with MVFFAddToFreeList because the latter * is also called when new segments are added under MVFFAlloc. */ -static void MVFFReduce(MVFF mvff, Range range) +static void MVFFReduce(MVFF mvff) { Arena arena; - RangeStruct alignedRange, oldRange; - Addr base, limit; - Size size; - Res res; - - AVERT(MVFF, mvff); - AVER(RangeBase(range) < RangeLimit(range)); - /* Could profitably AVER that the given range is free, */ - /* but the CBS doesn't provide that facility. */ - - size = RangeSize(range); - - arena = PoolArena(MVFF2Pool(mvff)); - base = AddrAlignUp(RangeBase(range), ArenaAlign(arena)); - limit = AddrAlignDown(RangeLimit(range), ArenaAlign(arena)); - if (base >= limit) { /* no whole tracts */ - return; - } - - RangeInit(&alignedRange, base, limit); - AVER(RangesNest(range, &alignedRange)); - - /* Delete the range from the free list before attempting to delete it - from the total allocated memory, so that we don't have dangling blocks - in the freelist, even for a moment. If we fail to delete from the - totalCBS we add back to the freelist, which can't fail. */ + RangeStruct freeRange; + Size freeLimit, targetFree; + Align align; - MVFFDeleteFromFree(mvff, &alignedRange); + AVERT(MVFF, mvff); + arena = PoolArena(MVFF2Pool(mvff)); + align = ArenaAlign(arena); - res = CBSDelete(&oldRange, MVFFTotalCBS(mvff), &alignedRange); - if (res != ResOK) { - RangeStruct coalesced; - MVFFAddToFree(&coalesced, mvff, &alignedRange); + /* Try to return memory when the amount of free memory exceeds a + threshold fraction of the total memory. */ + + /* NOTE: If this code becomes very hot, then the test of whether there's + a large free block in the CBS could be inlined, since it's a property + stored at the root node. */ + + freeLimit = (Size)(mvff->total * mvff->spare); + if (mvff->free < freeLimit) return; - } - mvff->total -= RangeSize(&alignedRange); - ArenaFree(base, AddrOffset(base, limit), MVFF2Pool(mvff)); + targetFree = freeLimit / 2; + while (mvff->free > targetFree && + CBSFindLargest(&freeRange, &freeRange, MVFFFreeCBS(mvff), + 0, FindDeleteNONE)) { + RangeStruct pageRange, oldRange; + Size size; + Res res; + Addr base, limit; + + base = AddrAlignUp(RangeBase(&freeRange), align); + limit = AddrAlignDown(RangeLimit(&freeRange), align); + + /* Give up if the block is too small to contain a whole page when + aligned, even though it might be masking smaller better aligned + pages that we could return, because CBSFindLargest won't be able + to find those. */ + if (base >= limit) + break; + + size = AddrOffset(base, limit); + + /* Don't return (much) more than we need to. */ + if (size > mvff->free - targetFree) + size = SizeAlignUp(mvff->free - targetFree, align); + + /* Calculate the range of pages we can return to the arena near the + top end of the free memory (because we're first fit). */ + RangeInit(&pageRange, AddrSub(limit, size), limit); + AVER(!RangeIsEmpty(&pageRange)); + AVER(RangesNest(&freeRange, &pageRange)); + AVER(RangeIsAligned(&pageRange, align)); + + /* Delete the range from the free list before attempting to delete it + from the total allocated memory, so that we don't have dangling blocks + in the freelist, even for a moment. If we fail to delete from the + totalCBS we add back to the freelist, which can't fail. */ + + MVFFDeleteFromFree(mvff, &pageRange); + + res = CBSDelete(&oldRange, MVFFTotalCBS(mvff), &pageRange); + if (res != ResOK) { + RangeStruct coalesced; + MVFFAddToFree(&coalesced, mvff, &pageRange); + return; + } + mvff->total -= RangeSize(&pageRange); + + ArenaFree(RangeBase(&pageRange), RangeSize(&pageRange), MVFF2Pool(mvff)); + } } @@ -374,7 +405,7 @@ static void MVFFFree(Pool pool, Addr old, Size size) size = SizeAlignUp(size, PoolAlignment(pool)); RangeInit(&range, old, AddrAdd(old, size)); MVFFAddToFree(&coalescedRange, mvff, &range); - MVFFReduce(mvff, &coalescedRange); + MVFFReduce(mvff); } /* MVFFFindLargest -- call CBSFindLargest and then fall back to @@ -464,7 +495,7 @@ static void MVFFBufferEmpty(Pool pool, Buffer buffer, RangeInit(&range, base, limit); MVFFAddToFree(&coalescedRange, mvff, &range); - MVFFReduce(mvff, &coalescedRange); + MVFFReduce(mvff); } @@ -510,6 +541,7 @@ static Res MVFFInit(Pool pool, ArgList args) Bool slotHigh = MVFF_SLOT_HIGH_DEFAULT; Bool arenaHigh = MVFF_ARENA_HIGH_DEFAULT; Bool firstFit = MVFF_FIRST_FIT_DEFAULT; + double spare = MVFF_SPARE_DEFAULT; MVFF mvff; Arena arena; Res res; @@ -533,6 +565,9 @@ static Res MVFFInit(Pool pool, ArgList args) if (ArgPick(&arg, args, MPS_KEY_ALIGN)) align = arg.val.align; + if (ArgPick(&arg, args, MPS_KEY_SPARE)) + spare = arg.val.d; + if (ArgPick(&arg, args, MPS_KEY_MVFF_SLOT_HIGH)) slotHigh = arg.val.b; @@ -545,6 +580,8 @@ static Res MVFFInit(Pool pool, ArgList args) AVER(extendBy > 0); /* .arg.check */ AVER(avgSize > 0); /* .arg.check */ AVER(avgSize <= extendBy); /* .arg.check */ + AVER(spare >= 0.0); /* .arg.check */ + AVER(spare <= 1.0); /* .arg.check */ AVER(SizeIsAligned(align, MPS_PF_ALIGN)); AVER(BoolCheck(slotHigh)); AVER(BoolCheck(arenaHigh)); @@ -557,6 +594,7 @@ static Res MVFFInit(Pool pool, ArgList args) pool->alignment = align; mvff->slotHigh = slotHigh; mvff->firstFit = firstFit; + mvff->spare = spare; SegPrefInit(MVFFSegPref(mvff)); SegPrefExpress(MVFFSegPref(mvff), arenaHigh ? SegPrefHigh : SegPrefLow, NULL); @@ -794,6 +832,8 @@ static Bool MVFFCheck(MVFF mvff) CHECKL(mvff->extendBy > 0); /* see .arg.check */ CHECKL(mvff->avgSize > 0); /* see .arg.check */ CHECKL(mvff->avgSize <= mvff->extendBy); /* see .arg.check */ + CHECKL(mvff->spare >= 0.0); /* see .arg.check */ + CHECKL(mvff->spare <= 1.0); /* see .arg.check */ CHECKL(mvff->total >= mvff->free); CHECKL(SizeIsAligned(mvff->free, PoolAlignment(MVFF2Pool(mvff)))); CHECKL(SizeIsAligned(mvff->total, ArenaAlign(PoolArena(MVFF2Pool(mvff))))); @@ -801,10 +841,10 @@ static Bool MVFFCheck(MVFF mvff) CHECKD(Freelist, MVFFFreelist(mvff)); CHECKL(BoolCheck(mvff->slotHigh)); CHECKL(BoolCheck(mvff->firstFit)); -#if MVFF_DEBUG - CHECKL(CBSSize(MVFFFreeCBS(mvff)) + - FreelistSize(MVFFFreelist(mvff)) == mvff->free); - CHECKL(CBSSize(MVFFTotalCBS(mvff)) == mvff->total); +#ifdef MVFF_DEBUG /* FIXME: Consider using just "if" */ + CHECKL(mvff->free == CBSSize(MVFFFreeCBS(mvff)) + + FreelistSize(MVFFFreelist(mvff))); + CHECKL(mvff->total == CBSSize(MVFFTotalCBS(mvff))); #endif return TRUE; } From 3dd5db139d60707b53b0cbfd50d0814d7fea5797 Mon Sep 17 00:00:00 2001 From: Richard Brooksby Date: Mon, 24 Feb 2014 23:40:08 +0000 Subject: [PATCH 2/2] Avoid checking every tract in a span on mvspancheck. Copied from Perforce Change: 184499 ServerID: perforce.ravenbrook.com --- mps/code/poolmv.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/mps/code/poolmv.c b/mps/code/poolmv.c index 14cf2c12ce2..b132581969d 100644 --- a/mps/code/poolmv.c +++ b/mps/code/poolmv.c @@ -132,9 +132,7 @@ typedef struct MVSpanStruct { static Bool MVSpanCheck(MVSpan span) { - Addr addr, base, limit; - Arena arena; - Tract tract; + Addr base, limit; CHECKS(MVSpan, span); @@ -170,13 +168,20 @@ static Bool MVSpanCheck(MVSpan span) CHECKL(span->largest == SpanSize(span)+1); } - /* Each tract of the span must refer to the span */ - arena = PoolArena(TractPool(span->tract)); - TRACT_FOR(tract, addr, arena, base, limit) { - CHECKD_NOSIG(Tract, tract); - CHECKL(TractP(tract) == (void *)span); +#ifdef MV_DEBUG + { + Addr addr; + Arena arena; + Tract tract; + /* Each tract of the span must refer to the span */ + arena = PoolArena(TractPool(span->tract)); + TRACT_FOR(tract, addr, arena, base, limit) { + CHECKD_NOSIG(Tract, tract); + CHECKL(TractP(tract) == (void *)span); + } + CHECKL(addr == limit); } - CHECKL(addr == limit); +#endif return TRUE; }