Skip to content

Remove print!/println! code from mmtk-core #655

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

Merged
merged 4 commits into from
Aug 31, 2022
Merged

Remove print!/println! code from mmtk-core #655

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
52bde66
Remove print!/println! code from mmtk-core
qinsoon Aug 31, 2022
1f9f02f
Use Write trait for print_vm_map
qinsoon Aug 31, 2022
9e56e09
Put print_vm_map to impl dyn Space
qinsoon Aug 31, 2022
9f9c18b
Make print_vm_map a top-level function
qinsoon Aug 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/plan/nogc/global.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl<VM: VMBinding> Plan for NoGC<VM> {
}

fn handle_user_collection_request(&self, _tls: VMMutatorThread, _force: bool) {
println!("Warning: User attempted a collection request, but it is not supported in NoGC. The request is ignored.");
warn!("User attempted a collection request, but it is not supported in NoGC. The request is ignored.");
}
}

Expand Down
54 changes: 28 additions & 26 deletions src/policy/space.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,28 +608,34 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
panic!("A copying space should override this method")
}

fn print_vm_map(&self) {
fn print_vm_map(&self) -> String {
Copy link
Collaborator

@wks wks Aug 31, 2022

Choose a reason for hiding this comment

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

A more general solution is to write to a &mut impl Write. It can write to a string in this way, and also a file, too, if the user desires. The Write trait still allows us to use the write! or writeln! macros for formatting, like the print! macro in the old code.

Example:

use std::fmt::{Error, Write};

fn print_something(out: &mut impl Write) -> Result<(), Error> {
    writeln!(out, "Hello world!")?;
    writeln!(out, "Number: {}", 42)?;

    Ok(())
}

fn main() {
    let mut s = String::new();
    print_something(&mut s).unwrap();
    println!("{}", s);
}

Copy link
Member Author

@qinsoon qinsoon Aug 31, 2022

Choose a reason for hiding this comment

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

Good idea. I changed the method. However, as we cannot have methods with generic type parameters (including impl T) in a object-safe trait (Space), I moved the method outside the trait.

let mut ret = String::new();
let common = self.common();
print!("{} ", common.name);
ret.push_str(common.name);
ret.push(' ');
if common.immortal {
print!("I");
ret.push('I');
} else {
print!(" ");
ret.push(' ');
}
if common.movable {
print!(" ");
ret.push(' ');
} else {
print!("N");
ret.push('N');
}
print!(" ");
ret.push(' ');
if common.contiguous {
print!("{}->{}", common.start, common.start + common.extent - 1);
ret.push_str(&format!(
"{}->{}",
common.start,
common.start + common.extent - 1
));
match common.vmrequest {
VMRequest::Extent { extent, .. } => {
print!(" E {}", extent);
ret.push_str(&format!(" E {}", extent));
}
VMRequest::Fraction { frac, .. } => {
print!(" F {}", frac);
ret.push_str(&format!(" F {}", frac));
}
_ => {}
}
Expand All @@ -639,18 +645,19 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
.common()
.get_head_discontiguous_region();
while !a.is_zero() {
print!(
ret.push_str(&format!(
"{}->{}",
a,
a + self.common().vm_map().get_contiguous_region_size(a) - 1
);
));
a = self.common().vm_map().get_next_contiguous_region(a);
if !a.is_zero() {
print!(" ");
ret.push(' ');
}
}
}
println!();
ret.push('\n');
ret
}

/// Ensure that the current space's metadata context does not have any issues.
Expand Down Expand Up @@ -713,9 +720,6 @@ pub struct SpaceOptions {
pub side_metadata_specs: SideMetadataContext,
}

/// Print debug info for SFT. Should be false when committed.
const DEBUG_SPACE: bool = cfg!(debug_assertions) && false;

impl<VM: VMBinding> CommonSpace<VM> {
pub fn new(
opt: SpaceOptions,
Expand Down Expand Up @@ -805,15 +809,13 @@ impl<VM: VMBinding> CommonSpace<VM> {
panic!("failed to mmap meta memory");
}

if DEBUG_SPACE {
println!(
"Created space {} [{}, {}) for {} bytes",
rtn.name,
start,
start + extent,
extent
);
}
debug!(
"Created space {} [{}, {}) for {} bytes",
rtn.name,
start,
start + extent,
extent
);

rtn
}
Expand Down
3 changes: 3 additions & 0 deletions src/util/sanity/memory_scan.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use crate::util::Address;
use crate::util::ObjectReference;

// This is legacy code, and no one is using this. Using gdb can achieve the same thing for debugging.
// The JikesRVM binding still declares this method and we need to remove it from JikesRVM.
#[deprecated]
pub fn scan_region() {
loop {
let mut buf = String::new();
Expand Down
8 changes: 3 additions & 5 deletions src/util/statistics/stats.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl Stats {
}
self.shared.increment_phase();
} else if !self.exceeded_phase_limit.load(Ordering::SeqCst) {
println!("Warning: number of GC phases exceeds MAX_PHASES");
eprintln!("Warning: number of GC phases exceeds MAX_PHASES");
self.exceeded_phase_limit.store(true, Ordering::SeqCst);
}
}
Expand All @@ -178,7 +178,7 @@ impl Stats {
}
self.shared.increment_phase();
} else if !self.exceeded_phase_limit.load(Ordering::SeqCst) {
println!("Warning: number of GC phases exceeds MAX_PHASES");
eprintln!("Warning: number of GC phases exceeds MAX_PHASES");
self.exceeded_phase_limit.store(true, Ordering::SeqCst);
}
}
Expand Down Expand Up @@ -232,9 +232,7 @@ impl Stats {
pub fn start_all(&self) {
let counters = self.counters.lock().unwrap();
if self.get_gathering_stats() {
println!("Error: calling Stats.startAll() while stats running");
println!(" verbosity > 0 and the harness mechanism may be conflicting");
debug_assert!(false);
panic!("calling Stats.startAll() while stats running");
}
self.shared.set_gathering_stats(true);

Expand Down