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

Tiered jitting: Implement additional profiler APIs #8474

Closed
noahfalk opened this issue Jul 4, 2017 · 10 comments
Closed

Tiered jitting: Implement additional profiler APIs #8474

noahfalk opened this issue Jul 4, 2017 · 10 comments
Assignees
Labels
area-Diagnostics-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@noahfalk
Copy link
Member

noahfalk commented Jul 4, 2017

Tiered jitting breaks a previous invariant that there would only be one jitted code body per <FunctionID, ReJitID, generic instantiation> tuple. In order to address this we need to add some new APIs that profilers can use to get information about these new method bodies. The breaking change doc added in dotnet/coreclr#12193 has a bit more background. The likely APIs (design not final) would be:

//Given functionId + rejitId, enumerate the native code start address of all jitted versions of this code that currently exist
ICorProfilerInfo9::GetNativeCodeStartAddresses(FunctionID functionID, ReJITID reJitId, ULONG32 cCodeStartAddresses, ULONG32 *pcCodeStartAddresses, UINT_PTR codeStartAddresses[]);

//Given the native code start address, return the native->IL mapping information for this jitted version of the code
ICorProfilerInfo9::GetILToNativeMapping3(UINT_PTR pNativeCodeStartAddress, ULONG32 cMap, ULONG32 *pcMap, COR_DEBUG_IL_TO_NATIVE_MAP map[]);

//Given the native code start address, return the the blocks of virtual memory that store this code (method code is not necessarily stored in a single contiguous memory region)
ICorProfilerInfo9::GetCodeInfo4(UINT_PTR pNativeCodeStartAddress, ULONG32 cCodeInfos, ULONG32* pcCodeInfos, COR_PRF_CODE_INFO codeInfos[]);

There may also be a tiered compilation opt-out mechanism added to the profiler API, but any work for that is tracked under #8473.

@noahfalk
Copy link
Member Author

noahfalk commented Jul 4, 2017

@mjsabby @SergeyKanzhelev @discostu105 - Figured you guys might have particular interest in these. Let me know if you think the APIs I suggested above sound reasonable.

Also please point out if you think there are any gaps that need addressing. For example, do we need a new API that maps code pointer in method -> code start address? As-is if you wanted to do that transformation you would need to do a multi-step process like this:
a) GetFunctionFromIP2 to get FunctionID+rejitID
b) GetCodeNativeCodeStartAddresses to map (FunctionID+rejitID) -> list of start addresses
c) Iterate GetCodeInfo4 to convert list of start addresses -> Dictionary<list of code regions, start address>
d) Lookup code pointer to determine which region it was in, then map that back to the start address with the dictionary you made.

@SergeyKanzhelev
Copy link

CC: @jacdavis
Are those methods used for anything else? Sorry for ignorance, I haven't looked into code, but why not simply make this work?

@noahfalk
Copy link
Member Author

noahfalk commented Jul 5, 2017

ICorProfilerFunctionControl::SetILInstrumentedCodeMap will work, but it is orthogonal to the APIs above. The APIs above would be used in scenarios where you previously used GetILToNativeMapping1/2 and GetCodeCodeInfo1/2/3. For example in the past to map a native offset to an IL offset, may have called:

  1. GetFunctionFromIP to learn the FunctionID
  2. GetCodeInfo to determine the segments of memory this function uses for code, so that you could compute a native offset within the method
  3. GetILToNativeMapping to compute the IL offset for that native offset.

That flow breaks down as soon as you can have more than one jitted method body for the same FunctionID (and same rejitid). You need a stronger identifier to indicate which jitted method body you are trying to refer to, because each jitted method body may have a different native->IL map and definately will have different memory bounds.

@SergeyKanzhelev
Copy link

Just to confirm - when you call to ICorProfilerFunctionControl::SetILInstrumentedCodeMap it set's the mapping between new IL to old IL. And new IL is mapped to the native code. So customer's original instructions and source code will be mapped to the Native code by proxy.

Is it fair to say you only need proposed API when you change method body drastically? By drastically I mean not making an additive code modification like inserting prefix, postfix or something in the middle. You need new API when you inserting new code that should be debuggable and need it's own mapping to Native Code.

@noahfalk
Copy link
Member Author

noahfalk commented Jul 6, 2017

Is it fair to say you only need proposed API when you change method body drastically?

Nope : ) It sounds like you are analyzing this from the perspective of using ReJIT in the profiler, but ignore that part for the moment. Imagine that you have a simple profiler that never uses ReJIT or any form of IL instrumentation. The changes to the method body aren't coming from your profiler, they are coming from the runtime when it jits a method again using different jit optimization flags. If that hypothetical simple profiler was using GetILToNativeMapping or GetCodeInfo before, it probably needs to be using the new APIs now.

Does your profiler use GetILToNativeMapping or GetCodeInfo today?

@SergeyKanzhelev
Copy link

no. Sorry

@noahfalk
Copy link
Member Author

noahfalk commented Jul 6, 2017

Nothing to be sorry about - thats good, it means you probably don't need to worry about these changes.

@mjsabby
Copy link
Contributor

mjsabby commented Jul 25, 2017

@noahfalk I think we're good on this, we do use the APIs you mentioned and I'll update it. Conceptually things should work for our profilers but have to make code changes.

@lt72 lt72 assigned noahfalk and davmason and unassigned noahfalk Oct 31, 2017
@lt72
Copy link
Contributor

lt72 commented Nov 7, 2017

@davmason: this should be complete, but for the tests... ?

@noahfalk
Copy link
Member Author

noahfalk commented Feb 6, 2018

Looping back to this. We ran the tests and found a bug in the feature, which @kouvel has now fixed (#16145). Unfortunately this fix didn't make the Preview 1 build, but it should be available for any future preview builds or the 2.1 release build. I think all the work here is done now.

@davmason @lt72 @mjsabby

@noahfalk noahfalk closed this as completed Feb 6, 2018
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Diagnostics-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

6 participants