This repository was archived by the owner on Jan 23, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Inliner refactoring: consolidate logging, reporting and dumping #3051
Merged
Conversation
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
@dotnet/jit-contrib PTAL |
src/jit/compiler.cpp
Outdated
* JitInlineResult::report | ||
* | ||
* Dump, log, and report information about an inline decision. | ||
*/ |
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.
new code should use the new function header comment format
LGTM, modulo nits. Do you see any asm diffs due to different messages? |
Running diffs now, don't expect to see anything. |
00d6085
to
670829f
Compare
Updated per feedback. May not have x64 diffs for a while, I'm caught on the wrong side the GUID change. I did verify there we no inlining diffs crossgenning mscorlib. |
With this change the responsibility for dumping, logging, and reporting inline decisions is moved into the JitInlineResult class instead of being distributed throughout the code base. This insures all the relevant inline information is handled in a consistent manner. The JitInlineResult is updated to hold a compiler instance and now requires a context string describing the what the jit is doing as it evaluates an inline candidate. There are 4 distinct contexts in use currently.
670829f
to
82ec59e
Compare
Fixed ctor initializer ordering. |
@BruceForstall no x64 diffs via ngen... |
LGTM |
AndyAyersMS
added a commit
that referenced
this pull request
Feb 6, 2016
Inliner refactoring: consolidate logging, reporting and dumping
AndyAyersMS
added a commit
to AndyAyersMS/coreclr
that referenced
this pull request
Feb 17, 2016
In dotnet#3051 the jit started calling `eeGetClassFullName` for both caller and callee handles even when dumping was disabled, passed this information to the logging messages where previously the caller and callee were identified via other context, and prepared the full name dump artifacts for all inline decisions rather than the selective cases handled previously. This change caused excessive memory use in some tests. This change makes the calls to `eeGetClassFullName` conditional on whether dumping is enabled, and does not require these calls for logging messages.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
With this change the responsibility for dumping, logging, and reporting
inline decisions is moved into the JitInlineResult class instead of being
distributed throughout the code base. This insures all the relevant inline
information is handled in a consistent manner.
The JitInlineResult is updated to hold a compiler instance and now
requires a context string describing the what the jit is doing as it evaluates
an inline candidate. There are 4 distinct contexts in use currently.