Skip to content

[cDAC] Reshape DacDbi API GetHeapSegments to EnumerateHeapSegments and implement in cDAC#128054

Open
Copilot wants to merge 3 commits into
mainfrom
copilot/implement-dacdbi-getheapsegments
Open

[cDAC] Reshape DacDbi API GetHeapSegments to EnumerateHeapSegments and implement in cDAC#128054
Copilot wants to merge 3 commits into
mainfrom
copilot/implement-dacdbi-getheapsegments

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 11, 2026

Existing native GetHeapSegments allocates - to avoid this, I reworked it into EnumerateHeapSegments which calls a callback on each segment to add it to a DacDbiArray structure (potentially allocating). There are a bunch of APIs like this.

…; implement in cDAC

Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot May 11, 2026 20:53
@rcj1 rcj1 changed the title Return IEnumerable from IGC.EnumerateHeapSegments instead of taking a callback [cDAC] Reshape GetHeapS\ May 11, 2026
@rcj1 rcj1 changed the title [cDAC] Reshape GetHeapS\ [cDAC] Reshape DacDbi API GetHeapSegments to EnumerateHeapSegments and implement in cDAC May 11, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

@rcj1 rcj1 added the NO-REVIEW Experimental/testing PR, do NOT review it label May 11, 2026
Copilot AI review requested due to automatic review settings May 11, 2026 22:24
@rcj1 rcj1 removed the NO-REVIEW Experimental/testing PR, do NOT review it label May 11, 2026
@rcj1 rcj1 marked this pull request as ready for review May 11, 2026 22:28
@rcj1 rcj1 requested review from hoyosjs, max-charlamb and noahfalk May 11, 2026 22:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces the DAC/DBI heap segment API pattern from “return an allocated array” (GetHeapSegments) to “enumerate via callback” (EnumerateHeapSegments) and wires up a corresponding managed cDAC implementation, contract support, and tests/mocks.

Changes:

  • Replaces IDacDbiInterface::GetHeapSegments with EnumerateHeapSegments(fpCallback, pUserData) across IDL/header, native DAC implementation, and RS consumption.
  • Adds IGC.EnumerateHeapSegments(GCHeapData) plus supporting types (GCSegmentClassification, GCHeapSegmentInfo) and implements the walk in GC_1.
  • Extends the cDAC test harness with a mock HeapSegment descriptor and adds unit tests covering workstation/server + regions/segments behaviors.
Show a summary per file
File Description
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Adds a mock HeapSegment layout and builder helpers to construct segment lists in tests.
src/native/managed/cdac/tests/GCTests.cs Adds unit tests validating segment enumeration behavior for wks/svr and regions/segments modes.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs Updates the COM surface to EnumerateHeapSegments and introduces CorDebugGenerationTypes for segment classification.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs Implements EnumerateHeapSegments in the managed cDAC DBI shim and adds DEBUG parity checking vs legacy DAC.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GC/GC_1.cs Implements raw per-heap segment list walking and classification (regions vs segments, readonly/non-GC, ephemeral marker).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IGC.cs Adds new public contract types and the EnumerateHeapSegments API.
src/coreclr/inc/dacdbi.idl Replaces GetHeapSegments with callback-based EnumerateHeapSegments and defines the callback typedef.
src/coreclr/debug/inc/dacdbiinterface.h Updates the native interface declaration and documents callback semantics.
src/coreclr/debug/di/process.cpp Converts RS heap-region enumeration to use the callback API and accumulates segments locally.
src/coreclr/debug/daccess/dacdbiimpl.h Updates the DAC implementation declaration to EnumerateHeapSegments.
src/coreclr/debug/daccess/dacdbiimpl.cpp Implements callback-based segment enumeration (no pre-count/alloc).
docs/design/datacontracts/GC.md Documents the new contract API and segment classification/ephemeral-marker behavior.

Copilot's findings

Comments suppressed due to low confidence (1)

docs/design/datacontracts/GC.md:318

  • The new HEAP_SEGMENT_FLAGS_READONLY constant is documented as type 'ulong', but HeapSegment.Flags is a pointer-sized value (TargetNUInt) and the implementation uses a uint constant (1) for masking. Please align the documented type with the actual contract/implementation (e.g., document it as a bit value usable with pointer-sized Flags) to avoid confusion for contract consumers.
| `HEAP_SEGMENT_FLAGS_READONLY` | ulong | `HeapSegment.Flags` bit identifying a readonly (e.g. frozen, non-GC) segment. | `1` |

```csharp
GCHeapType IGC.GetGCIdentifiers()
{
  • Files reviewed: 12/12 changed files
  • Comments generated: 4

Comment thread docs/design/datacontracts/GC.md
Comment thread src/coreclr/inc/dacdbi.idl
Copy link
Copy Markdown
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 A few small comments inline.

{
// Bounded traversal of the singly-linked HeapSegment list, guarding against cycles or
// corrupt links via a fixed iteration cap (MaxSegmentListIterations = 2048).
int iterationMax = MaxSegmentListIterations;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A large enough GC heap in regions mode can probably exceed this limit. Try a test app that creates 10+ GB of small objects to see how it behaves.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, with 10GB I get 2823 segments. In the native DAC we cap the number of regions at 8192. Maybe both should be just a little more liberal, say 16384?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The biggest app heaps I hear of today are in the 50-100GB range and we probably want to add headroom to grow even bigger. A limit at 50K segments would put us ~200GB.


[PreserveSig]
int GetHeapSegments(nint pSegments);
int EnumerateHeapSegments(delegate* unmanaged<ulong, ulong, int, uint, nint, void> fpCallback, nint pUserData);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: since the C# function pointer syntax doesn't allow naming the parameters it might help clarity to include a comment describing it.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants