Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cdac] Handle hot/cold map lookup in ExecutionManager.ReadyToRunJitManager.GetMethodInfo #110087

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Nov 22, 2024

Implement the hot/cold lookup logic for ExecutionManager.ReadyToRunJitManager.GetMethodInfo. Basic logic is now:

  1. Check if the address is in a thunk for READYTORUN_HELPER_DelayLoad_MethodCall
  2. Find the runtime function entry corresponding to the address
    • If the runtime function entry is cold code, find the runtime function entry for the corresponding hot part
  3. Look up the MethodDesc for the entry point using the ReadyToRunInfo's hash map
  4. Compute the relative offset based the function's start address
    • If the runtime function has a cold part and the address is in the cold part, compute the relative offset based on the size of the hot part and the offset from the start of the cold part

Sub-bullets of 2 and 4 are the parts added by this change.

Add tests for ExecutionManager for getting code blocks and method desc in R2R with hot/cold splitting, runtime functions lookup functionality, and hot/cold map lookup logic.

  • Pull out helper for RuntimeFunctions mock memory
  • ExecutionManagerTests just uses one specific runtime function / unwind info layout
    • The RuntimeFunctionsTests handle testing the matrix of different layouts

The RuntimeFunction and UnwindInfo structures are different depending on the platform. This matters for calculating the function length. To facilitate testing, I pulled out the runtime functions lookup into a helper class. This change puts reading the hot/cold map in a helper class HotColdLookup. Having separate helpers (similarly, HashMapLookup and NibbleMap) should let us mock them out for testing if we want - there'd still be some work for how they are created (for example, tests need to be able to provide some instance instead of the contract directly creating the class), but it puts us in a reasonable position.

Manually validated with !ip2md (temporarily commented out throwing not implemented for rejit ID requests) for an app published with R2R --hot-cold-splitting that we correctly map addresses in cold/hot blocks to hot/cold parts, determine their start addresses and sizes, and return the correct method desc.

Contributes to #99302, #109426

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 22, 2024
@elinor-fung elinor-fung added area-Diagnostics-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 22, 2024
@elinor-fung elinor-fung added this to the 10.0.0 milestone Nov 22, 2024
Copy link
Contributor

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.


namespace Microsoft.Diagnostics.DataContractReader.ExecutionManagerHelpers;

internal static class BinaryThenLinearSearch
Copy link
Member Author

Choose a reason for hiding this comment

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

If anyone has a better name...

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.

2 participants