MPS issue job002205

TitleMPS collection runs too slow if client allocates big objects
Statusclosed
Priorityessential
Assigned userRichard Kistruck
OrganizationRavenbrook
DescriptionMPS collection runs too slow if client allocates big objects

MPS does collection work incrementally: occasionally, when the client allocates, MPS also does a bit of collection work. The amount of work the MPS does should be in approximate proportion to the amount the client allocates.

But there is a defect: if the client allocates a big object (>>64KiB), the MPS only 'sees' this as 64KiB's worth. Therefore the MPS does its collection work too slowly.

This means memory use may get too large, because the MPS is not working hard enough to collect garbage.
AnalysisRHSK 2010-02-08

An MPS trace should get a TracePoll() call once every ArenaPollALLOCTIME bytes (=64KiB) of fillMutatorSize.

But if fillMutatorSize jumps up by (say) 6400KiB, this trips the TracePoll() condition only ONCE, instead of 100 times:

Pseudocode:
    ArenaPoll() {
        if (fillMutatorSize >= pollThreshold) {
            TracePoll();
            pollThreshold = fillMutatorSize + ArenaPollALLOCTIME;
        }
    }

 (By the way: TracePoll() considers starting a new trace, and then calls TraceQuantum(). TraceQuantum scans one trace->rate's worth of bytes.)

 This defeats the trace-rate logic, whereby MPS chooses a "finishingTime" (= how much fillMutatorSize should have grown by when the trace completes), uses that to predict how many polls it will get, and then tries to work hard enough on each poll to make the trace complete on time.

 Trace->rate logic is used by both incremental collections (which aim for a scan rate of 0.25 or more of the allocation rate), and full collections (which aim to make sure we don't run out of vmem).

 For more details, see:
 <http://www.ravenbrook.com/project/mps/master/manual/wiki/gc_story.html#rate>

 NB had the idea that this defect might exist.


 RHSK 2010-02-24

 Fix: Untangle ArenaPoll and ArenaStep; they run on independent notions of available time. If ArenaPoll has work, advance pollThreshold by ArenaPollALLOCTIME after each quantum of TracePoll work, until it has caught up with fillMutatorSize. This means trace work, of an in-progress trace, tracks fillMutatorSize as intended. If ArenaPoll has no work, bring pollThreshold immediately up-to-date, plus a sleep time (currently also ArenaPollALLOCTIME).

 ArenaPoll is called when the client is busy working, so it should be reasonably efficient: call ClockNow() only when required.

 Further improvement: the notion of a trace->rate quantum of work does sort-of work, though it may mean that the granularity (and hence non-incrementality) is ill-chosen for urgent traces. Trace-progress rate should perhaps be more dynamic anyway.

 Notes: this trace-starting and -progressing code intersects with the arena clamped state. The requirements for mps_arena_clamp(), in this year 2010, are not entirely clear, so I was not able to clear up this intersection. See changelist 169853 for an unsuccessful attempt.
How foundunknown
Evidencemps/master/code/global.c#21
Observed in1.108.2
Created byRichard Kistruck
Created on2010-02-08 13:06:27
Last modified byGareth Rees
Last modified on2014-04-12 22:07:10
History2010-02-08 RHSK Created
2010-02-24 RHSK Describe fix, and other non-fixed issues.

Fixes

Change Effect Date User Description
169855 closed 2010-02-24 16:18:10 Richard Kistruck MPS br/padding ArenaStep: do NOT change pollThreshold -- that's for ArenaPoll, not ArenaStep.
So ArenaStep may advance, but not retard, trace work.
169851 open 2010-02-23 13:43:27 Richard Kistruck 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().
169818 open 2010-02-12 17:17:32 Richard Kistruck MPS br/padding: global.c ArenaPoll: Just loop until pollThreshold is past fillMutatorSize. Don't be clever. Fixes defect in global.c#4, where if there was no TracePoll work to do, pollThreshold would not be updated. (Ooops).
169814 open 2010-02-12 14:16:53 Richard Kistruck 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.