Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ perf_counter = ["pfm"]
# CI scripts run those tests with this feature.
mock_test = []

# This feature is only used for benchmarks.
# It will expose some private functions for benchmarking.
bench = []
Copy link
Member

Choose a reason for hiding this comment

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

I suggest renaming this to something like test_private or test_private_items. This would be clear for both the function definitions in mmtk-core, and for the bench/test modules outside mmtk-core.

I found the following code confusing: we are in the bench crate, why do we have the bench feature for some tests but not other tests? A better feature name would make it more clear.

#[cfg(feature = "bench")]
mod bulk_meta;

#[cfg(feature = "mock_test")]
mod mock_bench;

Copy link
Member

Choose a reason for hiding this comment

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

It is also posssible to have mock_test = ["test_private_item"]. In that case, you don't have to change bench_main and you can simply add your benchmark.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am OK with renaming it to test_private.

The reason for not requiring the "bench" feature for some test cases is because the current mock tests don't require any of the items exposed from the "bench" feature. But it will probably make things clearer to unconditionally require the "bench" or "test_private" feature when doing benchmarks.


# .github/scripts/ci-common.sh extracts features from the following part (including from comments).
# So be careful when editing or adding stuff to the section below.

Expand Down
62 changes: 62 additions & 0 deletions benches/bulk_meta/bzero_bset.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
//! Benchmarks for bulk zeroing and setting.

use std::os::raw::c_void;

use criterion::Criterion;
use mmtk::util::{constants::LOG_BITS_IN_WORD, metadata::side_metadata::SideMetadataSpec, Address};

fn allocate_aligned(size: usize) -> Address {
let ptr = unsafe {
std::alloc::alloc_zeroed(std::alloc::Layout::from_size_align(size, size).unwrap())
};
Address::from_mut_ptr(ptr)
}

const LINE_BYTES: usize = 256usize; // Match an Immix line size.
const BLOCK_BYTES: usize = 32768usize; // Match an Immix block size.

// Asssume one-bit-per-word metadata (matching VO bits).
const LINE_META_BYTES: usize = LINE_BYTES >> LOG_BITS_IN_WORD;
const BLOCK_META_BYTES: usize = BLOCK_BYTES >> LOG_BITS_IN_WORD;

pub fn bench(c: &mut Criterion) {
c.bench_function("bzero_bset_line", |b| {
let start = allocate_aligned(LINE_META_BYTES);
let end = start + LINE_META_BYTES;

b.iter(|| {
SideMetadataSpec::bench_set_meta_bits(start, 0, end, 0);
SideMetadataSpec::bench_zero_meta_bits(start, 0, end, 0);
})
});

c.bench_function("bzero_bset_line_memset", |b| {
let start = allocate_aligned(LINE_META_BYTES);
let end = start + LINE_META_BYTES;

b.iter(|| unsafe {
libc::memset(start.as_mut_ref() as *mut c_void, 0xff, end - start);
libc::memset(start.as_mut_ref() as *mut c_void, 0x00, end - start);
})
});

c.bench_function("bzero_bset_block", |b| {
let start = allocate_aligned(BLOCK_META_BYTES);
let end = start + BLOCK_META_BYTES;

b.iter(|| {
SideMetadataSpec::bench_set_meta_bits(start, 0, end, 0);
SideMetadataSpec::bench_zero_meta_bits(start, 0, end, 0);
})
});

c.bench_function("bzero_bset_block_memset", |b| {
let start = allocate_aligned(BLOCK_META_BYTES);
let end = start + BLOCK_META_BYTES;

b.iter(|| unsafe {
libc::memset(start.as_mut_ref() as *mut c_void, 0xff, end - start);
libc::memset(start.as_mut_ref() as *mut c_void, 0x00, end - start);
})
});
}
1 change: 1 addition & 0 deletions benches/bulk_meta/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod bzero_bset;
22 changes: 15 additions & 7 deletions benches/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ use criterion::Criterion;
// However, I will just keep these benchmarks here. If we find it not useful, and we do not plan to improve MockVM, we can delete
// them.

#[cfg(feature = "bench")]
mod bulk_meta;

#[cfg(feature = "mock_test")]
mod mock_bench;

pub fn bench_main(_c: &mut Criterion) {
#[cfg(feature = "mock_test")]
#[cfg(feature = "mock_test")]
pub fn bench_mock(_c: &mut Criterion) {
match std::env::var("MMTK_BENCH") {
Ok(bench) => match bench.as_str() {
"alloc" => mock_bench::alloc::bench(_c),
Expand All @@ -33,12 +36,17 @@ pub fn bench_main(_c: &mut Criterion) {
},
Err(_) => panic!("Need to name a benchmark by the env var MMTK_BENCH"),
}
}

#[cfg(not(feature = "mock_test"))]
{
eprintln!("ERROR: Currently there are no benchmarks when the \"mock_test\" feature is not enabled.");
std::process::exit(1);
}
pub fn bench_main(_c: &mut Criterion) {
// If the "mock_test" feature is enabled, we only run mock test.
#[cfg(feature = "mock_test")]
return bench_mock(_c);

// Some benchmarks rely on the "bench" feature to expose some private functions.
// Run them with `cargo bench --features bench`.
#[cfg(feature = "bench")]
bulk_meta::bzero_bset::bench(_c);
Copy link
Member

Choose a reason for hiding this comment

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

See my comments about the features. However, if you do want to separate mock_test and bench, you should create a pattern here so others can create new benchmarks with the bench feature easily.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Criterion benchmarks usually don't use the match statement to select benchmarks. Instead it is similar to running tests. It can be selected on the command line. For example,

cargo bench --features test_private -- bzero

It will select all benchmarks that have "bzero" in the name.

The mock tests are special. Because they rely on a global singleton MockVM, we can only run one test at a time. Previously, we rely on the environment variable MMTK_BENCH to ensure only one mock test benchmark is selected at a time.

At the moment, I just isolated the match statement into the bench_mock function, and leave everything else in bench_main. To install more benchmarks, we simply call c.bench_function, or call functions in other modules (such as bulk_meta::bzero_bset::bench) to install more benchmarks. In the future, the bench_main function will simply call more functions, like these lines: https://github.com/mmtk/mmtk-core/pull/1174/files#diff-9ad1cee13898f4e5ac553eb567cd47a8de7d8852bf292c3d8d5991e23677db32R50-R51

In the future, we can refactor the mock_bench so that we can use the command line to select benchmarks, and as long as we select only one benchmark at a time, they will still be able to run. But if that's impossible, we can still rely on the environment variable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have now completely isolated mock benches from other benchmarks. Now regular benchmarks are in a separate module regular_bench, and each module inside it has its fn bench method for registering benchmarks. When others add more regular benchmarks, they change the fn bench in the right module; when adding a mock benchmark, they edit mock_bench::bench which now holds the match statement.

}

criterion_group!(benches, bench_main);
Expand Down
Loading