Implement cDAC Memory Region APIs (GetHandleTableMemoryRegions, GetGCBookkeepingMemoryRegions, GetGCFreeRegions)#124926
Implement cDAC Memory Region APIs (GetHandleTableMemoryRegions, GetGCBookkeepingMemoryRegions, GetGCFreeRegions)#124926
Conversation
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
… for memory region types Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
…p free region fields, and IGC contract methods Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
…Impl integration Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Add GCMemoryRegionTests with three test methods covering: - GetHandleTableMemoryRegions: single map, bucket, and segment - GetGCBookkeepingMemoryRegions: single card table entry - GetGCFreeRegions: workstation GC with one free region All tests use [Theory] with [ClassData(typeof(MockTarget.StdArch))] to run across all four architecture combinations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@copilot , we need to update the cDAC docs and write tests for these new APIs |
Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
|
@copilot try again |
…eeping entries, empty/multiple free regions Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Updated the cDAC docs (GC.md) with new API types, data descriptors, globals, constants, and pseudocode for all three methods. Added 6 additional test methods covering edge cases (empty buckets, multiple linked segments, multiple bookkeeping entries, empty/multiple free regions). Total: 9 test methods × 4 architectures = 36 tests, all passing. See commit |
docs/design/datacontracts/GC.md
Outdated
|
|
||
| int maxSlots = 1; | ||
| if (IsServerGC()) | ||
| maxSlots = (int)GetGCHeapCount(); |
There was a problem hiding this comment.
Fixed in 6320ebd1. Changed maxSlots to read from TotalCpuCount (the processor count) instead of using GetGCHeapCount() for server GC, matching the fix in PR #124875. Added the TotalCpuCount global pointer to datadescriptor.inc (under SERVER_GC), Constants.Globals, and updated the GC.md pseudocode accordingly.
|
@copilot fix the build break |
|
Ah I will merge my changes into main and then this should work better |
…it in server GC Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
…24875 infrastructure Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Rewrite test infrastructure usage to follow the proper pattern: - Use builder.CreateAllocator() instead of non-existent builder.BumpAllocator - Use builder.AddHeapFragment() to register allocated fragments - Use builder.BorrowAddressRange() + helpers to write data via Span<byte> - Use 32-bit compatible address ranges (0x0010_0000-0x0100_0000) - Wrap fragment addresses in TargetPointer for Assert.Equal comparisons Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…als, and pseudocode Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Started over from the latest |
|
@copilot fix the build break |
…d globals - Wrap FreeableSohSegment/FreeableUohSegment in #ifdef BACKGROUND_GC - Wrap FreeRegions/GlobalFreeHugeRegions/GlobalRegionsToDecommit/CountFreeRegionKinds in #ifdef USE_REGIONS - Change BookkeepingStart from GC_HEAP_FIELD to static pointer (it's PER_HEAP_ISOLATED) - Move BookkeepingStart to unconditional global pointer (not WKS-only) - Remove BookkeepingStart from IGCHeap interface (it's a global, not per-heap) - Simplify GetGCBookkeepingMemoryRegions to always use global pointer Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Fixed in
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot still broken |
…unt_free_region_kinds Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Fixed in |
max-charlamb
left a comment
There was a problem hiding this comment.
@copilot address feedback
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| int ISOSMemoryEnum.Next(uint count, SOSMemoryRegion[] memRegion, uint* pNeeded) |
There was a problem hiding this comment.
Can we handle a nullptr passed to memRegion like the DAC could?
There was a problem hiding this comment.
Fixed in 99dd6293. Added if (memRegion is null) return HResults.E_POINTER; check before accessing the array.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs
Show resolved
Hide resolved
…egion, add #DEBUG blocks, fix GC.md docs Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
All five feedback items addressed in
|
|
@copilot, can you add cDAC dumptests for these three new APIs |
…MemoryRegions, GetGCFreeRegions Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Added in
|
Description
Three
ISOSDacInterface13APIs (GetHandleTableMemoryRegions,GetGCBookkeepingMemoryRegions,GetGCFreeRegions) previously delegated to the legacy DAC or returnedE_NOTIMPL. This PR implements them natively in the cDAC by extending theIGCcontract and following the existingSOSMethodEnumpattern. Re-implemented on top of latestmainwhich includes PR #124875 infrastructure (handle table data types,TotalCpuCountglobal, etc.).Managed interface definitions (
ISOSDacInterface.cs)SOSMemoryRegionstruct andISOSMemoryEnumCOM interfaceISOSDacInterface13method signatures fromvoid**to typedout ISOSMemoryEnum?GC data descriptors (
datadescriptor.inc,datadescriptor.h)CDAC_TYPEblocks forCardTableInfoandRegionFreeListGCHeapfields (with conditional compilation guards):FreeableSohSegment,FreeableUohSegment(#ifdef BACKGROUND_GC),FreeRegions(#ifdef USE_REGIONS)BookkeepingStartas a static pointer to thePER_HEAP_ISOLATEDfield (unconditional global pointer, not a per-heap field)BookkeepingStart(unconditional),GlobalFreeHugeRegions,GlobalRegionsToDecommit(#ifdef USE_REGIONS), workstation free-region globals (GCHeapFreeableSohSegmentunderBACKGROUND_GC,GCHeapFreeRegionsunderUSE_REGIONS)HandleSegmentSize,CardTableInfoSize,CountFreeRegionKinds(#ifdef USE_REGIONS, usingFREE_REGION_KINDSmacro)Managed Data classes and type registration
CardTableInfo.cs,RegionFreeList.csDataTypeenum entries forCardTableInfo,RegionFreeListConstants.Globalsentries for all new globalsIGCHeap/GCHeapSVR/GCHeapWKSextended with optional fields:FreeableSohSegment,FreeableUohSegment,FreeRegionsIGC contract extension (
IGC.cs,GC_1.cs)GCMemoryRegionData,FreeRegionKindtypesGetHandleTableMemoryRegions(): walkshandle_table_map→ buckets → tables → segments (cap: 8192), usingTotalCpuCount(processor count) for the number of table slots per bucket in server GC (per PR Fix GC handle dac bug #124875)GetGCBookkeepingMemoryRegions(): readsbookkeeping_startfromBookkeepingStartglobal pointer (works for both WKS and server GC since it'sPER_HEAP_ISOLATED), walkscard_table_infolinked list (cap: 32)GetGCFreeRegions(): enumerates global free huge regions, global regions to decommit, per-heap free regions + freeable SOH/UOH segments for both workstation and server GC (cap: 2048)SOSMemoryEnum and SOSDacImpl integration
SOSMemoryEnum:[GeneratedComClass]implementingISOSMemoryEnumwithNext/Skip/Reset/GetCountSOSMemoryEnumconstructor acceptsTargetand usesToClrDataAddress(target)for proper sign extension on 32-bit targetsSOSMemoryEnum.Nexthandles nullmemRegionparameter by returningE_POINTERSOSDacImplmethods now callIGCcontract methods, wrap results inSOSMemoryEnum, and include#if DEBUGcross-validation blocks against_legacyImpl13cDAC documentation (
GC.md)FreeRegionKind,GCMemoryRegionData) and method signatures to the contract API sectionCardTableInfo,RegionFreeList, and newGCHeapfields (FreeableSohSegment,FreeableUohSegment,FreeRegions)HandleSegmentSize,CardTableInfoSize,CountFreeRegionKinds,BookkeepingStart,GlobalFreeHugeRegions,GlobalRegionsToDecommit, workstation free-region globals) and safety-cap constantsBookkeepingStartcorrectly listed only in the globals table (not as a GCHeap field, since it'sPER_HEAP_ISOLATED)GetHandleTableMemoryRegions,GetGCBookkeepingMemoryRegions,GetGCFreeRegionsand helper methods (AddFreeList,AddSegmentList)Native build fixes
#ifdef BACKGROUND_GCguards forFreeableSohSegment/FreeableUohSegmentfields and WKS global pointers#ifdef USE_REGIONSguards forFreeRegions,GlobalFreeHugeRegions,GlobalRegionsToDecommit,CountFreeRegionKindsBookkeepingStartfromGC_HEAP_FIELDtostatic constexpr decltype(...)pointer (it'sPER_HEAP_ISOLATED, not a per-heap instance field)BookkeepingStartto unconditionalCDAC_GLOBAL_POINTER(accessible in both WKS and SERVER_GC builds)FREE_REGION_KINDSmacro (defined at global scope) instead ofcount_free_region_kindsenum (which is insideGC_NAMESPACEand not accessible in the datadescriptor context)Unit tests (
GCMemoryRegionTests.cs)GetHandleTableMemoryRegions: single segment, all-null buckets (empty result), multiple linked segmentsGetGCBookkeepingMemoryRegions: single card table entry, missing global (empty result)GetGCFreeRegions: single free region, no free regions (empty result), multiple free regions across kinds with linked segmentsDump tests (
ServerGCDumpTests.cs,WorkstationGCDumpTests.cs)ServerGCDumpTestsand 3 toWorkstationGCDumpTestscovering all three new APIsGetHandleTableMemoryRegions: asserts at least one region with non-null start and non-zero sizeGetGCBookkeepingMemoryRegions: asserts at least one region with non-null start and non-zero sizeGetGCFreeRegions: validates any returned regions have non-null start and non-zero size (count may be zero since free regions depend on GC state)💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.