Skip to content

RetentionPath: the remainder PR #280 left behind — an undocumented tool and a doc comment without its method #334

Description

@PhysShell

Carved out while closing PR #280 as superseded. 0753a45 landed the witness as "RetentionPath extracted from PR #280, minus the dominator tree" (PR #309, arcs A1–A3), and that exclusion was deliberate. Three things did not come with it, and one of them is a defect rather than a deferral.

1. A doc comment outlived its method — audit/runtime/RetentionPath/Heap.cs:350-353

The extraction removed the dominator method but kept its <summary>:

350        /// <summary>
351        /// The dominator tree of the whole live graph, with retained sizes. This is the well-posed
352        /// version of "who holds it": not a path, but the one reference whose removal frees the object.
353        /// </summary>
354
355        public void Dispose()

C# binds a /// block to the next member regardless of the blank line, so Dispose() is now documented as the dominator tree. Anyone reading the file — or any generated doc — is told the tool computes dominance, which is exactly what was removed. This is the only item here that is straightforwardly wrong rather than merely absent; it should either be deleted or moved onto whatever eventually implements it.

2. RetentionPath is the only runtime tool with no README section

audit/runtime/README.md documents its three neighbours and not this one:

tool README section
LeakHarness ## How the leak-harness works (Plan.md §4.1)
DuplicateDetector ## Duplicate-immutable detector (Plan.md §2 cat. 11)
PropertyChangedStorm ## PropertyChanged-storm profiler (Plan.md §2 cat. 6)
RetentionPath none

Meanwhile README.md:27 still advertises the capability in the stack table:

| Heap analysis (retained, duplicates, retention paths) | ClrMD |

So the README promises retention paths, the tool now delivers them, and the two are not connected. PR #280 carried a 131-line section covering the census / roots split, why roots samples, and the "what it does not do" caveats; none of it landed.

3. The dominator tree is deferred, and the pointer to it dangles

Program.cs on main tells the reader where to go:

The principled version of this is a dominator tree (which single reference, if cut, frees the object — and how much memory that frees). See the README.

The README has no such text. The deferral itself is fine and recorded in RetentionPath.csproj; what is not fine is a cross-reference to a section that does not exist.

For the record of what was set aside: Dominators.cs was 444 lines implementing Cooper–Harvey–Kennedy dominance with retained sizes. Its argument is that "who holds this object" is ill-posed for an object reachable from several roots — a path walk picks one arbitrarily, while the immediate dominator is the one reference whose removal actually frees the object, and it answers honestly when two references hold the object jointly by naming the point where the paths meet. The shipped roots verb takes the other route: sample, rank the resolved paths, and say plainly that the shares are shares of the sample.

Scope

Small and self-contained. Items 1 and 2 are a deletion and a doc section — neither needs the dominator tree back. Item 3 resolves as a side effect of item 2 (say in the README what the tool does and does not do), or by restoring Dominators.cs if the dominance verb is wanted after all — but that is a separate decision, not a prerequisite.

Refs #280 (closed as superseded), #309, 0753a45.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions