Skip to content

Commit

Permalink
Merge branch 'master' into fix/modbuf-global-gc-bug
Browse files Browse the repository at this point in the history
  • Loading branch information
k-sareen authored Aug 23, 2024
2 parents 5732d7c + 160b770 commit 5e37a4b
Show file tree
Hide file tree
Showing 39 changed files with 1,413 additions and 276 deletions.
57 changes: 36 additions & 21 deletions .github/scripts/ci-style.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,51 @@ if [[ $CLIPPY_VERSION == "clippy 0.1.72"* ]]; then
export CARGO_INCREMENTAL=0
fi

if [[ $CLIPPY_VERSION == "clippy 0.1.77"* && $CARGO_BUILD_TARGET == "x86_64-apple-darwin" ]]; then
export SKIP_CLIPPY=1
fi

# --- Check main crate ---

# check base
cargo clippy
# check all features
for_all_features "cargo clippy"
# check release
for_all_features "cargo clippy --release"
# check for tests
for_all_features "cargo clippy --tests"

# target-specific features
if [[ $arch == "x86_64" && $os == "linux" ]]; then
cargo clippy --features perf_counter
cargo clippy --release --features perf_counter
cargo clippy --tests --features perf_counter
fi
if [[ $SKIP_CLIPPY == 1 ]]; then
echo "Skipping clippy version $CLIPPY_VERSION on $CARGO_BUILD_TARGET"
else
# check base
cargo clippy
# check all features
for_all_features "cargo clippy"
# check release
for_all_features "cargo clippy --release"
# check for tests
for_all_features "cargo clippy --tests"

# mock tests
cargo clippy --features mock_test
cargo clippy --features mock_test --tests
cargo clippy --features mock_test --benches
# target-specific features
if [[ $arch == "x86_64" && $os == "linux" ]]; then
cargo clippy --features perf_counter
cargo clippy --release --features perf_counter
cargo clippy --tests --features perf_counter
fi

# mock tests
cargo clippy --features mock_test
cargo clippy --features mock_test --tests
cargo clippy --features mock_test --benches

# non-mock benchmarks
cargo clippy --features test_private --benches
fi

# --- Check auxiliary crate ---

style_check_auxiliary_crate() {
crate_path=$1

cargo clippy --manifest-path=$crate_path/Cargo.toml
cargo fmt --manifest-path=$crate_path/Cargo.toml -- --check
if [[ $SKIP_CLIPPY == 1 ]]; then
echo "Skipping clippy test for $crate_path"
else
cargo clippy --manifest-path=$crate_path/Cargo.toml
cargo fmt --manifest-path=$crate_path/Cargo.toml -- --check
fi
}

style_check_auxiliary_crate macros
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/merge-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ env:
# Ignore some actions for the merge check:
# - This action itself
# - Public API check, doc broken link check: we allow them to fail.
# - Minimal tests for stable Rust: we allow them to fail.
# - Minimal tests and style checks for stable Rust: we allow them to fail.
# - Extended binding tests: it may take long to run. We don't want to wait for them.
# Note: The action name for openjdk tests is different due to the reused workflow.
IGNORED_ACTIONS: |
Expand All @@ -23,6 +23,9 @@ env:
"minimal-tests-core/x86_64-unknown-linux-gnu/stable",
"minimal-tests-core/i686-unknown-linux-gnu/stable",
"minimal-tests-core/x86_64-apple-darwin/stable",
"style-check/x86_64-unknown-linux-gnu/stable",
"style-check/i686-unknown-linux-gnu/stable",
"style-check/x86_64-apple-darwin/stable",
"extended-tests-openjdk / test",
"extended-tests-v8",
"extended-tests-jikesrvm",
Expand Down
43 changes: 43 additions & 0 deletions .github/workflows/minimal-tests-core.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,49 @@ jobs:
- name: Test
run: ./.github/scripts/ci-test.sh

style-check:
needs: setup-test-matrix
strategy:
fail-fast: false
matrix:
target:
- { os: ubuntu-22.04, triple: x86_64-unknown-linux-gnu }
- { os: ubuntu-22.04, triple: i686-unknown-linux-gnu }
- { os: macos-12, triple: x86_64-apple-darwin }
rust: ${{ fromJson(needs.setup-test-matrix.outputs.rust )}}

name: style-check/${{ matrix.target.triple }}/${{ matrix.rust }}
runs-on: ${{ matrix.target.os }}

env:
# This determines the default target which cargo-build, cargo-test, etc. use.
CARGO_BUILD_TARGET: "${{ matrix.target.triple }}"

steps:
- uses: actions/checkout@v4
- name: Install Rust
run: |
# "rustup toolchain install" should always install the host toolchain,
# so we don't specify the triple.
rustup toolchain install ${{ matrix.rust }}
rustup override set ${{ matrix.rust }}
# Ensure we install the target support for the target we are testing for.
# This is especially important for i686-unknown-linux-gnu
# because it's different from the host.
rustup target add ${{ matrix.target.triple }}
rustup component add rustfmt clippy
# Show the Rust toolchain and target we are actually using
- run: rustup show
- run: cargo --version
- run: cargo rustc -- --print cfg

