-
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 1 commit
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 |
---|---|---|
|
@@ -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,14 @@ class ICorJitInfo : public ICorDynamicInfo | |
UINT32 ExecutionCount; | ||
}; | ||
|
||
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. It would be helpful to comment this structure and its fields. E.g., what is Is UINT32 big enough for 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. I'll add some description. For count overflow I currently don't have great answers; it should be rare (we'd need methods that spent a lot of cycles in Tier0) but obviously it's not impossible. Right now overflow won't cause any crashes but it might lead to nonsensical profile data. |
||
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 +275,17 @@ 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. | ||
virtual CORINFO_CLASS_HANDLE getLikelyClass( | ||
CORINFO_METHOD_HANDLE ftnHnd, | ||
CORINFO_CLASS_HANDLE baseHnd, | ||
UINT32 ilOffset, | ||
UINT32 * pLikelihood, // estimated likelihood of the class (0...100) | ||
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. Clarify this is a decimal percentage? i.e. 100 = 100% certainty. 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. Clarify these are OUT params |
||
UINT32 * pNumberOfClasses // 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.