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.

Sign up for GitHub

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

Jump to bottom

Replace debug printf log with trace #7076

Merged
merged 1 commit into from
Aug 1, 2023
Merged

Replace debug printf log with trace #7076

merged 1 commit into from
Aug 1, 2023

Conversation

kangyining
Copy link
Contributor

@kangyining kangyining commented Jul 28, 2023 •
edited
Loading

The print statements are replaced with tracepoints in the flip function within MemorySubSpaceSemiSpace.cpp. This will allow them to be enabled via the -Xtrace option instead of a compile time flag.

Note we might also want to remove debugTiltedScavenge flag
in the future.

@kangyining
Copy link
Contributor Author

kangyining commented Jul 28, 2023 •
edited
Loading

Work in progress. Will squash commits after comparing with the master build.

amicic
amicic reviewed Jul 28, 2023
View reviewed changes
gc/base/j9mm.tdf Outdated Show resolved Hide resolved
amicic
amicic reviewed Jul 28, 2023
View reviewed changes
gc/base/j9mm.tdf Outdated Show resolved Hide resolved
amicic
amicic reviewed Jul 28, 2023
View reviewed changes
gc/base/j9mm.tdf Outdated Show resolved Hide resolved
amicic
amicic reviewed Jul 28, 2023
View reviewed changes
gc/base/j9mm.tdf Outdated Show resolved Hide resolved
amicic
amicic reviewed Jul 28, 2023
View reviewed changes
gc/base/j9mm.tdf Outdated Show resolved Hide resolved
amicic
amicic reviewed Jul 28, 2023
View reviewed changes
gc/base/j9mm.tdf Outdated Show resolved Hide resolved
@amicic
Copy link
Contributor

amicic commented Jul 28, 2023

looks good, you can squash commits

@amicic
Copy link
Contributor

amicic commented Jul 28, 2023

@babsingh please, proceed with final review/merge

@kangyining kangyining changed the title WIP: replace debug printf log with trace replace debug printf log with trace Jul 28, 2023
babsingh
babsingh requested changes Jul 31, 2023
View reviewed changes
Copy link
Contributor

@babsingh babsingh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • PR description and commit message should match; they currently don't match.
  • Capitalize the first character: replace -> Replace.
  • Resolve feedback comments, which have been addressed.

gc/base/j9mm.tdf Show resolved Hide resolved
gc/base/MemorySubSpaceSemiSpace.cpp Outdated Show resolved Hide resolved
gc/base/MemorySubSpaceSemiSpace.cpp Outdated Show resolved Hide resolved
gc/base/MemorySubSpaceSemiSpace.cpp Outdated Show resolved Hide resolved
@babsingh
Copy link
Contributor

babsingh commented Jul 31, 2023 •
edited
Loading

The flip function inside gc/base/MemorySubSpaceSemiSpace.cpp originally uses debug flag and omrtty_prinf() function for printing log. Replace it with trace points here.

  • Typo: omrtty_prinf -> omrtty_printf
  • here can be removed.
  • Revised with a justification for the change: The print statements are replaced with tracepoints in the flip function within MemorySubSpaceSemiSpace.cpp. This will allow them to be enabled via the -Xtrace option instead of a compile time flag.

@kangyining kangyining force-pushed the fix_MSSSS_log_fix branch 2 times, most recently from 02df66c to 7f9ddf8 Compare July 31, 2023 17:14
@babsingh babsingh changed the title replace debug printf log with trace Replace debug printf log with trace Jul 31, 2023
@babsingh
Copy link
Contributor

jenkins build all

@babsingh
Copy link
Contributor

babsingh commented Aug 1, 2023

@kangyining Now, I see two commits. The new Merge branch 'eclipse:master' into fix_MSSSS_log_fix will need to be removed before I can merge.

@kangyining
Replace debug printf log with trace
dac230f 
The print statements are replaced with tracepoints in the flip function
within MemorySubSpaceSemiSpace.cpp. This will allow them to be enabled
via the -Xtrace option instead of a compile time flag.

Note we might also want to remove debugTiltedScavenge flag
in the future.
@kangyining
Copy link
Contributor Author

Squashed, thanks.

@babsingh
Copy link
Contributor

babsingh commented Aug 1, 2023

jenkins build all

@babsingh
Copy link
Contributor

babsingh commented Aug 1, 2023 •
edited
Loading

zOS PR build: https://ci.eclipse.org/omr/job/PullRequest-linux_390-64/4194

Known failure: #1435; unrelated to this PR.

@babsingh babsingh merged commit 43a2b1d into eclipse:master Aug 1, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amicic amicic amicic approved these changes

@babsingh babsingh babsingh approved these changes

@charliegracie charliegracie Awaiting requested review from charliegracie

@youngar youngar Awaiting requested review from youngar

Assignees
No one assigned
Labels
comp:gc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kangyining @amicic @babsingh