Skip to content

Use SHash as DAC instance hash#125631

Merged
hoyosjs merged 1 commit into
dotnet:mainfrom
hoyosjs:juhoyosa/inst-table-shash
Mar 20, 2026
Merged

Use SHash as DAC instance hash#125631
hoyosjs merged 1 commit into
dotnet:mainfrom
hoyosjs:juhoyosa/inst-table-shash

Conversation

@hoyosjs

@hoyosjs hoyosjs commented Mar 16, 2026

Copy link
Copy Markdown
Member

Replaces the hand-rolled hash table implementation in DacInstanceManager withan SHash based implementation. This hash is more efficient for the high volume of find operations the DAC issues when verifying cached cross-process reads. During mini dump collection, DacInstanceManager is the central cache for all memory read from the target process. The hand-rolled hash table used a fixed bucket array that degraded quickly for Find and insertion operations.

Measured minidump collection against a repro app with 2.5k frame deep stacks over 50 threads and the speedup was roughly 9.5x.

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

Copilot AI left a comment

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.

Pull request overview

This PR replaces the std::unordered_map-based hash table in DacInstanceManager with an SHash-based implementation by removing the #define DAC_HASHTABLE that previously selected the hand-rolled hash table, activating the #else branch which now uses SHash instead of std::unordered_map. This yields a ~9.5x speedup for minidump collection on deep-stack scenarios.

Changes:

  • Remove #define DAC_HASHTABLE and the std::unordered_map / std::hash_compare based implementation, replacing it with SHash<DacInstanceSHashTraits>
  • Update all hash table operations (Add, Find, Supersede, Flush, ClearEnumMemMarker, DumpAllInstances) to use SHash APIs (LookupPtr, ReplacePtr, Remove, RemoveAll, Begin/End)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/coreclr/debug/daccess/dacimpl.h Replace std::unordered_map types with SHash<DacInstanceSHashTraits> and custom traits class
src/coreclr/debug/daccess/daccess.cpp Update all hash operations to SHash API calls

You can also share your feedback on Copilot code review. Take the survey.

Comment thread src/coreclr/debug/daccess/dacimpl.h
@hoyosjs

hoyosjs commented Mar 19, 2026

Copy link
Copy Markdown
Member Author

/ba-g iOS failures are deadletters.

@hoyosjs hoyosjs requested review from a team and noahfalk March 19, 2026 18:34
@hoyosjs hoyosjs merged commit 15c9f8d into dotnet:main Mar 20, 2026
101 of 109 checks passed
@hoyosjs hoyosjs deleted the juhoyosa/inst-table-shash branch March 20, 2026 23:28
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.