From 74ff7d56bc0e39aa695fd1202e235cdbd7963f27 Mon Sep 17 00:00:00 2001 From: Richard Kistruck Date: Fri, 12 Feb 2010 14:16:53 +0000 Subject: [PATCH 1/9] Mps br/padding: make arenapoll do lots of tracepolls if required, to catch up after fillmutatorsize jumps by > 64kib (job002205) Changes: - separate ArenaPoll and ArenaStep code paths; - simplify ArenaPoll; - loop calling TracePoll to catch-up; zcoll: 100MB is a more sensible arena size than 0.5 MB Warning: barely "experimental" code quality, omitting the following necessities: - consideration of interactions with ArenaStep, - re-engineering of ArenaPoll and friends. Copied from Perforce Change: 169814 ServerID: perforce.ravenbrook.com --- mps/code/global.c | 32 ++++++++++++++++++++++++++++---- mps/code/zcoll.c | 4 +++- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/mps/code/global.c b/mps/code/global.c index 55bb5013a79..71a86d99659 100644 --- a/mps/code/global.c +++ b/mps/code/global.c @@ -649,19 +649,43 @@ void (ArenaPoll)(Globals globals) #else void ArenaPoll(Globals globals) { - double size; + Arena arena; + Clock start; + Size tracedSize; + Bool didSomeTraceWork = FALSE; AVERT(Globals, globals); if (globals->clamped) return; - size = globals->fillMutatorSize; - if (globals->insidePoll || size < globals->pollThreshold) + if (globals->insidePoll) + return; + if(globals->fillMutatorSize < globals->pollThreshold) return; globals->insidePoll = TRUE; - (void)ArenaStep(globals, 0.0, 0.0); + /* fillMutatorSize has advanced; call TracePoll enough to catch up. */ + start = ClockNow(); + arena = GlobalsArena(globals); + do { + tracedSize = TracePoll(globals); + if(tracedSize == 0) + break; + + didSomeTraceWork = TRUE; + arena->tracedSize += tracedSize; + + /* Increment pollThreshold; check: enough precision? */ + AVER(globals->pollThreshold + < globals->pollThreshold + ArenaPollALLOCTIME); + globals->pollThreshold += ArenaPollALLOCTIME; + } while (tracedSize > 0 + && globals->pollThreshold <= globals->fillMutatorSize); + + if(didSomeTraceWork) { + arena->tracedTime += (ClockNow() - start) / (double) ClocksPerSec(); + } globals->insidePoll = FALSE; } diff --git a/mps/code/zcoll.c b/mps/code/zcoll.c index 373e60ec2c3..9bc9e931884 100644 --- a/mps/code/zcoll.c +++ b/mps/code/zcoll.c @@ -704,7 +704,9 @@ int main(int argc, char **argv) /* 16<<20 == 16777216 == 16 Mebibyte */ /* 1<<19 == 524288 == 1/2 Mebibyte */ - testscriptA("Arena(size 524288), Make(keep-1-in 5, keep 50000, rootspace 30000, sizemethod 1), Collect."); + /* This is bogus! sizemethod 1 can make a 300,000-slot dylan vector, ie. 1.2MB. */ + /* Try 100MB arena */ + testscriptA("Arena(size 100000000), Make(keep-1-in 5, keep 50000, rootspace 30000, sizemethod 1), Collect."); /* LSP -- Large Segment Padding (job001811) * From e68fc07c6be57992eca995407a86353b888d1547 Mon Sep 17 00:00:00 2001 From: Richard Kistruck Date: Fri, 12 Feb 2010 15:27:48 +0000 Subject: [PATCH 2/9] Mps br/padding: testrunner.py: vc9.0: build and run w3i3m9. So user should do \vc9vars.bat before invoking ..\tool\testrunner.py Copied from Perforce Change: 169815 ServerID: perforce.ravenbrook.com --- mps/tool/test-runner.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mps/tool/test-runner.py b/mps/tool/test-runner.py index 2ca61d35580..d3f5e2df932 100755 --- a/mps/tool/test-runner.py +++ b/mps/tool/test-runner.py @@ -64,7 +64,7 @@ def mpsplatformcode() : # Here, we simplify and get it right for Windows and Macs. try : compiler = {'xc':'gc', - 'w3':'mv', + 'w3':'m9', }[os] except : pass @@ -79,7 +79,7 @@ mpsplatform = mpsplatformcode() make = '' if mpsplatform[4:6] == 'gc' : make = "make -r -f %s.gmk VARIETY=%%s %%s >> %%s" % mpsplatform -elif mpsplatform[4:6] == 'mv' : +elif mpsplatform[4:6] == 'm9' : make = "nmake /f %s.nmk VARIETY=%%s %%s.exe >>%%s" % mpsplatform run = '' @@ -119,6 +119,7 @@ runtestlist([ "awlut", "awluthe", "mpsicv", + "zcoll", "zmess", "messtest", ], ["we", "hi", "di", "ci"], testout) From 0eac7f0aa5ac6ca604f458d03b7b2e0fa5a56af4 Mon Sep 17 00:00:00 2001 From: Richard Kistruck Date: Fri, 12 Feb 2010 15:32:49 +0000 Subject: [PATCH 3/9] Mps br/padding: exp-169816: job002205 experimental quick-fix Copied from Perforce Change: 169816 ServerID: perforce.ravenbrook.com --- mps/code/version.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mps/code/version.c b/mps/code/version.c index aa570675bb4..90c53406965 100644 --- a/mps/code/version.c +++ b/mps/code/version.c @@ -29,7 +29,7 @@ SRCID(version, "$Id$"); * (Note: before 2006-02-01 the style was "release.epcore.chub") */ -#define MPS_RELEASE "exp-168778" +#define MPS_RELEASE "exp-169816" /* MPSCopyrightNotice -- copyright notice for the binary @@ -39,7 +39,7 @@ SRCID(version, "$Id$"); */ char MPSCopyrightNotice[] = - "Portions copyright (c) 2009 Ravenbrook Limited and Global Graphics Software."; + "Portions copyright (c) 2010 Ravenbrook Limited and Global Graphics Software."; /* MPSVersion -- return version string From fe4b04e0a9a63b870d874a5208ce7bf9ee49321a Mon Sep 17 00:00:00 2001 From: Richard Kistruck Date: Fri, 12 Feb 2010 17:17:32 +0000 Subject: [PATCH 4/9] Mps br/padding: global.c arenapoll: just loop until pollthreshold is past fillmutatorsize. don't be clever. fixes defect in global.cwhere if there was no tracepoll work to do, pollthreshold would not be updated. (ooops). Copied from Perforce Change: 169818 ServerID: perforce.ravenbrook.com GitHub-reference: https://github.com/Ravenbrook/mps/issues/4 --- mps/code/global.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/mps/code/global.c b/mps/code/global.c index 71a86d99659..933e09f6b3e 100644 --- a/mps/code/global.c +++ b/mps/code/global.c @@ -652,7 +652,6 @@ void ArenaPoll(Globals globals) Arena arena; Clock start; Size tracedSize; - Bool didSomeTraceWork = FALSE; AVERT(Globals, globals); @@ -666,26 +665,29 @@ void ArenaPoll(Globals globals) globals->insidePoll = TRUE; /* fillMutatorSize has advanced; call TracePoll enough to catch up. */ - start = ClockNow(); + /* [Experimental code; RHSK 2010-02-12. No consideration of */ + /* efficiency: many calls to ClockNow(), and continued calls to */ + /* TracePoll even when it has reported that it has no work to do.] */ arena = GlobalsArena(globals); do { - tracedSize = TracePoll(globals); - if(tracedSize == 0) - break; + start = ClockNow(); - didSomeTraceWork = TRUE; - arena->tracedSize += tracedSize; + tracedSize = TracePoll(globals); + + if(tracedSize > 0) { + /* Record the work done. */ + arena->tracedSize += tracedSize; + arena->tracedTime += (ClockNow() - start) / (double) ClocksPerSec(); + } /* Increment pollThreshold; check: enough precision? */ AVER(globals->pollThreshold < globals->pollThreshold + ArenaPollALLOCTIME); globals->pollThreshold += ArenaPollALLOCTIME; - } while (tracedSize > 0 - && globals->pollThreshold <= globals->fillMutatorSize); - if(didSomeTraceWork) { - arena->tracedTime += (ClockNow() - start) / (double) ClocksPerSec(); - } + } while (globals->pollThreshold <= globals->fillMutatorSize); + + AVER(globals->fillMutatorSize < globals->pollThreshold); globals->insidePoll = FALSE; } From 9f68e586bbddaa0ef8d6a12ae1eb994a2bb4560f Mon Sep 17 00:00:00 2001 From: Richard Kistruck Date: Fri, 12 Feb 2010 17:23:30 +0000 Subject: [PATCH 5/9] mps br/padding: exp-169819: job002205 experimental quick-fix, corrected. delete exp-169816. Copied from Perforce Change: 169819 ServerID: perforce.ravenbrook.com --- mps/code/version.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mps/code/version.c b/mps/code/version.c index 90c53406965..5a87248d9d2 100644 --- a/mps/code/version.c +++ b/mps/code/version.c @@ -29,7 +29,7 @@ SRCID(version, "$Id$"); * (Note: before 2006-02-01 the style was "release.epcore.chub") */ -#define MPS_RELEASE "exp-169816" +#define MPS_RELEASE "exp-169819" /* MPSCopyrightNotice -- copyright notice for the binary From 3d9188df9d3d224253c63a05a5c5f6539a03a7ae Mon Sep 17 00:00:00 2001 From: Richard Kistruck Date: Tue, 23 Feb 2010 13:43:27 +0000 Subject: [PATCH 6/9] Mps br/padding arenapoll: non-naive fix for j2205: correct updating of pollthreshold, depending on whether we have no work and are sleeping, or have work and are advancing the clock by one unit. if there's no work, don't keep checking. avoid multiple calls to clock(). Copied from Perforce Change: 169851 ServerID: perforce.ravenbrook.com --- mps/code/global.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/mps/code/global.c b/mps/code/global.c index 933e09f6b3e..2e353f9b29c 100644 --- a/mps/code/global.c +++ b/mps/code/global.c @@ -651,7 +651,9 @@ void ArenaPoll(Globals globals) { Arena arena; Clock start; + Count quanta; Size tracedSize; + double nextPollThreshold = 0.0; AVERT(Globals, globals); @@ -665,27 +667,31 @@ void ArenaPoll(Globals globals) globals->insidePoll = TRUE; /* fillMutatorSize has advanced; call TracePoll enough to catch up. */ - /* [Experimental code; RHSK 2010-02-12. No consideration of */ - /* efficiency: many calls to ClockNow(), and continued calls to */ - /* TracePoll even when it has reported that it has no work to do.] */ arena = GlobalsArena(globals); - do { - start = ClockNow(); - + start = ClockNow(); + quanta = 0; + while(globals->pollThreshold <= globals->fillMutatorSize) { tracedSize = TracePoll(globals); - if(tracedSize > 0) { - /* Record the work done. */ + if(tracedSize == 0) { + /* No work to do. Sleep until NOW + a bit. */ + nextPollThreshold = globals->fillMutatorSize + ArenaPollALLOCTIME; + } else { + /* We did one quantum of work; consume one unit of 'time'. */ + quanta += 1; arena->tracedSize += tracedSize; - arena->tracedTime += (ClockNow() - start) / (double) ClocksPerSec(); + nextPollThreshold = globals->pollThreshold + ArenaPollALLOCTIME; } - /* Increment pollThreshold; check: enough precision? */ - AVER(globals->pollThreshold - < globals->pollThreshold + ArenaPollALLOCTIME); - globals->pollThreshold += ArenaPollALLOCTIME; + /* Advance pollThreshold; check: enough precision? */ + AVER(nextPollThreshold > globals->pollThreshold); + globals->pollThreshold = nextPollThreshold; + } - } while (globals->pollThreshold <= globals->fillMutatorSize); + /* Don't count time spent checking for work, if there was no work to do. */ + if(quanta > 0) { + arena->tracedTime += (ClockNow() - start) / (double) ClocksPerSec(); + } AVER(globals->fillMutatorSize < globals->pollThreshold); From 0314914d5e891e31080dc1baae46979f354e690f Mon Sep 17 00:00:00 2001 From: Richard Kistruck Date: Wed, 24 Feb 2010 16:07:17 +0000 Subject: [PATCH 7/9] Mps br/padding (non-working changelist) see whether we can clear up what "clamped" means: ArenaPoll will still call TracePoll if clamped... but TracePoll won't start a new trace if clamped ArenaStep won't start an opportunistic full collect if clamped ArenaStep won't advance pollThreshold, ever traceFlip asserts that clamped is FALSE. (see http://info.ravenbrook.com/mail/2010/02/23/13-19-24/0.txt) --- But no, clamped is more complex than that. - Certain mps.h calls affect it. - Certain MPS tests use it for more control and reproducibility. - MPS itself uses it, as part of starting a full collect, for which it must first run any current trace to completion without starting any new ones. In particular, this set of changes asserts: MPS ASSERTION FAILURE: ArenaGlobals(arena)->clamped == FALSE trace.c 534 because mps_arena_collect() tries to start a full collect while clamped. --- So this changelist is for historical interest only, and will be backed out. Copied from Perforce Change: 169853 ServerID: perforce.ravenbrook.com --- mps/code/global.c | 12 +++--------- mps/code/trace.c | 4 +++- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/mps/code/global.c b/mps/code/global.c index 2e353f9b29c..346ff7bc291 100644 --- a/mps/code/global.c +++ b/mps/code/global.c @@ -657,8 +657,6 @@ void ArenaPoll(Globals globals) AVERT(Globals, globals); - if (globals->clamped) - return; if (globals->insidePoll) return; if(globals->fillMutatorSize < globals->pollThreshold) @@ -715,8 +713,9 @@ static Bool arenaShouldCollectWorld(Arena arena, /* don't collect the world if we're not given any time */ if ((interval > 0.0) && (multiplier > 0.0)) { - /* don't collect the world if we're already collecting. */ - if (arena->busyTraces == TraceSetEMPTY) { + /* don't collect the world if we're clamped or busy. */ + if (!ArenaGlobals(arena)->clamped + && arena->busyTraces == TraceSetEMPTY) { /* don't collect the world if it's very small */ arenaSize = ArenaCommitted(arena) - ArenaSpareCommitted(arena); if (arenaSize > 1000000) { @@ -746,7 +745,6 @@ static Bool arenaShouldCollectWorld(Arena arena, Bool ArenaStep(Globals globals, double interval, double multiplier) { - double size; Size scanned; Bool stepped; Clock start, end, now; @@ -788,10 +786,6 @@ Bool ArenaStep(Globals globals, double interval, double multiplier) arena->tracedTime += (now - start) / (double) clocks_per_sec; } - size = globals->fillMutatorSize; - globals->pollThreshold = size + ArenaPollALLOCTIME; - AVER(globals->pollThreshold > size); /* enough precision? */ - return stepped; } diff --git a/mps/code/trace.c b/mps/code/trace.c index ff13b6180a9..0186083a1c5 100644 --- a/mps/code/trace.c +++ b/mps/code/trace.c @@ -531,6 +531,8 @@ static void traceFlip(Trace trace) AVER(trace->state == TraceUNFLIPPED); AVER(!TraceSetIsMember(arena->flippedTraces, trace)); + AVER(ArenaGlobals(arena)->clamped == FALSE); /* clamped => no flip */ + EVENT_PP(TraceFlipBegin, trace, arena); traceFlipBuffers(ArenaGlobals(arena)); @@ -1772,7 +1774,7 @@ Size TracePoll(Globals globals) arena = GlobalsArena(globals); scannedSize = (Size)0; - if(arena->busyTraces == TraceSetEMPTY) { + if(!globals->clamped && arena->busyTraces == TraceSetEMPTY) { /* If no traces are going on, see if we need to start one. */ Size sFoundation, sCondemned, sSurvivors, sConsTrace; double tTracePerScan; /* tTrace/cScan */ From 748cce6eebbb49762e9202e3f7177125a8603dc7 Mon Sep 17 00:00:00 2001 From: Richard Kistruck Date: Wed, 24 Feb 2010 16:14:38 +0000 Subject: [PATCH 8/9] mps br/padding (back out non-working changelist 169853 (attempt to clear up what "clamped" means)) Copied from Perforce Change: 169854 ServerID: perforce.ravenbrook.com --- mps/code/global.c | 12 +++++++++--- mps/code/trace.c | 4 +--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/mps/code/global.c b/mps/code/global.c index 346ff7bc291..2e353f9b29c 100644 --- a/mps/code/global.c +++ b/mps/code/global.c @@ -657,6 +657,8 @@ void ArenaPoll(Globals globals) AVERT(Globals, globals); + if (globals->clamped) + return; if (globals->insidePoll) return; if(globals->fillMutatorSize < globals->pollThreshold) @@ -713,9 +715,8 @@ static Bool arenaShouldCollectWorld(Arena arena, /* don't collect the world if we're not given any time */ if ((interval > 0.0) && (multiplier > 0.0)) { - /* don't collect the world if we're clamped or busy. */ - if (!ArenaGlobals(arena)->clamped - && arena->busyTraces == TraceSetEMPTY) { + /* don't collect the world if we're already collecting. */ + if (arena->busyTraces == TraceSetEMPTY) { /* don't collect the world if it's very small */ arenaSize = ArenaCommitted(arena) - ArenaSpareCommitted(arena); if (arenaSize > 1000000) { @@ -745,6 +746,7 @@ static Bool arenaShouldCollectWorld(Arena arena, Bool ArenaStep(Globals globals, double interval, double multiplier) { + double size; Size scanned; Bool stepped; Clock start, end, now; @@ -786,6 +788,10 @@ Bool ArenaStep(Globals globals, double interval, double multiplier) arena->tracedTime += (now - start) / (double) clocks_per_sec; } + size = globals->fillMutatorSize; + globals->pollThreshold = size + ArenaPollALLOCTIME; + AVER(globals->pollThreshold > size); /* enough precision? */ + return stepped; } diff --git a/mps/code/trace.c b/mps/code/trace.c index 0186083a1c5..ff13b6180a9 100644 --- a/mps/code/trace.c +++ b/mps/code/trace.c @@ -531,8 +531,6 @@ static void traceFlip(Trace trace) AVER(trace->state == TraceUNFLIPPED); AVER(!TraceSetIsMember(arena->flippedTraces, trace)); - AVER(ArenaGlobals(arena)->clamped == FALSE); /* clamped => no flip */ - EVENT_PP(TraceFlipBegin, trace, arena); traceFlipBuffers(ArenaGlobals(arena)); @@ -1774,7 +1772,7 @@ Size TracePoll(Globals globals) arena = GlobalsArena(globals); scannedSize = (Size)0; - if(!globals->clamped && arena->busyTraces == TraceSetEMPTY) { + if(arena->busyTraces == TraceSetEMPTY) { /* If no traces are going on, see if we need to start one. */ Size sFoundation, sCondemned, sSurvivors, sConsTrace; double tTracePerScan; /* tTrace/cScan */ From e141e431af5f99437843f86c9a847a9e1b466eea Mon Sep 17 00:00:00 2001 From: Richard Kistruck Date: Wed, 24 Feb 2010 16:18:10 +0000 Subject: [PATCH 9/9] Mps br/padding arenastep: do not change pollthreshold -- that's for arenapoll, not arenastep. So ArenaStep may advance, but not retard, trace work. Copied from Perforce Change: 169855 ServerID: perforce.ravenbrook.com --- mps/code/global.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/mps/code/global.c b/mps/code/global.c index 2e353f9b29c..a9090d00372 100644 --- a/mps/code/global.c +++ b/mps/code/global.c @@ -746,7 +746,6 @@ static Bool arenaShouldCollectWorld(Arena arena, Bool ArenaStep(Globals globals, double interval, double multiplier) { - double size; Size scanned; Bool stepped; Clock start, end, now; @@ -788,10 +787,6 @@ Bool ArenaStep(Globals globals, double interval, double multiplier) arena->tracedTime += (now - start) / (double) clocks_per_sec; } - size = globals->fillMutatorSize; - globals->pollThreshold = size + ArenaPollALLOCTIME; - AVER(globals->pollThreshold > size); /* enough precision? */ - return stepped; }