# Setup Environments
- name: Setup Environments
run: ./.github/scripts/ci-setup-${{ matrix.target.triple }}.sh

# Style checks
- name: Style checks
run: ./.github/scripts/ci-style.sh
Expand Down
27 changes: 27 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,30 @@
0.27.0 (2024-08-09)
===

## What's Changed

### Policy
* Clear pin bits by @udesou in https://github.com/mmtk/mmtk-core/pull/1166
* Set the unlog bits for objects in the bootimage by @k-sareen in https://github.com/mmtk/mmtk-core/pull/1168
* Fix bug in line mark when `MARK_LINE_AT_SCAN_TIME=false` by @k-sareen in https://github.com/mmtk/mmtk-core/pull/1171

### API
* Require object reference to be aligned by @qinsoon in https://github.com/mmtk/mmtk-core/pull/1159
* Internal pointer support by @qinsoon in https://github.com/mmtk/mmtk-core/pull/1165
* Heap traversal by @wks in https://github.com/mmtk/mmtk-core/pull/1174

### CI
* Parallelize and workaround CI style checks by @wks in https://github.com/mmtk/mmtk-core/pull/1184

### Misc
* Extensible eBPF timeline attributes by @wks in https://github.com/mmtk/mmtk-core/pull/1162
* Only map with executable permissions when using code space by @k-sareen in https://github.com/mmtk/mmtk-core/pull/1176
* Fix style check for Rust 1.80 by @qinsoon in https://github.com/mmtk/mmtk-core/pull/1178
* Extra assertions for mutators and epilogues by @wks in https://github.com/mmtk/mmtk-core/pull/1182
* Refactor iterate_meta_bits by @wks in https://github.com/mmtk/mmtk-core/pull/1181

**Full Changelog**: https://github.com/mmtk/mmtk-core/compare/v0.26.0...v0.27.0

0.26.0 (2024-07-01)
===

Expand Down
10 changes: 7 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "mmtk"
version = "0.26.0"
version = "0.27.0"
authors = ["The MMTk Developers <>"]
edition = "2021"
license = "MIT OR Apache-2.0"
Expand Down Expand Up @@ -38,7 +38,7 @@ log = { version = "0.4", features = ["max_level_trace", "release_max_level_off"]
memoffset = "0.9"
mimalloc-sys = { version = "0.1.6", optional = true }
# MMTk macros - we have to specify a version here in order to publish the crate, even though we use the dependency from a local path.
mmtk-macros = { version="0.26.0", path = "macros/" }
mmtk-macros = { version="0.27.0", path = "macros/" }
num_cpus = "1.8"
num-traits = "0.2"
pfm = { version = "0.1.1", optional = true }
Expand All @@ -55,6 +55,7 @@ sysinfo = "0.30.9"
[dev-dependencies]
paste = "1.0.8"
rand = "0.8.5"
rand_chacha = "0.3.1"
criterion = "0.4"

[build-dependencies]
Expand All @@ -73,7 +74,10 @@ perf_counter = ["pfm"]

# This feature is only used for tests with MockVM.
# CI scripts run those tests with this feature.
mock_test = []
mock_test = ["test_private"]

# This feature will expose some private functions for testings or benchmarking.
test_private = []

# .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
49 changes: 16 additions & 33 deletions benches/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,25 @@ use criterion::criterion_group;
use criterion::criterion_main;
use criterion::Criterion;

// As we can only initialize one MMTk instance, we have to run each benchmark in a separate process.
// So we only register one benchmark to criterion ('bench_main'), and based on the env var MMTK_BENCH,
// we pick the right benchmark to run.
#[cfg(all(feature = "mock_test", feature = "test_private"))]
pub mod mock_bench;

// The benchmark can be executed with the following command. The feature `mock_test` is required, as the tests use MockVM.
// MMTK_BENCH=alloc cargo bench --features mock_test
// MMTK_BENCH=sft cargo bench --features mock_test

// [Yi] I am not sure if these benchmarks are helpful any more after the MockVM refactoring. MockVM is really slow, as it
// is accessed with a lock, and it dispatches every call to function pointers in a struct. These tests may use MockVM,
// so they become slower as well. And the slowdown
// from MockVM may hide the actual performance difference when we change the functions that are benchmarked.
// We may want to improve the MockVM implementation so we can skip dispatching for benchmarking, or introduce another MockVM
// implementation for benchmarking.
// 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 = "mock_test")]
mod mock_bench;
#[cfg(all(not(feature = "mock_test"), feature = "test_private"))]
pub mod regular_bench;

pub fn bench_main(_c: &mut Criterion) {
#[cfg(feature = "mock_test")]
match std::env::var("MMTK_BENCH") {
Ok(bench) => match bench.as_str() {
"alloc" => mock_bench::alloc::bench(_c),
"internal_pointer" => mock_bench::internal_pointer::bench(_c),
"sft" => mock_bench::sft::bench(_c),
_ => panic!("Unknown benchmark {:?}", bench),
},
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);
cfg_if::cfg_if! {
if #[cfg(feature = "mock_test")] {
// If the "mock_test" feature is enabled, we only run mock test.
mock_bench::bench(_c);
} else if #[cfg(feature = "test_private")] {
regular_bench::bench(_c);
} else {
eprintln!("ERROR: Benchmarks in mmtk_core requires the test_priavte feature (implied by mock_test) to run.");
eprintln!(" Rerun with `MMTK_BENCH=\"bench_name\" cargo bench --features mock_test` to run mock-test benchmarks.");
eprintln!(" Rerun with `cargo bench --features test_private -- bench_name` to run other benchmarks.");
std::process::exit(1);
}
}
}

