Skip to content

Stabilize TypeRef allocation gate - #4635

Open
richlander wants to merge 6 commits into
mainfrom
fix-4630-type-ref-allocation-gate
Open

Stabilize TypeRef allocation gate#4635
richlander wants to merge 6 commits into
mainfrom
fix-4630-type-ref-allocation-gate

Conversation

@richlander

@richlander richlander commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Summary

Makes the TypeRef zero-allocation gate deterministic on Linux without changing product behavior or accepting amortized allocations.

  • serializes the allocation-test class so unrelated xUnit work cannot interfere with the process-global measurement guard;
  • uses a fixed 10,000-call warmup followed by a fixed 10,000-call sample;
  • drains prior GC/finalizer work and accepts a sample only when a bounded no-GC region remained active for the whole window;
  • retries only invalid region attempts, never a valid non-zero product sample; and
  • adds a non-vacuity case that allocates once every 2,000 operations and must remain detectable.

Closes #4630.

Demo

Before, a GC suspension could retire the measuring thread's unused allocation context and inflate its counter even when TypeRef allocated nothing:

Expected: 0
Actual:   3744

The original round-1 remedy reproduced the same mechanism under bounded in-process pressure:

REAL TypeRefShallowEqualityAndHashing_DoNotAllocate: 97/1000 failed
Actual: 8048

After, the gate retains a fixed 10,000-operation zero-byte sample but accepts it only when the runtime confirms that no GC interrupted the window. A neighboring amortized-allocation case remains non-zero, proving the guard does not turn the measurement into a success-shaped fallback.

Validation

  • dotnet run --project src/ILInspector.Analysis.Tests -c Release -- -method '*TypeRefShallowEqualityAndHashing_DoNotAllocate*' -method '*TypeRefExactAndLegacySimpleNames_AgreeWithoutDelimiterInference*' -method '*AllocationMeasurement_RejectsAmortizedOperationAllocation*'
  • 100 fresh-process repetitions of the TypeRef zero-allocation and amortized-allocation harness tests
  • focused Release runs with server GC and a 256 MiB GC heap hard limit
  • dotnet run --project src/ILInspector.Analysis.Tests -c Release (1,148 total, 0 failed, 8 environment-dependent corpus skips)

richlander and others added 3 commits August 24, 2026 08:48
Run allocation probes on an isolated thread and require steady-state zero-allocation batches so tiered runtime work cannot contaminate the asserted sample. Keep a non-vacuity test that proves persistent operation allocations remain visible.

Fixes #4630

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use a fixed 10,000-call warmup and fixed 10,000-call measurement rather than searching for a quiet batch, preserving sensitivity to amortized allocations. Propagate isolated-thread exceptions and timeouts for attributable failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@richlander

Copy link
Copy Markdown
Owner Author

Round 1 review reconciliation for locked head 80e5d9c223d0b61bed847888a6275c2f8b364b5f (base 570bcb491ceffbb368ee0a12a04edcee8b42de2a):

  • GPT-5.6 Sol: clean. Focused Release evidence passed, including 50 repetitions each under default runtime settings, DOTNET_TieredCompilation=0, and DOTNET_TieredPGO=0.
  • Claude Opus: accepted one blocking finding. A GC suspension can retire the measuring thread's unused allocation context and inflate GC.GetAllocatedBytesForCurrentThread() even when TypeRef allocates nothing. The dedicated thread does not prevent global GC suspension, and the 10x wider window increases exposure.

I independently reproduced the finding against this exact head with the real test under bounded allocation pressure: 97/1,000 failures, all showing the same false-positive shape (Actual: 8048). The current candidate is superseded.

The replacement will serialize the allocation gate, drain prior GC/finalizer work, and accept only a fixed-width sample taken while a verified bounded no-GC region remained active. Region mechanics will invalidate a bounded attempt; they will not convert into a product allocation result. The fixed 10,000-call sample and amortized-allocation non-vacuity gate remain.

Serialize the allocation gate and accept a fixed-width sample only while a bounded no-GC region remains active. Retry invalid region attempts without minimizing or discarding any valid non-zero product sample.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@richlander

Copy link
Copy Markdown
Owner Author

Round 2 replacement candidate pushed: exact head 05252f909af44458da1053c07e9cd86a3180bc6f, effective base 570bcb491ceffbb368ee0a12a04edcee8b42de2a. Resolution commit 05252f909 addresses the accepted GC allocation-context retirement finding; reviewer dispatch will wait for current-head ci-required and zero conflicts.

@richlander

Copy link
Copy Markdown
Owner Author

Round 2 fixed-head reviews dispatched at exact head 05252f909af44458da1053c07e9cd86a3180bc6f after current-head ci-required succeeded and GitHub reported zero conflicts.

@richlander

Copy link
Copy Markdown
Owner Author

Round 2 review reconciliation for exact head 05252f909af44458da1053c07e9cd86a3180bc6f (base 570bcb491ceffbb368ee0a12a04edcee8b42de2a):

  • GPT-5.6 Sol: clean. Focused, constrained-GC, 100 fresh-process, full-suite, and xUnit serialization evidence passed.
  • Claude Opus: clean. Reproduced allocation-context retirement and verified the replacement rejects every region-breaking path before accepting the counter; confirmed no no-GC region leaks, operation exceptions retain precedence, xUnit v3 fully awaits parallel collections before this serialized class, valid non-zero samples are never retried, and the amortized non-vacuity sample remains exactly observable.

No findings require action. Both required reviews are review-clean on the unchanged current head.

@richlander

Copy link
Copy Markdown
Owner Author

Approved clean-review carry-forward completed: merge-only head 4cad68f3f0c12e37903c0f9e1298726167dbc457 integrates exact origin/main tip 665da8399270e05a0a4729bb39b66b5de14d44fc. The landed range was analyzed as non-interacting with the allocation gate; the focused Release gate passed after integration. The two round-2 clean reviews carry to this head. Final readiness now waits for current-head ci-required and positive mergeability.

@richlander

Copy link
Copy Markdown
Owner Author

Approved clean-review carry-forward completed again: merge-only head 381dde09e8c2786f4720f9f1e0eeb7aa3239255d integrates exact origin/main tip 642568c831bd4aa96bf7a7a09205f79bdcd16426. The additional landed ranges were analyzed as non-interacting with the allocation gate; the focused Release gate passed after integration. Both round-2 clean reviews carry to this head. Final readiness waits for current-head ci-required and positive mergeability.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeRef shallow equality allocation gate flakes on Linux CI

1 participant