Skip to content

Rebase memory region API branch onto latest main to clean up PR diff#124926

Draft
Copilot wants to merge 25 commits intomainfrom
copilot/implement-memory-region-apis
Draft

Rebase memory region API branch onto latest main to clean up PR diff#124926
Copilot wants to merge 25 commits intomainfrom
copilot/implement-memory-region-apis

Conversation

Copy link
Contributor

Copilot AI commented Feb 26, 2026

Description

The PR branch had accumulated a massive diff (1,093 files, 53K+ insertions, 9K+ deletions) due to a bad merge from main that dragged unrelated changes into the PR. This commit resets the branch to latest main and re-applies only the 16 files relevant to the cDAC memory region API implementation.

Before: 1,093 files changed, 53,345 insertions, 9,120 deletions
After: 16 files changed, 1,262 insertions, 3 deletions

The actual PR content (memory region APIs) is unchanged — only the branch history was cleaned up:

  • docs/design/datacontracts/GC.md — new API types, data descriptors, globals, pseudocode
  • src/coreclr/gc/datadescriptor/ — native GC data descriptor definitions
  • src/native/managed/cdac/.../Contracts/GC/GC_1.csGetHandleTableMemoryRegions, GetGCBookkeepingMemoryRegions, GetGCFreeRegions implementations
  • src/native/managed/cdac/.../Legacy/SOSDacImpl.cs — legacy DAC integration
  • src/native/managed/cdac/tests/ — unit tests and dump tests

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 4 commits February 26, 2026 22:41
… 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 AI changed the title [WIP] Implement native cDAC Memory Region APIs Implement cDAC Memory Region APIs (GetHandleTableMemoryRegions, GetGCBookkeepingMemoryRegions, GetGCFreeRegions) Feb 26, 2026
@max-charlamb
Copy link
Member

@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>
@rcj1
Copy link
Contributor

rcj1 commented Feb 27, 2026

@copilot try again

…eeping entries, empty/multiple free regions

Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 27, 2026

@copilot try again

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 50041df6.


int maxSlots = 1;
if (IsServerGC())
maxSlots = (int)GetGCHeapCount();
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot this is incorrect. We need to ensure that we limit by the processor count in this case. See #124875

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

@rcj1
Copy link
Contributor

rcj1 commented Feb 27, 2026

@copilot fix the build break

@rcj1
Copy link
Contributor

rcj1 commented Feb 27, 2026

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>
@rcj1 rcj1 marked this pull request as ready for review February 27, 2026 02:44
Copilot AI review requested due to automatic review settings February 27, 2026 02:44
@rcj1 rcj1 marked this pull request as draft February 27, 2026 02:44
@max-charlamb
Copy link
Member

@copilot, rebase on main, fix any merge conflicts, and update to follow new styles

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
@rcj1
Copy link
Contributor

rcj1 commented Mar 7, 2026

@copilot try again

Copilot AI and others added 2 commits March 7, 2026 06:10
…le merge artifacts

Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
…ebug.ValidateHResult

Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 7, 2026

@copilot try again

Updated in 5bef595 and 5663378. Fixed the three ISOSDacInterface13 method implementations to use DacComNullableByRef<ISOSMemoryEnum> pattern (ppEnum.Interface = ...) with Debug.ValidateHResult for #DEBUG validation blocks, and DacComNullableByRef<ISOSMemoryEnum> legacyOut = new(isNullRef: false) for legacy calls. Also renamed memRegionmemRegions in ISOSMemoryEnum.Next. All 948 cDAC tests pass.

…plicate SOSMemoryRegion/ISOSMemoryEnum definitions

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label Mar 9, 2026
Copilot AI changed the title Implement cDAC Memory Region APIs (GetHandleTableMemoryRegions, GetGCBookkeepingMemoryRegions, GetGCFreeRegions) Implement ISOSDacInterface memory region enumeration APIs via cDAC Mar 10, 2026
This commit replaces the entire working tree with a clean version based on
the latest main, containing only the PR-relevant memory region API changes.
The previous merge from main introduced unrelated changes into the diff.
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