Expand Down
31 changes: 31 additions & 0 deletions benches/mock_bench/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,34 @@
use criterion::Criterion;

pub mod alloc;
pub mod internal_pointer;
pub mod sft;

// As we can only initialize one MMTk instance, we have to run each benchmark in a separate process.
// So we only register one benchmark to criterion ('bench_main'), and based on the env var MMTK_BENCH,
// we pick the right benchmark to run.

// The benchmark can be executed with the following command. The feature `mock_test` is required, as the tests use MockVM.
// MMTK_BENCH=alloc cargo bench --features mock_test
// MMTK_BENCH=sft cargo bench --features mock_test

// [Yi] I am not sure if these benchmarks are helpful any more after the MockVM refactoring. MockVM is really slow, as it
// is accessed with a lock, and it dispatches every call to function pointers in a struct. These tests may use MockVM,
// so they become slower as well. And the slowdown
// from MockVM may hide the actual performance difference when we change the functions that are benchmarked.
// We may want to improve the MockVM implementation so we can skip dispatching for benchmarking, or introduce another MockVM
// implementation for benchmarking.
// 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.

pub fn bench(c: &mut Criterion) {
match std::env::var("MMTK_BENCH") {
Ok(bench) => match bench.as_str() {
"alloc" => alloc::bench(c),
"internal_pointer" => internal_pointer::bench(c),
"sft" => sft::bench(c),
_ => panic!("Unknown benchmark {:?}", bench),
},
Err(_) => panic!("Need to name a benchmark by the env var MMTK_BENCH"),
}
}
87 changes: 87 additions & 0 deletions benches/regular_bench/bulk_meta/bscan.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
//! Benchmarks for scanning side metadata for non-zero bits.
use criterion::Criterion;
use mmtk::util::{
constants::LOG_BITS_IN_WORD, test_private::scan_non_zero_bits_in_metadata_bytes, Address,
};
use rand::{seq::IteratorRandom, SeedableRng};
use rand_chacha::ChaCha8Rng;

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 BLOCK_BYTES: usize = 32768usize; // Match an Immix block size.

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

/// Set this many distinct bits in the bitmap.
const NUM_OBJECTS: usize = 200;

/// Get a deterministic seeded Rng.
fn get_rng() -> ChaCha8Rng {
// Create an Rng from a seed and an explicit Rng type.
// Not secure at all, but completely deterministic and reproducible.
// The following seed is read from /dev/random
const SEED64: u64 = 0x4050cb1b5ab26c70;
ChaCha8Rng::seed_from_u64(SEED64)
}

/// A bitmap, with known location of each bit for assertion.
struct PreparedBitmap {
start: Address,
end: Address,
set_bits: Vec<(Address, u8)>,
}

/// Make a bitmap of the desired size and set bits.
fn make_standard_bitmap() -> PreparedBitmap {
let start = allocate_aligned(BLOCK_META_BYTES);
let end = start + BLOCK_META_BYTES;
let mut rng = get_rng();

let mut set_bits = (0..(BLOCK_BYTES >> LOG_BITS_IN_WORD))
.choose_multiple(&mut rng, NUM_OBJECTS)
.iter()
.map(|total_bit_offset| {
let word_offset = total_bit_offset >> LOG_BITS_IN_WORD;
let bit_offset = total_bit_offset & ((1 << LOG_BITS_IN_WORD) - 1);
(start + (word_offset << LOG_BITS_IN_WORD), bit_offset as u8)
})
.collect::<Vec<_>>();

set_bits.sort();

for (addr, bit) in set_bits.iter() {
let word = unsafe { addr.load::<usize>() };
let new_word = word | (1 << bit);
unsafe { addr.store::<usize>(new_word) };
}

PreparedBitmap {
start,
end,
set_bits,
}
}

pub fn bench(c: &mut Criterion) {
c.bench_function("bscan_block", |b| {
let bitmap = make_standard_bitmap();
let mut holder: Vec<(Address, u8)> = Vec::with_capacity(NUM_OBJECTS);

b.iter(|| {
holder.clear();
scan_non_zero_bits_in_metadata_bytes(bitmap.start, bitmap.end, &mut |addr, shift| {
holder.push((addr, shift));
});
});

assert_eq!(holder.len(), NUM_OBJECTS);
assert_eq!(holder, bitmap.set_bits);
});
}
Loading

0 comments on commit 5e37a4b

Please sign in to comment.