-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Initial version of class profiling for PGO #45133
Changes from all commits
3c2f24a
2b0b1ca
1533a1d
b357320
f490d8c
48d757b
b72de10
7b7efc9
23f26bf
95c6d8e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -208,11 +208,11 @@ TODO: Talk about initializing strutures before use | |
// | ||
////////////////////////////////////////////////////////////////////////////////////////////////////////// | ||
|
||
constexpr GUID JITEEVersionIdentifier = { /* 8031aa05-4568-40fc-a0d2-d971d8edba16 */ | ||
0x8031aa05, | ||
0x4568, | ||
0x40fc, | ||
{0xa0, 0xd2, 0xd9, 0x71, 0xd8, 0xed, 0xba, 0x16} | ||
constexpr GUID JITEEVersionIdentifier = { /* 0d235fe4-65a1-487a-8553-c845496da901 */ | ||
0x0d235fe4, | ||
0x65a1, | ||
0x487a, | ||
{0x85, 0x53, 0xc8, 0x45, 0x49, 0x6d, 0xa9, 0x01} | ||
}; | ||
|
||
////////////////////////////////////////////////////////////////////////////////////////////////////////// | ||
|
@@ -623,6 +623,7 @@ enum CorInfoHelpFunc | |
CORINFO_HELP_STACK_PROBE, // Probes each page of the allocated stack frame | ||
|
||
CORINFO_HELP_PATCHPOINT, // Notify runtime that code has reached a patchpoint | ||
CORINFO_HELP_CLASSPROFILE, // Update class profile for a call site | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need to change the JIT-EE GUID |
||
|
||
CORINFO_HELP_COUNT, | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -251,6 +251,34 @@ class ICorJitInfo : public ICorDynamicInfo | |
UINT32 ExecutionCount; | ||
}; | ||
|
||
// Data structure for a single class probe. | ||
// | ||
// ILOffset is the IL offset in the method for the call site being probed. | ||
// Currently it must be ORed with CLASS_FLAG and (for interface calls) | ||
// INTERFACE_FLAG. | ||
// | ||
// Count is the number of times a call was made at that call site. | ||
// | ||
// SIZE is the number of entries in the table. | ||
// | ||
// SAMPLE_INTERVAL must be >= SIZE. SAMPLE_INTERVAL / SIZE | ||
// gives the average number of calls between table updates. | ||
// | ||
struct ClassProfile | ||
{ | ||
enum { | ||
SIZE = 8, | ||
SAMPLE_INTERVAL = 32, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do these values need to be powers of two? For example could SIZE be 7 here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They can be any positive value provided N >= S. |
||
CLASS_FLAG = 0x80000000, | ||
INTERFACE_FLAG = 0x40000000, | ||
OFFSET_MASK = 0x3FFFFFFF | ||
}; | ||
|
||
UINT32 ILOffset; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like pgo.cpp (presumably elsewhere) has some "magic" code related to this: "(classProfile->ILOffset & 0x40000000)". Should that be specified here (as documentation and code) and the magic code use some member function to do this check? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, will do. It's currently done so that we can allocate class profiles and block counts as one big chunk without telling the runtime how many of either exist. The high bit tells us this is a class profile, and the next bit which kind. This magic will go away once we get more of the runtime side implemented and can specify a more flexible "schema" for the profile data. |
||
UINT32 Count; | ||
CORINFO_CLASS_HANDLE ClassTable[SIZE]; | ||
}; | ||
|
||
// allocate a basic block profile buffer where execution counts will be stored | ||
// for jitted basic blocks. | ||
virtual HRESULT allocMethodBlockCounts ( | ||
|
@@ -267,6 +295,24 @@ class ICorJitInfo : public ICorDynamicInfo | |
UINT32 * pNumRuns // pointer to the total number of profile scenarios run | ||
) = 0; | ||
|
||
// Get the likely implementing class for a virtual call or interface call made by ftnHnd | ||
// at the indicated IL offset. baseHnd is the interface class or base class for the method | ||
// being called. May returns NULL. | ||
// | ||
// pLikelihood is the estimated percent chance that the class at runtime is the class | ||
// returned by this method. A well-estimated monomorphic call site will return a likelihood | ||
// of 100. | ||
// | ||
// pNumberOfClasses is the estimated number of different classes seen at the site. | ||
// A well-estimated monomorphic call site will return 1. | ||
virtual CORINFO_CLASS_HANDLE getLikelyClass( | ||
CORINFO_METHOD_HANDLE ftnHnd, | ||
CORINFO_CLASS_HANDLE baseHnd, | ||
UINT32 ilOffset, | ||
UINT32 * pLikelihood, // OUT, estimated likelihood of the class (0...100) | ||
UINT32 * pNumberOfClasses // OUT, estimated number of possible classes | ||
) = 0; | ||
|
||
// Associates a native call site, identified by its offset in the native code stream, with | ||
// the signature information and method handle the JIT used to lay out the call site. If | ||
// the call site has no signature information (e.g. a helper call) or has no method handle | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be "almost diffable" with icorjitinfo.h, so add
ClassProfile
struct as wellThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented out, like BlockCounts is ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If necessary. I can't remember why BlockCounts is commented out; if it's required or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess if #45044 is merged, it will be unnecessary.