From 4589bff65b9eb064b207cf8225356df8ef20b6ab Mon Sep 17 00:00:00 2001 From: Emile van Reenen <9017146+emilevr@users.noreply.github.com> Date: Mon, 6 Nov 2023 23:01:52 +0200 Subject: [PATCH] Various UX and perf improvements. Additional benchmarks (#7) * Remove page_size dependency and use default bucket size. Fix multi-bucket bug * Forbid unsafe code * Implement From trait in Size * Change RapIdArena to be threadsafe * Change RapId to struct * Small refactor of a test * Add iterator for RapIdArena and tests * Add a workflow that can be use to cache BuildIt builds * Remove coverage badge * Add some alt keys and change colors for better UX on Macs * Add additional DirectoryItem::build() benchmark with more threads * Minor update to readme for Mac/Linux. * Add additional benchmarks to test different thread counts * Add a few test cases for alt keys --- .git-hooks/post-checkout | 0 .git-hooks/pre-commit | 0 .github/workflows/benchmark.yaml | 2 +- Cargo.lock | 11 - Cargo.toml | 15 +- README.md | 7 +- benches/{arenas.rs => arenas_alloc.rs} | 13 +- benches/arenas_read.rs | 171 +++++++++++++++ benches/directory_item_build_x2.rs | 34 +++ benches/directory_item_build_x3.rs | 34 +++ buildit/src/benchmark.rs | 25 ++- buildit/src/coverage.rs | 12 +- coverage/html/badges/flat.svg | 23 -- src/cli/tui.rs | 44 +++- src/cli/tui_test.rs | 38 ++-- src/directory_item.rs | 9 + src/main.rs | 4 +- src/rapid_arena.rs | 173 ++++++++++----- src/rapid_arena_test.rs | 292 +++++++++++++------------ src/size.rs | 7 + src/size_test.rs | 13 ++ src/space_bench.rs | 2 +- 22 files changed, 655 insertions(+), 274 deletions(-) mode change 100644 => 100755 .git-hooks/post-checkout mode change 100644 => 100755 .git-hooks/pre-commit rename benches/{arenas.rs => arenas_alloc.rs} (96%) create mode 100644 benches/arenas_read.rs create mode 100644 benches/directory_item_build_x2.rs create mode 100644 benches/directory_item_build_x3.rs delete mode 100644 coverage/html/badges/flat.svg diff --git a/.git-hooks/post-checkout b/.git-hooks/post-checkout old mode 100644 new mode 100755 diff --git a/.git-hooks/pre-commit b/.git-hooks/pre-commit old mode 100644 new mode 100755 diff --git a/.github/workflows/benchmark.yaml b/.github/workflows/benchmark.yaml index c732798..a8e6e09 100644 --- a/.github/workflows/benchmark.yaml +++ b/.github/workflows/benchmark.yaml @@ -31,4 +31,4 @@ jobs: restore-keys: tmp.sample-${{ github.job }} - name: Run all benchmarks - run: buildit benchmark + run: buildit benchmark --bench-names directory_item_build directory_item_build_x2 directory_item_build_x3 diff --git a/Cargo.lock b/Cargo.lock index c722314..1961b3f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -808,16 +808,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "page_size" -version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30d5b2194ed13191c1999ae0704b7839fb18384fa22e49b57eeaa97d79ce40da" -dependencies = [ - "libc", - "winapi", -] - [[package]] name = "parking_lot" version = "0.12.1" @@ -1216,7 +1206,6 @@ dependencies = [ "log", "log4rs", "memory-stats", - "page_size", "ratatui", "rayon", "regex", diff --git a/Cargo.toml b/Cargo.toml index 3faed92..4f1b548 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,13 +28,25 @@ path = "src/space_bench.rs" required-features = ["bench"] [[bench]] -name = "arenas" +name = "arenas_alloc" +harness = false + +[[bench]] +name = "arenas_read" harness = false [[bench]] name = "directory_item_build" harness = false +[[bench]] +name = "directory_item_build_x2" +harness = false + +[[bench]] +name = "directory_item_build_x3" +harness = false + [features] # Uncomment the "default = ..." below to include the nightly features in the build by default, and in the Rust # analyzer. If you do so then you will have to add "+nightly" to cargo commands, e.g. "cargo +nightly build". @@ -52,7 +64,6 @@ crossterm = { version = "^0.27.0", optional = true } dirs = { version = "^5.0.1", optional = true } log = { version = "^0.4.20", optional = true } log4rs = { version = "^1.2.0", optional = true } -page_size = "^0.6.0" ratatui = { version = "^0.23.0", default-features = false, features = ["crossterm"], optional = true } rayon = "^1.7.0" serde = { version = "^1.0.188", features = ["derive"], optional = true } diff --git a/README.md b/README.md index 8b300ad..e73f2ae 100644 --- a/README.md +++ b/README.md @@ -144,7 +144,12 @@ This repository contains: ### Enabling Git hooks (once off) After the initial checkout please run the post-checkout hook manually via: -```git config core.hooksPath ./.git-hooks ; git hook run post-checkout``` +```bash +git config core.hooksPath ./.git-hooks +# On MacOS / Linux, make the hook scripts executable. On Windows, comment out the line below: +chmod +x .git-hooks/* +git hook run post-checkout +``` This will setup the hooks directory, enabling the pre-commit hook to run linting, code coverage and benchmarks. diff --git a/benches/arenas.rs b/benches/arenas_alloc.rs similarity index 96% rename from benches/arenas.rs rename to benches/arenas_alloc.rs index 595c677..d7fd8b3 100644 --- a/benches/arenas.rs +++ b/benches/arenas_alloc.rs @@ -6,7 +6,7 @@ use space_rs::{ rapid_arena::{RapId, RapIdArena}, DirectoryItemType, Size, SizeDisplayFormat, }; -use std::time::Duration; +use std::{ops::DerefMut, time::Duration}; pub struct StdItem { pub path_segment: String, @@ -42,9 +42,9 @@ pub struct RapIdArenaItem { pub children: Vec>, } -fn bench_arenas(c: &mut Criterion) { +fn bench_arenas_alloc(c: &mut Criterion) { let sample_size = 100; - let mut group = c.benchmark_group("arenas"); + let mut group = c.benchmark_group("arenas_alloc"); group.measurement_time(Duration::from_secs(3)); group.warm_up_time(Duration::from_secs(1)); group.sample_size(sample_size); @@ -243,7 +243,7 @@ fn benchmark_rapid_arena( b.iter(|| { black_box({ - let parent = arena.alloc(RapIdArenaItem { + let mut parent = arena.alloc(RapIdArenaItem { parent: None, path_segment: "parent".to_string(), item_type: DirectoryItemType::Directory, @@ -261,7 +261,7 @@ fn benchmark_rapid_arena( children: vec![], }); - arena.get_mut(parent).unwrap().children.push(child); + parent.deref_mut().children.push(child); }) }); @@ -327,7 +327,8 @@ fn report_memory_usage( } else { println!("Couldn't get the current memory usage!"); } + println!(""); } -criterion_group!(benches, bench_arenas); +criterion_group!(benches, bench_arenas_alloc); criterion_main!(benches); diff --git a/benches/arenas_read.rs b/benches/arenas_read.rs new file mode 100644 index 0000000..2990a9e --- /dev/null +++ b/benches/arenas_read.rs @@ -0,0 +1,171 @@ +use criterion::{criterion_group, criterion_main, Criterion}; +use id_arena::{Arena, Id}; +use space_rs::{ + rapid_arena::{RapId, RapIdArena}, + DirectoryItemType, Size, +}; +use std::{ + ops::{Deref, DerefMut}, + time::Duration, +}; + +const ITEM_COUNT: usize = 100000; +const ITEM_READ_COUNT_PER_ITERATION: usize = 5; + +pub struct StdItem { + pub path_segment: String, + pub item_type: DirectoryItemType, + pub size_in_bytes: Size, + pub child_count: usize, + pub children: Vec, +} + +pub struct BumpaloItem<'a> { + pub path_segment: &'a str, + pub item_type: DirectoryItemType, + pub size_in_bytes: &'a Size, + pub child_count: usize, + pub children: Vec>, +} + +pub struct IdArenaItem { + pub parent: Option>, + pub path_segment: String, + pub item_type: DirectoryItemType, + pub size_in_bytes: Size, + pub child_count: usize, + pub children: Vec>, +} + +pub struct RapIdArenaItem { + pub parent: Option>, + pub path_segment: String, + pub item_type: DirectoryItemType, + pub size_in_bytes: Size, + pub child_count: usize, + pub children: Vec>, +} + +fn bench_arenas_read(c: &mut Criterion) { + let mut group = c.benchmark_group("arenas_read"); + group.measurement_time(Duration::from_secs(3)); + group.warm_up_time(Duration::from_secs(1)); + group.sample_size(100); + + benchmark_std(&mut group); + benchmark_id_arena(&mut group); + benchmark_rapid(&mut group); + + group.finish(); +} + +fn benchmark_std(group: &mut criterion::BenchmarkGroup<'_, criterion::measurement::WallTime>) { + let mut items = Vec::::with_capacity(ITEM_COUNT); + // Alloc + for _ in 0..ITEM_COUNT { + let mut parent = StdItem { + path_segment: "parent".into(), + item_type: DirectoryItemType::Directory, + size_in_bytes: Size::new(1234), + child_count: 1, + children: vec![], + }; + + parent.children.push(StdItem { + path_segment: "child".into(), + item_type: DirectoryItemType::File, + size_in_bytes: Size::new(1234), + child_count: 0, + children: vec![], + }); + + items.push(parent); + } + + group.bench_function("std", |b| { + b.iter(|| { + for i in 0..ITEM_COUNT * ITEM_READ_COUNT_PER_ITERATION { + let _ = &items[i % ITEM_COUNT]; + } + }); + }); +} + +fn benchmark_id_arena(group: &mut criterion::BenchmarkGroup<'_, criterion::measurement::WallTime>) { + let mut arena = Arena::::new(); + let mut ids = vec![]; + + // Alloc + for _ in 0..ITEM_COUNT { + let parent = arena.alloc(IdArenaItem { + parent: None, + path_segment: "parent".to_string(), + item_type: DirectoryItemType::Directory, + size_in_bytes: Size::new(1234), + child_count: 1, + children: vec![], + }); + + let child = arena.alloc(IdArenaItem { + parent: Some(parent), + path_segment: "child".to_string(), + item_type: DirectoryItemType::File, + size_in_bytes: Size::new(1234), + child_count: 0, + children: vec![], + }); + + arena.get_mut(parent).unwrap().children.push(child); + + ids.push(parent); + } + + group.bench_function("id-arena", |b| { + b.iter(|| { + for i in 0..ITEM_COUNT * ITEM_READ_COUNT_PER_ITERATION { + let _ = &arena.get(ids[i % ITEM_COUNT]); + } + }); + }); +} + +fn benchmark_rapid(group: &mut criterion::BenchmarkGroup<'_, criterion::measurement::WallTime>) { + let mut arena = RapIdArena::::new(); + let mut ids = vec![]; + + // Alloc + for _ in 0..ITEM_COUNT { + let mut parent = arena.alloc(RapIdArenaItem { + parent: None, + path_segment: "parent".to_string(), + item_type: DirectoryItemType::Directory, + size_in_bytes: Size::new(1234), + child_count: 1, + children: vec![], + }); + + let child = arena.alloc(RapIdArenaItem { + parent: Some(parent), + path_segment: "child".to_string(), + item_type: DirectoryItemType::File, + size_in_bytes: Size::new(1234), + child_count: 0, + children: vec![], + }); + + parent.deref_mut().children.push(child); + + ids.push(parent); + } + + group.bench_function("rapid-arena", |b| { + b.iter(|| { + for i in 0..ITEM_COUNT * ITEM_READ_COUNT_PER_ITERATION { + let _ = ids[i % ITEM_COUNT].deref(); + } + }); + }); +} + +criterion_group!(benches, bench_arenas_read); +criterion_main!(benches); diff --git a/benches/directory_item_build_x2.rs b/benches/directory_item_build_x2.rs new file mode 100644 index 0000000..c2c25ae --- /dev/null +++ b/benches/directory_item_build_x2.rs @@ -0,0 +1,34 @@ +use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use space_rs::DirectoryItem; +use std::path::Path; + +const DIRECTORY_PATH: &str = "./tmp.sample"; + +pub fn directory_item_build_x2(c: &mut Criterion) { + let path = &Path::new(DIRECTORY_PATH).to_path_buf(); + + use std::thread::available_parallelism; + let thread_count = available_parallelism().unwrap().get() * 2; + println!("Using {} Rayon threads", thread_count); + + rayon::ThreadPoolBuilder::new() + .num_threads(thread_count) + .build_global() + .unwrap(); + + c.bench_function( + &format!("DirectoryItem::build() x2 on {}", path.display()), + |b| { + b.iter(|| { + black_box(DirectoryItem::build(vec![path.clone()])); + }) + }, + ); +} + +criterion_group! { + name = benches; + config = Criterion::default().sample_size(10); + targets = directory_item_build_x2 +} +criterion_main!(benches); diff --git a/benches/directory_item_build_x3.rs b/benches/directory_item_build_x3.rs new file mode 100644 index 0000000..e15b36a --- /dev/null +++ b/benches/directory_item_build_x3.rs @@ -0,0 +1,34 @@ +use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use space_rs::DirectoryItem; +use std::path::Path; + +const DIRECTORY_PATH: &str = "./tmp.sample"; + +pub fn directory_item_build_x3(c: &mut Criterion) { + let path = &Path::new(DIRECTORY_PATH).to_path_buf(); + + use std::thread::available_parallelism; + let thread_count = available_parallelism().unwrap().get() * 3; + println!("Using {} Rayon threads", thread_count); + + rayon::ThreadPoolBuilder::new() + .num_threads(thread_count) + .build_global() + .unwrap(); + + c.bench_function( + &format!("DirectoryItem::build() x3 on {}", path.display()), + |b| { + b.iter(|| { + black_box(DirectoryItem::build(vec![path.clone()])); + }) + }, + ); +} + +criterion_group! { + name = benches; + config = Criterion::default().sample_size(10); + targets = directory_item_build_x3 +} +criterion_main!(benches); diff --git a/buildit/src/benchmark.rs b/buildit/src/benchmark.rs index 6b286f9..9b1b989 100644 --- a/buildit/src/benchmark.rs +++ b/buildit/src/benchmark.rs @@ -12,14 +12,22 @@ use zip::ZipArchive; use crate::command::BuildItCommand; #[derive(Args, Debug)] -pub struct BenchmarkCommandArgs {} +pub struct BenchmarkCommandArgs { + /// One or more optional benchmarks to run. If not specified then all benchmarks will be run. + #[arg(long = "bench-names", value_parser, num_args = 1.., value_delimiter = ' ')] + pub bench_names: Option>, +} #[derive(Debug)] -pub struct BenchmarkCommand {} +pub struct BenchmarkCommand { + bench_names: Option>, +} impl BenchmarkCommand { - pub fn new(_: BenchmarkCommandArgs) -> Self { - BenchmarkCommand {} + pub fn new(args: BenchmarkCommandArgs) -> Self { + BenchmarkCommand { + bench_names: args.bench_names, + } } } @@ -57,7 +65,14 @@ impl BuildItCommand for BenchmarkCommand { } println!("👷 Running benchmarks ..."); - cmd!("cargo", "bench").run()?; + let mut args = vec!["bench"]; + if let Some(bench_names) = &self.bench_names { + bench_names.iter().for_each(|n| { + args.push("--bench"); + args.push(n); + }); + } + cmd("cargo", args).run()?; println!("✔️ Benchmarks completed successfully."); Ok(()) diff --git a/buildit/src/coverage.rs b/buildit/src/coverage.rs index 9333076..94330a1 100644 --- a/buildit/src/coverage.rs +++ b/buildit/src/coverage.rs @@ -132,19 +132,22 @@ impl BuildItCommand for CoverageCommand { // ^\\s*#!\\[.*$ => lines containing only a crate attribute // ^\\s*\\}\\s*else\\s*\\{\\s*$ => lines containing only "} else {" // ^\\s*//.*$ => lines containing only a comment + // ^\\s*(pub|pub\\s*\\(\\s*crate\\s*\\)\\s*)?\\s*struct\\s+[^ ]+\\s*\\{\\s*$ => lines containing only a struct definition // ^\\s*(pub|pub\\s*\\(\\s*crate\\s*\\)\\s*)?\\s*fn\\s+.*\\s*[(){}]*\\s*$ => lines containing only a fn definition // ^\\s*(pub|pub\\s*\\(\\s*crate\\s*\\)\\s*)?\\s*[^ ]+\\s*:\\s*[^ ]+\\s*,\\s*$ => lines containing only a property declaration // ^\\s*loop\\s*\\{\\s*$ => lines containing only 'loop {' // ^\\s*[^ ]+\\s*=>\\s*(\\{)?\\s*$ => lines only containing the expression part of a match clause // ^\\s*\\)\\s*->\\s*[^ ]+\\s*\\{\\s*$ => lines only ') -> some_return_type {' // ^\\s*[{}(),;\\[\\] ]*\\s*$ => lines with only delimiters or whitespace, no logic - // ^\\s*impl\\s+[^ ]+\\s*\\{\\s*$ => lines containing only an impl declaration - // ^\\s*impl\\s+[^ ]+\\s+for\\s+[^ ]*\\s*\\{\\s*$ => lines containing only an impl for declaration + // ^\\s*impl(<.*>)?\\s*[^ ]+\\s*\\{\\s*$ => lines containing only an impl declaration + // ^\\s*impl(<.*>)?\\s*[^ ]+\\s+for\\s+[^ ]*\\s*\\{\\s*$ => lines containing only an impl for declaration + // ^\\s*(pub|pub\\s*\\(\\s*crate\\s*\\)\\s*)?\\s*const\\s+.*\\s*[(){}]*\\s*$ => lines containing only a const definition "^\\s*(debug_)?assert(_eq|_ne)?!\ |^\\s*#\\[.*$\ |^\\s*#!\\[.*$\ |^\\s*\\}\\s*else\\s*\\{\\s*$\ |^\\s*//.*$\ + |^\\s*(pub|pub\\s*\\(\\s*crate\\s*\\)\\s*)?\\s*struct\\s+[^ ]+\\s*\\{\\s*$\ |^\\s*(pub|pub\\s*\\(\\s*crate\\s*\\)\\s*)?\\s*fn\\s+.*\\s*[(){}]*\\s*$\ |^\\s*(pub|pub\\s*\\(\\s*crate\\s*\\)\\s*)?\\s*[^ ]+\\s*:\\s*[^ ]+\\s*,\\s*$\ |^\\s*loop\\s*\\{\\s*$\ @@ -152,8 +155,9 @@ impl BuildItCommand for CoverageCommand { |^\\s*\\)\\s*->\\s*[^ ]+\\s*\\{\\s*$\ |^\\s[});({]*\\s*$\ |^\\s*[{}(),;\\[\\] ]*\\s*$\ - |^\\s*impl\\s+[^ ]+\\s*\\{\\s*$\ - |^\\s*impl\\s+[^ ]+\\s+for\\s+[^ ]*\\s*\\{\\s*$", + |^\\s*impl(<.*>)?\\s*[^ ]+\\s*\\{\\s*$\ + |^\\s*impl(<.*>)?\\s*[^ ]+\\s+for\\s+[^ ]*\\s*\\{\\s*$\ + |^\\s*(pub|pub\\s*\\(\\s*crate\\s*\\)\\s*)?\\s*const\\s+.*\\s*[(){}]*\\s*$", "--ignore-not-existing", "--ignore", "buildit/*", diff --git a/coverage/html/badges/flat.svg b/coverage/html/badges/flat.svg deleted file mode 100644 index b526ffe..0000000 --- a/coverage/html/badges/flat.svg +++ /dev/null @@ -1,23 +0,0 @@ - - coverage: 98% - - - - - - - - - - - - - - - coverage - - 98% - - - \ No newline at end of file diff --git a/src/cli/tui.rs b/src/cli/tui.rs index 904440a..840cf92 100644 --- a/src/cli/tui.rs +++ b/src/cli/tui.rs @@ -34,17 +34,41 @@ mod tui_test; const VERSION: &str = env!("CARGO_PKG_VERSION"); +#[cfg(target_os = "macos")] +const KEY_HELP_KEY_FG_COLOR: Color = Color::Rgb(0, 0, 64); +#[cfg(not(target_os = "macos"))] const KEY_HELP_KEY_FG_COLOR: Color = Color::Rgb(88, 144, 255); + +#[cfg(target_os = "macos")] +const TABLE_HEADER_BG_COLOR: Color = Color::Rgb(64, 64, 64); +#[cfg(not(target_os = "macos"))] const TABLE_HEADER_BG_COLOR: Color = Color::Rgb(64, 64, 192); + +#[cfg(target_os = "macos")] +const TABLE_HEADER_FG_COLOR: Color = Color::Rgb(0, 0, 0); +#[cfg(not(target_os = "macos"))] +const TABLE_HEADER_FG_COLOR: Color = Color::White; + const DANGER_BG_COLOR: Color = Color::Rgb(192, 64, 64); + +#[cfg(target_os = "macos")] +const TITLE_FG_COLOR: Color = Color::Black; +#[cfg(not(target_os = "macos"))] const TITLE_FG_COLOR: Color = Color::White; + +#[cfg(target_os = "macos")] +const TITLE_BG_COLOR: Color = Color::Rgb(128, 128, 128); +#[cfg(not(target_os = "macos"))] const TITLE_BG_COLOR: Color = Color::Rgb(64, 64, 64); + const VALUE_FG_COLOR: Color = Color::Rgb(88, 144, 255); pub(crate) const HELP_KEY: char = '?'; pub(crate) const QUIT_KEY_1: char = 'q'; pub(crate) const COLLAPSE_SELECTED_CHILDREN_KEY: char = '-'; +pub(crate) const COLLAPSE_SELECTED_CHILDREN_KEY_ALT: char = '_'; pub(crate) const EXPAND_SELECTED_CHILDREN_KEY: char = '+'; +pub(crate) const EXPAND_SELECTED_CHILDREN_KEY_ALT: char = '='; pub(crate) const VIEW_SIZE_THRESHOLD_0_PERCENT_KEY: char = '0'; pub(crate) const VIEW_SIZE_THRESHOLD_10_PERCENT_KEY: char = '1'; pub(crate) const VIEW_SIZE_THRESHOLD_20_PERCENT_KEY: char = '2'; @@ -189,12 +213,10 @@ fn render_loop( KeyCode::PageDown => view_state.next(view_state.visible_height), KeyCode::Home => view_state.first(), KeyCode::End => view_state.last(), - KeyCode::Char(COLLAPSE_SELECTED_CHILDREN_KEY) => { - view_state.collapse_selected_children() - } - KeyCode::Char(EXPAND_SELECTED_CHILDREN_KEY) => { - view_state.expand_selected_children() - } + #[rustfmt::skip] + KeyCode::Char(COLLAPSE_SELECTED_CHILDREN_KEY) | KeyCode::Char(COLLAPSE_SELECTED_CHILDREN_KEY_ALT) => { view_state.collapse_selected_children() }, + #[rustfmt::skip] + KeyCode::Char(EXPAND_SELECTED_CHILDREN_KEY) | KeyCode::Char(EXPAND_SELECTED_CHILDREN_KEY_ALT) => { view_state.expand_selected_children() }, _ => {} } } @@ -241,7 +263,7 @@ fn render_title_bar( area: &Rect, title_style: Style, ) { - let version_style = title_style.gray().dim(); + let version_style = title_style; let title = "Space"; let version_display = format!("v{VERSION}"); @@ -290,7 +312,7 @@ fn render_title_bar( fn get_key_help<'a>(title_style: Style, available_width: i32) -> (Line<'a>, i32) { let mut available_width = available_width; let key_style = title_style.fg(KEY_HELP_KEY_FG_COLOR); - let key_help_style = title_style.gray().dim(); + let key_help_style = title_style; #[rustfmt::skip] let all_key_help = vec![ @@ -319,14 +341,16 @@ fn get_key_help<'a>(title_style: Style, available_width: i32) -> (Line<'a>, i32) } fn render_table(f: &mut Frame, view_state: &mut ViewState, area: &Rect) { - let table_header_style = Style::default().bg(TABLE_HEADER_BG_COLOR); + let table_header_style = Style::default() + .bg(TABLE_HEADER_BG_COLOR) + .fg(TABLE_HEADER_FG_COLOR); let selected_style = Style::default().add_modifier(Modifier::REVERSED); let table_selected_index = view_state.table_selected_index; let header_cells = ["Size", "", "Path", "", "Incl"] .iter() - .map(|h| Cell::from(*h).style(Style::default().fg(Color::White))); + .map(|h| Cell::from(*h)); let header = Row::new(header_cells) .style(table_header_style) .height(1) diff --git a/src/cli/tui_test.rs b/src/cli/tui_test.rs index b04dd20..a9edc17 100644 --- a/src/cli/tui_test.rs +++ b/src/cli/tui_test.rs @@ -617,9 +617,13 @@ fn render_given_expand_input_for_collapsed_directory_item_expands_it() -> anyhow Ok(()) } -#[test] +#[rstest] +#[ignore] +#[case(COLLAPSE_SELECTED_CHILDREN_KEY)] #[ignore] +#[case(COLLAPSE_SELECTED_CHILDREN_KEY_ALT)] fn render_given_collapse_children_input_for_directory_item_collapses_all_children( + #[case] key: char, ) -> anyhow::Result<()> { // Arrange // NOTE: The real terminal height will be used when this test runs, so make sure the test is independent @@ -627,10 +631,7 @@ fn render_given_collapse_children_input_for_directory_item_collapses_all_childre let (mut view_state, temp_dir_path) = make_test_view_state(0f32)?; let mut output = TestOut::new(); let mut input_event_source = TestInputEventSource::new(vec![ - Event::Key(KeyEvent::new( - KeyCode::Char(COLLAPSE_SELECTED_CHILDREN_KEY), - KeyModifiers::NONE, - )), // Collapses children + Event::Key(KeyEvent::new(KeyCode::Char(key), KeyModifiers::NONE)), // Collapses children Event::Key(KeyEvent::new(KeyCode::Char(QUIT_KEY_1), KeyModifiers::NONE)), ]); @@ -648,9 +649,13 @@ fn render_given_collapse_children_input_for_directory_item_collapses_all_childre Ok(()) } -#[test] +#[rstest] #[ignore] +#[case(EXPAND_SELECTED_CHILDREN_KEY)] +#[ignore] +#[case(EXPAND_SELECTED_CHILDREN_KEY_ALT)] fn render_given_expand_children_input_for_collapsed_directory_item_expands_all_children( + #[case] key: char, ) -> anyhow::Result<()> { // Arrange // NOTE: The real terminal height will be used when this test runs, so make sure the test is independent @@ -660,10 +665,7 @@ fn render_given_expand_children_input_for_collapsed_directory_item_expands_all_c let mut input_event_source = TestInputEventSource::new(vec![ Event::Key(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)), // Select 1 Event::Key(KeyEvent::new(KeyCode::Left, KeyModifiers::NONE)), // Collapses 1 - Event::Key(KeyEvent::new( - KeyCode::Char(EXPAND_SELECTED_CHILDREN_KEY), - KeyModifiers::NONE, - )), // Expands children + Event::Key(KeyEvent::new(KeyCode::Char(key), KeyModifiers::NONE)), // Expands children Event::Key(KeyEvent::new(KeyCode::Char(QUIT_KEY_1), KeyModifiers::NONE)), ]); @@ -678,9 +680,13 @@ fn render_given_expand_children_input_for_collapsed_directory_item_expands_all_c Ok(()) } -#[test] +#[rstest] +#[ignore] +#[case(EXPAND_SELECTED_CHILDREN_KEY)] #[ignore] +#[case(EXPAND_SELECTED_CHILDREN_KEY_ALT)] fn render_given_expand_children_input_for_expanded_directory_item_expands_all_children( + #[case] key: char, ) -> anyhow::Result<()> { // Arrange // NOTE: The real terminal height will be used when this test runs, so make sure the test is independent @@ -690,14 +696,8 @@ fn render_given_expand_children_input_for_expanded_directory_item_expands_all_ch let mut input_event_source = TestInputEventSource::new(vec![ Event::Key(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)), // Select 1 Event::Key(KeyEvent::new(KeyCode::Left, KeyModifiers::NONE)), // Collapses 1 - Event::Key(KeyEvent::new( - KeyCode::Char(EXPAND_SELECTED_CHILDREN_KEY), - KeyModifiers::NONE, - )), // Expands direct children - Event::Key(KeyEvent::new( - KeyCode::Char(EXPAND_SELECTED_CHILDREN_KEY), - KeyModifiers::NONE, - )), // Expands all children + Event::Key(KeyEvent::new(KeyCode::Char(key), KeyModifiers::NONE)), // Expands direct children + Event::Key(KeyEvent::new(KeyCode::Char(key), KeyModifiers::NONE)), // Expands all children Event::Key(KeyEvent::new(KeyCode::Char(QUIT_KEY_1), KeyModifiers::NONE)), ]); diff --git a/src/directory_item.rs b/src/directory_item.rs index e0f8cf9..5bdbb89 100644 --- a/src/directory_item.rs +++ b/src/directory_item.rs @@ -62,6 +62,7 @@ impl DirectoryItem { items } + #[inline(always)] fn from_root(path: &PathBuf) -> DirectoryItem { let mut item = if let Ok(metadata) = fs::symlink_metadata(path) { if metadata.is_file() { @@ -100,6 +101,7 @@ impl DirectoryItem { } } + #[inline(always)] fn from_failure(path: &Path) -> DirectoryItem { DirectoryItem { path_segment: get_file_name_from_path(path), @@ -110,6 +112,7 @@ impl DirectoryItem { } } + #[inline(always)] fn from_directory(path: &Path) -> DirectoryItem { let children = { let mut children = Self::get_child_items(path); @@ -130,6 +133,7 @@ impl DirectoryItem { } } + #[inline(always)] fn get_child_items(path: &Path) -> Vec { fs::read_dir(path) .into_iter() @@ -154,6 +158,7 @@ impl DirectoryItem { } /// Given the total size in bytes, returns the fraction of that total that his item uses. + #[inline(always)] pub fn get_fraction(&self, total_size_in_bytes: u64) -> f32 { if total_size_in_bytes == 0 { 0f32 @@ -164,23 +169,27 @@ impl DirectoryItem { } impl Ord for DirectoryItem { + #[inline(always)] fn cmp(&self, other: &Self) -> Ordering { self.size_in_bytes.cmp(&other.size_in_bytes) } } impl PartialOrd for DirectoryItem { + #[inline(always)] fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } impl PartialEq for DirectoryItem { + #[inline(always)] fn eq(&self, other: &Self) -> bool { self.size_in_bytes == other.size_in_bytes } } +#[inline(always)] fn get_file_name_from_path(path: &Path) -> String { match path.file_name() { Some(file_name) => file_name.to_string_lossy().to_string(), diff --git a/src/main.rs b/src/main.rs index 58f38c8..2c856da 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,3 +1,5 @@ +#![forbid(unsafe_code)] + use clap::{ColorChoice, Parser}; use cli::cli_command::CliCommand; use cli::view_command::ViewCommand; @@ -22,7 +24,7 @@ mod test_utils; mod logging; -pub(crate) const DEFAULT_SIZE_THRESHOLD_PERCENTAGE: u8 = 1; +const DEFAULT_SIZE_THRESHOLD_PERCENTAGE: u8 = 1; #[derive(Clone, Debug, Parser)] #[clap( diff --git a/src/rapid_arena.rs b/src/rapid_arena.rs index ae37e16..f45d3f4 100644 --- a/src/rapid_arena.rs +++ b/src/rapid_arena.rs @@ -1,44 +1,55 @@ //! Implements a fast arena allocator that uses fixed size buckets and returns IDs for allocated objects. -use page_size; -use std::{cmp::max, marker::PhantomData, mem::size_of, ops::Index}; +use std::{ + mem::size_of, + ops::{Deref, DerefMut}, + ptr, + sync::RwLock, +}; #[cfg(test)] #[path = "./rapid_arena_test.rs"] mod rapid_arena_test; +const DEFAULT_BUCKET_SIZE_IN_BYTES: usize = 64 * 1024; + /// An arena that can be used to allocate objects efficiently. #[derive(Debug)] pub struct RapIdArena { - buckets: Vec>, items_per_bucket: usize, - bucket_index: usize, + internals: RwLock>, } -/// An ID that identifies an allocated object. #[derive(Debug)] -pub struct RapId { - index: usize, - _t: PhantomData, +struct ArenaInternals { + buckets: Vec>, + bucket_index: usize, } impl RapIdArena { /// Creates a new arena for the specified type. pub fn new() -> Self { - let items_per_bucket = max(page_size::get(), page_size::get_granularity()) / size_of::(); + let items_per_bucket = DEFAULT_BUCKET_SIZE_IN_BYTES / size_of::(); RapIdArena:: { - buckets: vec![Vec::::with_capacity(items_per_bucket)], items_per_bucket, - bucket_index: 0, + internals: RwLock::new(ArenaInternals { + buckets: vec![Vec::::with_capacity(items_per_bucket)], + bucket_index: 0, + }), } } /// Creates a new arena with each bucket able to hold the specified number of items. pub fn new_with_bucket_size(items_per_bucket: usize) -> Self { + if items_per_bucket == 0 { + panic!("The specified number of items per bucket is invalid! The value must be greater than 0.") + } RapIdArena:: { - buckets: vec![Vec::::with_capacity(items_per_bucket)], items_per_bucket, - bucket_index: 0, + internals: RwLock::new(ArenaInternals { + buckets: vec![Vec::::with_capacity(items_per_bucket)], + bucket_index: 0, + }), } } @@ -50,50 +61,52 @@ impl RapIdArena { /// Allocates the specified item inside the arena. #[inline] pub fn alloc(&mut self, item: T) -> RapId { - let mut bucket = &mut self.buckets[self.bucket_index]; + let mut internals = self.internals.write().unwrap(); + let mut bucket_index = internals.bucket_index; + let mut bucket = &mut internals.buckets[bucket_index]; + if bucket.len() == self.items_per_bucket { - self.buckets + internals + .buckets .push(Vec::::with_capacity(self.items_per_bucket)); - self.bucket_index += 1; - bucket = &mut self.buckets[self.bucket_index]; + bucket_index += 1; + internals.bucket_index = bucket_index; + bucket = &mut internals.buckets[bucket_index]; } - let index = bucket.len(); + let item_index = bucket.len(); bucket.push(item); - RapId:: { - index, - _t: PhantomData, + RapId { + p: ptr::NonNull::from(&bucket[item_index]), } } - /// Returns a reference to the item identified by the specified ID. - #[inline] - pub fn get(&self, id: RapId) -> Option<&T> { - self.buckets[self.bucket_index].get(id.index()) - } - - /// Returns a mutable reference to the item identified by the specified ID. - #[inline] - pub fn get_mut(&mut self, id: RapId) -> Option<&mut T> { - self.buckets[self.bucket_index].get_mut(id.index()) - } - /// Returns the number of allocated items in the arena. pub fn len(&self) -> usize { - self.bucket_index * self.items_per_bucket + self.buckets[self.bucket_index].len() + let internals = self.internals.read().unwrap(); + let bucket_index = internals.bucket_index; + bucket_index * self.items_per_bucket + internals.buckets[bucket_index].len() } /// Returns true is the arena is empty. pub fn is_empty(&self) -> bool { - self.bucket_index == 0 && self.buckets[self.bucket_index].len() == 0 + self.len() == 0 } - /// Resets the arena to the default state, with a single empty bucket. - pub fn reset(&mut self) { - self.bucket_index = 0; - self.buckets.truncate(1); + /// Returs an iterator for the arena contents. This iterator is threadsafe. + pub fn iter(&self) -> RapIdArenaIterator { + let mut data = vec![]; + let arena_internals = self.internals.read().unwrap(); + for bucket in &arena_internals.buckets { + for item in bucket { + data.push(RapId { + p: ptr::NonNull::from(item), + }) + } + } + RapIdArenaIterator:: { data, index: 0 } } } @@ -103,27 +116,85 @@ impl Default for RapIdArena { } } -impl Index> for RapIdArena { - type Output = T; +// Safety: items_per_bucket is immutable and all the internal values are protected via a RwLock. +unsafe impl Send for RapIdArena {} - #[inline] - fn index(&self, index: RapId) -> &Self::Output { - &self.buckets[self.bucket_index][index.index()] - } +// Safety: items_per_bucket is immutable and all the internal values are protected via a RwLock. +unsafe impl Sync for RapIdArena {} + +/// An iterator for a RapIdArena instance. +#[derive(Debug)] +pub struct RapIdArenaIterator { + data: Vec>, + index: usize, } -impl RapId { - #[inline] - fn index(&self) -> usize { - self.index +impl Iterator for RapIdArenaIterator { + type Item = RapId; + + fn next(&mut self) -> Option { + if self.index < self.data.len() { + let value = self.data[self.index]; + self.index += 1; + Some(value) + } else { + None + } } } +/// An ID that contains an allocated object. +#[derive(Debug)] +pub struct RapId { + p: ptr::NonNull, +} + impl Copy for RapId {} impl Clone for RapId { #[inline] - fn clone(&self) -> RapId { - *self + fn clone(&self) -> Self { + Self { p: self.p } } } + +impl Deref for RapId { + type Target = T; + + fn deref(&self) -> &Self::Target { + unsafe { + // Safety: The pointer is aligned, initialized, and dereferenceable by the guarantees made by Vec. + // We require readers to borrow the RapId, and the lifetime of the return value is elided to the + // lifetime of the input. This means the borrow checker will enforce that no one can mutate the + // contents of the RapId until the reference returned is dropped. + self.p.as_ref() + } + } +} + +impl DerefMut for RapId { + /// NOTE! If the mutable reference is used concurrently from multiple threads, then T has to be threadsafe + /// or race conditions may occur. Wrap T in Mutex or RwLock rather than storing T instances directly + /// in the arena. + /// However, it is safe to modify instances in a single parallel iterator as each item is accessed + /// only by a single thread at a time. + fn deref_mut(&mut self) -> &mut Self::Target { + unsafe { + // Safety: The pointer is aligned, initialized, and dereferenceable by the guarantees made by Vec. + // We require readers to borrow the RapId, and the lifetime of the return value is elided to the + // lifetime of the input. This means the borrow checker will enforce that no one can mutate the + // contents of the RapId until the reference returned is dropped. + self.p.as_mut() + } + } +} + +// Safety: No one besides us has the raw pointer, so we can safely transfer the RapId to another thread if T +// can be safely transferred. +unsafe impl Send for RapId {} + +// Safety: Since there exists a public way to go from a `&RapId` to a `&T` in an unsynchronized fashion +// (such as `Deref`), then `RapId` can't be `Sync` if `T` isn't. Conversely, `RapId` itself does not use +// any interior mutability whatsoever: all the mutations are performed through an exclusive reference +// (`&mut`). This means it suffices that `T` be `Sync` for `RapId` to be `Sync`. +unsafe impl Sync for RapId {} diff --git a/src/rapid_arena_test.rs b/src/rapid_arena_test.rs index 54cfde2..3cf28f4 100644 --- a/src/rapid_arena_test.rs +++ b/src/rapid_arena_test.rs @@ -1,40 +1,53 @@ -use rstest::rstest; - use super::*; +use criterion::black_box; +use rayon::prelude::{ParallelBridge, ParallelIterator}; +use rstest::rstest; +use std::{thread, time::Duration}; #[derive(Debug, PartialEq)] struct Something { - value1: usize, - value2: String, + some_value: usize, + some_string: String, +} + +fn alloc_items(arena: &mut RapIdArena, count: usize) -> Vec> { + let mut ids = vec![]; + for i in 0..count { + ids.push(arena.alloc(Something { + some_value: i, + some_string: format!("i = {}", i), + })); + } + ids } #[test] fn new_creates_single_bucket_with_expected_bucket_size() { // Arrange - let items_per_bucket = - max(page_size::get(), page_size::get_granularity()) / size_of::(); + let items_per_bucket = DEFAULT_BUCKET_SIZE_IN_BYTES / size_of::(); // Act let arena = RapIdArena::::new(); // Assert - assert_eq!(1, arena.buckets.len()); - assert_eq!(0, arena.bucket_index); + let internals = arena.internals.read().unwrap(); + assert_eq!(1, internals.buckets.len()); + assert_eq!(0, internals.bucket_index); assert_eq!(items_per_bucket, arena.items_per_bucket()); } #[test] fn default_creates_single_bucket_with_expected_bucket_size() { // Arrange - let items_per_bucket = - max(page_size::get(), page_size::get_granularity()) / size_of::(); + let items_per_bucket = DEFAULT_BUCKET_SIZE_IN_BYTES / size_of::(); // Act let arena = RapIdArena::::default(); // Assert - assert_eq!(1, arena.buckets.len()); - assert_eq!(0, arena.bucket_index); + let internals = arena.internals.read().unwrap(); + assert_eq!(1, internals.buckets.len()); + assert_eq!(0, internals.bucket_index); assert_eq!(items_per_bucket, arena.items_per_bucket()); } @@ -47,72 +60,69 @@ fn new_with_bucket_size_creates_single_bucket_with_expected_bucket_size() { let arena = RapIdArena::::new_with_bucket_size(items_per_bucket); // Assert - assert_eq!(1, arena.buckets.len()); - assert_eq!(0, arena.bucket_index); + let internals = arena.internals.read().unwrap(); + assert_eq!(1, internals.buckets.len()); + assert_eq!(0, internals.bucket_index); assert_eq!(items_per_bucket, arena.items_per_bucket()); } #[test] -fn alloc_then_get_returns_expected_item() { +fn alloc_then_deref_returns_expected_item() { // Arrange let mut arena = RapIdArena::::new(); - let value1 = 123; - let value2 = "abc".to_string(); + let v1 = 123; + let s1 = "abc".to_string(); // Act let id = arena.alloc(Something { - value1, - value2: value2.clone(), + some_value: v1, + some_string: s1.clone(), }); // Assert - let actual = arena.get(id).expect("The ID is expected to be valid!"); - assert_eq!(value1, actual.value1); - assert_eq!(value2, actual.value2); + let actual = id.deref(); + assert_eq!(v1, actual.some_value); + assert_eq!(s1, actual.some_string); } #[test] fn cloned_id_returns_expected_item() { // Arrange let mut arena = RapIdArena::::new(); - let value1 = 1024; - let value2 = "some string".to_string(); + let v1 = 1024; + let s1 = "some string".to_string(); let id = arena.alloc(Something { - value1, - value2: value2.clone(), + some_value: v1, + some_string: s1.clone(), }); // Act let cloned_id = id.clone(); // Assert - let entry = arena - .get(cloned_id) - .expect("The ID is expected to be valid!"); - assert_eq!(value1, entry.value1); - assert_eq!(value2, entry.value2); + let entry = cloned_id.deref(); + assert_eq!(v1, entry.some_value); + assert_eq!(s1, entry.some_string); } #[test] fn copied_id_returns_expected_item() { // Arrange let mut arena = RapIdArena::::new(); - let value1 = 1024; - let value2 = "some string".to_string(); + let v1 = 1024; + let s1 = "some string".to_string(); let id = arena.alloc(Something { - value1, - value2: value2.clone(), + some_value: v1, + some_string: s1.clone(), }); // Act let copied_id = id; // Assert - let entry = arena - .get(copied_id) - .expect("The ID is expected to be valid!"); - assert_eq!(value1, entry.value1); - assert_eq!(value2, entry.value2); + let entry = copied_id.deref(); + assert_eq!(v1, entry.some_value); + assert_eq!(s1, entry.some_string); } #[rstest] @@ -130,113 +140,66 @@ fn alloc_creates_correct_number_of_buckets( let mut arena = RapIdArena::::new_with_bucket_size(items_per_bucket); // Act - for i in 0..alloc_count { - arena.alloc(Something { - value1: i, - value2: format!("i = {}", i), - }); - } + alloc_items(&mut arena, alloc_count); // Assert assert_eq!(alloc_count, arena.len()); - assert_eq!(expected_bucket_count, arena.buckets.len()); + let internals = arena.internals.read().unwrap(); + assert_eq!(expected_bucket_count, internals.buckets.len()); } #[test] fn index_operator_returns_expected_item() { // Arrange let mut arena = RapIdArena::::new(); - let value1 = 777; - let value2 = "a string".to_string(); + let v1 = 777; + let s1 = "a string".to_string(); // Act let id = arena.alloc(Something { - value1, - value2: value2.clone(), + some_value: v1, + some_string: s1.clone(), }); // Assert - let actual = &arena[id]; - assert_eq!(value1, actual.value1); - assert_eq!(value2, actual.value2); + let actual = id.deref(); + assert_eq!(v1, actual.some_value); + assert_eq!(s1, actual.some_string); } #[test] -fn get_item_in_second_bucket_returns_expected_item() { +fn deref_given_multiple_buckets_returns_each_item() { // Arrange let bucket_size = 5; let mut arena = RapIdArena::::new_with_bucket_size(bucket_size); - let mut ids = vec![]; - - for i in 0..=bucket_size { - ids.push(arena.alloc(Something { - value1: i, - value2: format!("i = {}", i), - })); - } - - // Act - let entry = arena.get(ids[5]).expect("The ID is expected to be valid!"); - - // Assert - assert_eq!(bucket_size, entry.value1); - assert_eq!(format!("i = {}", bucket_size), entry.value2); -} - -#[test] -fn get_with_invalid_id_returns_none() { - // Arrange - let arena = RapIdArena::::new(); - let id: RapId = RapId:: { - index: 123, - _t: PhantomData, - }; - - // Act - let value = arena.get(id); - - // Assert - assert!(value.is_none()); -} - -#[test] -fn get_mut_with_invalid_id_returns_none() { - // Arrange - let mut arena = RapIdArena::::new(); - let id: RapId = RapId:: { - index: 123, - _t: PhantomData, - }; + let count = (bucket_size as f32 * 111f32) as usize; + let ids = alloc_items(&mut arena, count); - // Act - let value = arena.get_mut(id); + for i in 0..count { + // Act + let entry = ids[i].deref(); - // Assert - assert!(value.is_none()); + // Assert + assert_eq!(i, entry.some_value); + assert_eq!(format!("i = {}", i), entry.some_string); + } } #[test] -fn get_mut_then_modified_modifies_correct_entry() { +fn get_then_modify_modifies_entry() { // Arrange let mut arena = RapIdArena::::new(); - let mut ids = vec![]; - let value1 = 111; - for i in 0..=2 { - ids.push(arena.alloc(Something { - value1: value1 + i * 111, - value2: format!("i = {}", i), - })); - } + let mut ids = alloc_items(&mut arena, 3); // Act - let mut something = arena.get_mut(ids[1]).expect("Expected ID to be valid!"); - something.value1 += 1; - something.value2 = "world".to_string(); + let something = ids[1].deref_mut(); + something.some_value += 1; + something.some_string = "world".to_string(); // Assert - let entry = arena.get(ids[1]).expect("Expected ID to be valid!"); - assert_eq!(223, entry.value1); - assert_eq!("world", entry.value2); + let entry = ids[1].deref(); + assert_eq!(2, entry.some_value); + assert_eq!("world", entry.some_string); } #[rstest] @@ -249,12 +212,7 @@ fn len_with_allocs_returns_correct_length( ) { // Arrange let mut arena = RapIdArena::::new_with_bucket_size(items_per_bucket); - for i in 0..alloc_count { - arena.alloc(Something { - value1: i, - value2: format!("entry {}", i), - }); - } + alloc_items(&mut arena, alloc_count); // Act let len = arena.len(); @@ -288,27 +246,83 @@ fn is_empty_given_non_empty_arena_returns_false() { assert!(!is_empty); } -#[rstest] -#[case(5, 0)] -#[case(5, 503)] -fn reset_results_in_single_empty_bucket( - #[case] items_per_bucket: usize, - #[case] alloc_count: usize, -) { - // Arrange - let mut arena = RapIdArena::::new_with_bucket_size(items_per_bucket); - for i in 0..alloc_count { - arena.alloc(Something { - value1: i, - value2: format!("entry {}", i), - }); +#[test] +fn deref_multi_threaded_test() { + let mut arena = RapIdArena::::new_with_bucket_size(1); + let ids = alloc_items(&mut arena, 7); + std::thread::scope(|s| { + for _ in 0..14 { + s.spawn(|| { + let item_count = arena.len(); + for i in 0..item_count * 503 { + let id = &ids[i % item_count]; + let item = id.deref(); + black_box({ + let _ = item.some_value + 1; + }) + } + }); + } + }); +} + +#[test] +fn deref_mut_multi_threaded_test() { + let mut arena = RapIdArena::>::new_with_bucket_size(1); + const ITEM_COUNT: usize = 7; + let mut ids = vec![]; + for i in 0..ITEM_COUNT { + ids.push(arena.alloc(RwLock::new(Something { + some_value: i, + some_string: format!("i = {}", i), + }))); } + std::thread::scope(|s| { + for _ in 0..(ITEM_COUNT * 3) { + s.spawn(|| { + for i in 0..ITEM_COUNT * 11 { + let mut id = ids[i % ITEM_COUNT]; + let mut item = id.deref_mut().write().unwrap(); + + let some_value = item.some_value + 1; + let some_string = format!("i = {}", some_value); + + item.some_value = some_value; + item.some_string = some_string.clone(); + + thread::sleep(Duration::from_millis(1)); + + assert_eq!(some_value, item.some_value); + assert_eq!(some_string, item.some_string) + } + }); + } + }); +} + +#[test] +fn rayon_test() { + // Arrange + let mut arena = RapIdArena::::new_with_bucket_size(5); + let count = 31; + alloc_items(&mut arena, count); + // Since we start from 0 not 1, our formula is n(n-1)/2 instead of n(n+1)/2 + let expected_sum = count * (count - 1) / 2; + // Act - arena.reset(); + let sum: usize = arena + .iter() + .par_bridge() + .map(|id| id.deref().some_value) + .sum(); // Assert - assert_eq!(0, arena.bucket_index); - assert_eq!(1, arena.buckets.len()); - assert_eq!(items_per_bucket, arena.items_per_bucket); + assert_eq!(expected_sum, sum); +} + +#[test] +#[should_panic] +fn new_with_bucket_size_of_zero_should_panic() { + RapIdArena::::new_with_bucket_size(0); } diff --git a/src/size.rs b/src/size.rs index 8bc1e05..4859e52 100644 --- a/src/size.rs +++ b/src/size.rs @@ -109,3 +109,10 @@ impl Size { config[config.len() - 1] } } + +impl From for u64 { + #[inline] + fn from(value: Size) -> Self { + value.get_value() + } +} diff --git a/src/size_test.rs b/src/size_test.rs index 0fbdb9e..de4ff43 100644 --- a/src/size_test.rs +++ b/src/size_test.rs @@ -84,3 +84,16 @@ fn get_value_returns_correct_value() { // Assert assert_eq!(VALUE, value); } + +#[test] +fn into_returns_correct_value() { + // Arrange + const EXPECTED: u64 = 123; + let size = Size::new(EXPECTED); + + // Act + let value: u64 = size.into(); + + // Assert + assert_eq!(EXPECTED, value); +} diff --git a/src/space_bench.rs b/src/space_bench.rs index aeb5cd8..e1b88a6 100644 --- a/src/space_bench.rs +++ b/src/space_bench.rs @@ -21,7 +21,7 @@ mod test_utils; mod cli; -pub(crate) const DEFAULT_SIZE_THRESHOLD_PERCENTAGE: u8 = 1; +const DEFAULT_SIZE_THRESHOLD_PERCENTAGE: u8 = 1; #[derive(Clone, Debug, Parser)] #[clap(