Print vlog message for all assumption reclamations - #24001
Merged
Conversation
Contributor
Author
|
@mpirvu please review this RAS change when you get time |
mpirvu
self-requested a review
May 28, 2026 20:46
mpirvu
reviewed
May 28, 2026
mpirvu
left a comment
mpirvu
left a comment
Contributor
There was a problem hiding this comment.
LGTM. There is a strange printf in the existing code. Is it possible that the containing method never gets executed?
| printf("Freeing Assumption 0x%" OMR_PRIxPTR " and next assumption is 0x%" OMR_PRIxPTR "\n", | ||
| (uintptr_t)assumptionList, (uintptr_t)next); | ||
|
|
||
| if (TR::Options::getCmdLineOptions()->getVerboseOption(TR_VerboseRuntimeAssumptions)) { |
Contributor
There was a problem hiding this comment.
Why is there a printf in this code? Because I have never seen this on the console, I assume this code never gets executed.
Contributor
Author
There was a problem hiding this comment.
I will look into this code path and remove the printf in any case.
Contributor
Author
There was a problem hiding this comment.
Looks like that code can only run under an option based on
This routine gets into the one you asked about (with
printf) a couple of levels down the call chain. So that explains why we have never seen the printf, but I will remove it anyway now.
Contributor
Author
There was a problem hiding this comment.
Updated commit to remove printf
The verbose log messages printed when runtime assumptions get reclaimed were not being emitted at all of the right spots in the code. This commit adds a message at the two additional places where runtime assumptions memory gets freed. Signed-off-by: Vijay Sundaresan vijaysun@ca.ibm.com
Contributor
|
jenkins test sanity.functional xlinux jdk26 |
vijaysun-omr
changed the title
Contributor
|
Tests have passed. Merging. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The verbose log messages printed when runtime assumptions get reclaimed were not being emitted at all of the right spots in the code. This commit adds a message at the two additional places where runtime assumptions memory gets freed.
Signed-off-by: Vijay Sundaresan vijaysun@ca.ibm.com