Skip to content

141-followup-residual-characterisation #202

Description

@G00dS0ul

Summary

CacheFootprintBenchmark.ReturnToIdle_AfterClearCache asserts that the managed heap returns to
within a fixed budget of an idle baseline after DiskScannerEngine.ClearCache(). As part of #141
that assertion was found to be unable to discriminate the bug it was written for, and its limit
was raised from 8,000,000 to 12,000,000 bytes as a stopgap.

This issue is to characterise what that residual actually consists of, and either make the
assertion meaningful or replace it with one that is.

Evidence

All figures measured on one machine within one session, so they are comparable to each other.
residual = GC.GetTotalMemory(forceFullCollection: true) - _idleManagedBytes, computed at
GSAnalyzer.Benchmarks/Suites/Storage/CacheFootprintBenchmark.cs:142.

Engine state Analyzer snapshots after ClearCache Residual
Before #141 retained for up to their 15-minute TTL 8,053,520 B
#141 levers A+B evicted (proven, see below) 8,633,168 B
#141 levers A+B+C evicted 8,491,944 B
#141 levers A+B+C evicted < 8,000,000 B (passed)
#141 levers A+B+C evicted < 8,000,000 B (passed)

The ranges overlap. Fixing the bug did not move the number outside its own run-to-run noise,
which is roughly 600 KB. And 8,000,000 B was below the floor the benchmark reaches even on
completely unmodified code — the assertion was red at baseline, so it never had headroom to
regress into.

Eviction is genuinely fixed; it is simply invisible to this metric. It is proven by two unit tests
in the core repo:

  • ScanCacheServiceTests.ClearCache_EvictsAnalyzerSnapshots_ViaSnapshotResetToken — an entry
    registered with _engine.SnapshotResetToken is gone after ClearCache().
  • ScanCacheServiceTests.SnapshotResetToken_AfterClearCache_IsFreshSoLaterSnapshotsSurvive — the
    token source is replaced, so snapshots cached after a reset are not dead on arrival.

What the residual is not

Ruled out during #141:

  • Not the analyzer snapshots. Evicted, per the tests above.
  • Not DirectorySizeCache. ClearCache() empties it.
  • Not ScanCacheService. ClearCache() calls its Clear(), and the benchmark constructs the
    engine with no cache service anyway (CacheFootprintBenchmark.cs:99).
  • Not uncompacted Large Object Heap. ClearCache() now sets
    GCLargeObjectHeapCompactionMode.CompactOnce and runs two blocking compacting collections with
    WaitForPendingFinalizers between them.

Plausible remaining sources — none verified

Listed as hypotheses, not findings. Each needs a profiler to confirm or kill.

  1. Expiration-token plumbing. The benchmark calls Analyze / GetExtensionBreakdown /
    Analyze for 1,000 leaf folders (CacheFootprintBenchmark.cs:132-137) = 3,000 cache
    entries, each now carrying a CancellationChangeToken, a CancellationTokenRegistration and
    MemoryCache's per-entry token list. The +579,648 B seen between baseline and levers A+B works
    out at ~193 B/entry, which is the right order — consistent with, but not proof of, this
    explanation.
  2. MemoryCache internal capacity. The two MemoryCache instances keep their internal
    dictionary sized for 3,000 entries after eviction.
  3. BenchmarkDotNet's own per-iteration machinery, which sits inside the same measurement.
  4. Baseline placement. _idleManagedBytes is captured once in [GlobalSetup]
    (CacheFootprintBenchmark.cs:104) before any scan, while the assertion runs on each of 8
    iterations. Anything that accumulates across iterations is charged to the residual.

Suggested direction

  1. Characterise first, tune second. Take a dotnet-gcdump immediately after ClearCache() in
    this benchmark and attribute the retained set. Everything above is a guess until that exists.
  2. Then consider replacing the metric. A direct assertion — "the analyzer caches report zero
    entries after ClearCache()" — tests the actual property, is immune to allocation noise, and
    would have caught the original bug cleanly. The residual check could stay alongside it as a
    loose smoke test for gross retention.
  3. If the token plumbing turns out to dominate, weigh a cheaper eviction mechanism — but note the
    constraint below before proposing one.

Constraints

Relationship to #141

#141 fixed the two things it could prove: the transient spike
(Footprint_FullStoragePipeline 38,001,528 B → ~23,730,000 B, −37.5%) and snapshot eviction on
ClearCache. It raised this threshold rather than chase a number nobody had characterised, on the
grounds that a threshold asserting noise is worse than one asserting nothing. This issue is the
part that was deliberately deferred.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    PerformanceOptimize current featuresbackendC#memoryMemory leakage and spikescannerStorage scanner

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions