Make MethodDataObject::FillEntryDataForAncestor lazy per-level - #131176
Merged
Conversation
PR dotnet#130151 optimized FillEntryDataForAncestor to pre-resolve virtual slots whose final vtable entry is NULL (never overridden or called), which cannot be MethodImpl targets, so they are safe to fill even when a MethodImpl exists in the hierarchy. However, producing the null-slot bitmap was done on the initial MethodTable (m_pDeclMT), and to fill those slots across the hierarchy it added an eager loop that walked the entire parent chain on the first PopulateNextLevel call, defeating the class's lazy per-level design and adding tail latency. Since MethodDataObject retains a pointer to the initial MethodTable, the null slots for the entries introduced at any level can be computed on demand from m_pDeclMT's canonical MethodTable. Compute unprocessedNonOverriddenSlots per level instead: for each introduced virtual slot, when a MethodImpl exists at or below the current level (m_containsMethodImpl is monotonic across the upward walk), fill it only if its final vtable entry is NULL. This removes the eager full-chain walk, the null-slot bitvector, and its heap allocation, and restores one IntroducedMethodIterator per level while producing identical Decl/Impl data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6eeba00-418e-42e7-8e32-762bc1cedeb6
|
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. |
Contributor
|
Tagging subscribers to this area: @agocke |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors MethodTable::MethodDataObject::FillEntryDataForAncestor to preserve MethodDataObject’s intended lazy, one-ancestor-per-PopulateNextLevel() population behavior when MethodImpls are present in the inheritance chain, while retaining the earlier optimization that pre-resolves “final vtable slot is NULL” virtuals to avoid O(V*N) fallback costs.
Changes:
- Removes the eager full-parent-chain walk and the null-slot bitvector (and its potential heap allocation) previously done during the first
PopulateNextLevel()call. - When
m_containsMethodImplis true, performs the “NULL final slot” check on-demand per level usingm_pDeclMT’s canonical vtable to decide whether a virtual slot can be safely pre-resolved. - Keeps non-virtual slot pre-resolution restricted to the most-derived
m_pDeclMTlevel, as before.
AaronRobinsonMSFT
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #130151. That PR optimized
MethodTable::MethodDataObject::FillEntryDataForAncestorto pre-resolve virtual slots whose final vtable entry isNULL— slots that have never been overridden by a subclass nor called, and therefore cannot be the target of aMethodImpl, so they are safe to fill even when aMethodImplexists somewhere in the inheritance chain. This avoids the expensiveGetMethodDescForSlot_NoThrowfallback (O(N) in the number of methods in the type hierarchy) that otherwise makesMethodDataObjectpopulation O(V*N) for deep hierarchies with many un-overridden/uncalled virtuals — the shape that shows up in WPF startup (dotnet/wpf#11740).However, that implementation produced the null-slot bitmap on the initial
MethodTable(m_pDeclMT) and, to fill those slots across the hierarchy, added an eager loop that walked the entire parent chain (running anIntroducedMethodIteratorat every level) on the very firstPopulateNextLevelcall. That defeats the class's lazy, one-level-per-PopulateNextLeveldesign and adds tail latency.Change
MethodDataObjectalready retains a pointer to the initialMethodTable, so the "is this slot's final vtable entry NULL" question can be answered on demand fromm_pDeclMT's canonicalMethodTableat any level. This PR computes the set of un-overridden/uncalled slots per level instead:MethodImplexists at or below the current level, fill it only if its final vtable entry onm_pDeclMT's canonical MT isNULL.m_containsMethodImplis monotonic across the most-derived → base walk, so at each level it reflects exactly theMethodImpls that can redirect slots introduced at that level (aMethodImplat a more-base level targets lower slot numbers and cannot affect slots introduced further derived).This removes the eager full-chain walk, the null-slot bitvector, and its heap allocation, restoring a single
IntroducedMethodIteratorper level while producing identicalDecl/ImplMethodDescdata. Null slots are introduced at exactly one level (never overridden), so they are filled exactly once, at their introducing level.Net: -167 / +36 lines; the method is now allocation-free under its
LIMITED_METHOD_CONTRACT.Validation
Builds clean (CoreCLR Debug and Release, Windows x64).
Ran a .NET 10 self-contained WPF app on a locally-built .NET 11 runtime (only the app's own binaries stay .NET 10; the full .NET 11
Microsoft.NETCore.Appand .NET 11 WPF pack are used). WPF heavily exercises this exact path; the app renders correctly with no errors.Startup A/B vs current
main(swapping only the matchedclrjit/coreclr/System.Private.CoreLibtrio), interleaved, medians of 60 iterations:main(baseline)Performance-neutral-to-slightly-faster at the median, with a much tighter tail (max 1431 ms vs 3191 ms) from removing the eager full-chain walk. (Absolute numbers are inflated because the framework here is not R2R/PGO-optimized for a dev
coreclr; the comparison isolates this change.)Note
This pull request was authored with assistance from GitHub Copilot.