Skip to content

Replace CHashTable and CHashTableAndData with SHash - #136239

Merged
AaronRobinsonMSFT merged 13 commits into
dotnet:mainfrom
AaronRobinsonMSFT:arobins/remove-chash-tables
Oct 23, 2026
Merged

AaronRobinsonMSFT merged 13 commits into
dotnet:mainfrom
AaronRobinsonMSFT:arobins/remove-chash-tables

Conversation

@AaronRobinsonMSFT

@AaronRobinsonMSFT AaronRobinsonMSFT commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Changes

Replace the fixed-bucket debugger and profiler hash tables with growing SHash implementations.

  • Migrate debugger object/field caches and the EE pending-evaluation, module, method-info, and patch tables.
  • Migrate profiler allocation-by-class counts, preserving NOTHROW/OOM handling and reusing table capacity between collections.
  • Use owning COM enumeration snapshots, preserving reference ownership, locking, and clone/reset/skip behavior.
  • Keep patch slots in a separate contiguous array, preserving live slot indices across both SHash rehashing and array growth, duplicate breakpoint chains, and controller ordering.
  • Keep LookupPtr read-only and use callback-based Update for existing entries in the shared and NativeAOT SHash implementations. Use native iteration, DAC backing-storage enumeration, and retained-capacity clearing; document borrowed lookup-pointer lifetimes.
  • Pair interop-safe nothrow array allocation with matching deallocation, value-initialize reused patch slots, and retain the trivial-copyability assertion required by array relocation.

Removed

Delete CHashTable, CHashTableAndData<>, HASHENTRY, FREEHASHENTRY, HASHFIND, their pointer typedefs, the unused CNewData/CNewDataNoThrow allocators, and the profiler's CHashTableImpl wrapper.

Validation

Built Windows x64 Checked CoreCLR with Release libraries/host. Ran native debugger/SHash checks, cDAC unit tests, runtime loader and Hot Reload suites, managed stack-trace tests, live debugger scenarios, and profiler allocation/GC tests.

Observed performance

A native Windows x64 Checked /O2 lookup diagnostic compared the migrated DBI table with a synthetic legacy 101-bucket table: one million lookups per round, three rounds. Medians below are ns/lookup; lower is better.

Entries Legacy SHash Change
100 5.97 8.47 2.50 ns slower
10,000 184.33 11.34 16.3x faster
100,000 1,590.45 35.41 44.9x faster

These are local synthetic lookup measurements.

Known limitations

  • Collectible-module live debugging still reproduces the Debugger::TrapAllRuntimeThreads GC contract assertion previously reproduced on the original baseline. The profiler GC test also prints the same GetObjectGeneration diagnostics as the baseline, despite passing.
  • The Windows x86 GCBridge CI failure was not reproduced locally with pre-migration, migrated, or original CI runtimes. Whether the migration caused that failure remains unestablished.
  • Actual VS/WinDBG clients, debugger-driven ApplyChanges, and NativeAOT GC-stress execution were not exercised locally.

Related to #11056; the measurements above do not establish a fix for current ordinary Release exception-formatting performance. This broader migration supersedes #132264.

Resolves #11466

Note

This description includes content generated by GitHub Copilot.

AaronRobinsonMSFT and others added 6 commits October 21, 2026 10:51
Add index-based traversal, DAC backing-memory enumeration, and optional retained-capacity clearing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
Preserve COM enumeration ownership, patch-slot indices, and pending-evaluation ordering and OOM handling.

Validated ICorDebugEnum cloning/reset/skip, ICorDebugBreakpointEnum and ICorDebugStepperEnum enumeration, and ICorDebugEval::NewString.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
Use SHash for profiler allocation counts and remove CHashTable, CHashTableAndData, and their unused allocators.

Validated ICorProfilerCallback::ObjectsAllocatedByClass and allocation-statistics suppression. Full x64 Checked CoreCLR and Release libraries/host build passed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
Release interop-safe array storage when element construction fails.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
Enforce trivial copyability and hash-entry layout for byte-managed patch storage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
Remove const casts from debugger and profiler table access.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@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.

AaronRobinsonMSFT and others added 2 commits October 21, 2026 13:22
Unix VPTR macros declare an int constructor and suppress the implicit default constructor.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Replace CHashTable and CHashTableAndData with SHash Replace CHashTable and CHashTableAndData with SHash Oct 21, 2026
@AaronRobinsonMSFT
AaronRobinsonMSFT marked this pull request as ready for review October 21, 2026 21:02
Comment thread src/coreclr/debug/di/hash.cpp Outdated
@jkotas
jkotas self-requested a review October 21, 2026 21:39
Comment thread src/coreclr/debug/di/hash.cpp Outdated
Comment thread src/coreclr/inc/shash.h
AaronRobinsonMSFT and others added 2 commits October 21, 2026 15:25
Remove indexed traversal and debugger adapters. Release Cordb entries through the traits cleanup hook.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
Remove the callback API and reuse existing slots for replacement. Split trait-controlled cleanup from explicit cleanup overrides, and mark in-place updates as non-cleaning.

Validated Checked x64 runtime/DAC/DBI, native EE/DBI checks, and live ICorDebug scenarios. The cleanup-override check fails before the change and passes afterward. Optimized counter code is unchanged by the overload split.

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

Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
@steveisok
steveisok requested a review from a team October 22, 2026 01:27
@max-charlamb

Copy link
Copy Markdown
Member

This looks good to me from a cDAC perspective. I queued a runtime-diagnostics pipeline on this branch to verify.

Keep LookupPtr read-only and replace ReplacePtr and lookup-entry mutation casts with Update.

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

Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
@jkotas

jkotas commented Oct 23, 2026

Copy link
Copy Markdown
Member

/azp run runtime-coreclr outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Comment thread src/coreclr/inc/utilcode.h Outdated
Comment thread src/coreclr/nativeaot/Runtime/GcStressControl.cpp Outdated

@hoyosjs hoyosjs left a comment

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.

I like this better than the old design - that being said to document:

Enumerations will now report the data as seen by the snapshot - so it may miss new or see deleted cordb objects. To me this is ok and a design point. Debuggers might need to adjust to it though.

AaronRobinsonMSFT and others added 2 commits October 23, 2026 11:48
Use typed patch slots and value initialization; clarify borrowed lookup lifetimes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce4b0890-6559-4b90-b46f-11179e70f973

@jkotas jkotas left a comment

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.

LGTM!

It may be a good idea to run some debugger tests if you have not done it already.

@AaronRobinsonMSFT

Copy link
Copy Markdown
Member Author

@steveisok and @hoyosjs I've shared this PR with our VS peers as an FYI. I've tried a slew of scenarios locally, but obviously not every possible scenario.

@tommcdon tommcdon left a comment

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.

Changes look good to me. I didn't observe any negative impact to debugger scenarios.

@AaronRobinsonMSFT
AaronRobinsonMSFT merged commit 4eb45ba into dotnet:main Oct 23, 2026
109 of 111 checks passed
@github-project-automation github-project-automation Bot moved this to Done in AppModel Oct 23, 2026
@AaronRobinsonMSFT
AaronRobinsonMSFT deleted the arobins/remove-chash-tables branch October 23, 2026 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Poorly performing hash used in the DBI/EE

5 participants