From 65acf125fb028b2bfb39564fdf565785871ea148 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Thu, 16 Oct 2025 15:15:28 -0700 Subject: [PATCH] Fix use of PRNG in Rope benchmarks (#39949) Using a seeded PRNG to produce consistent test data and reduce variability makes sense. However, the way it was used previously, by always cloning the RNG, means that every generated string is the same, and every offset is the same. After this change, the tested value stream should still be the same on each run of the benchmark, but the values within each run will vary. The `generate_random_text` measured in chars also seems possibly inconsistent with later comments about it being a number of bytes. Release Notes: - N/A --- crates/rope/benches/rope_benchmark.rs | 48 ++++++++++++++++----------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/crates/rope/benches/rope_benchmark.rs b/crates/rope/benches/rope_benchmark.rs index bf891a4f837c3e63975aea1233b15c913758c175..4ae6f1b54f19b756e12cc399181bdf6b5d894ad5 100644 --- a/crates/rope/benches/rope_benchmark.rs +++ b/crates/rope/benches/rope_benchmark.rs @@ -9,18 +9,21 @@ use rope::{Point, Rope}; use sum_tree::Bias; use util::RandomCharIter; -fn generate_random_text(mut rng: StdRng, text_len: usize) -> String { - RandomCharIter::new(&mut rng).take(text_len).collect() +/// Generate a random text of the given length using the provided RNG. +/// +/// *Note*: The length is in *characters*, not bytes. +fn generate_random_text(rng: &mut StdRng, text_len: usize) -> String { + RandomCharIter::new(rng).take(text_len).collect() } -fn generate_random_rope(rng: StdRng, text_len: usize) -> Rope { +fn generate_random_rope(rng: &mut StdRng, text_len: usize) -> Rope { let text = generate_random_text(rng, text_len); let mut rope = Rope::new(); rope.push(&text); rope } -fn generate_random_rope_ranges(mut rng: StdRng, rope: &Rope) -> Vec> { +fn generate_random_rope_ranges(rng: &mut StdRng, rope: &Rope) -> Vec> { let range_max_len = 50; let num_ranges = rope.len() / range_max_len; @@ -47,7 +50,7 @@ fn generate_random_rope_ranges(mut rng: StdRng, rope: &Rope) -> Vec ranges } -fn generate_random_rope_points(mut rng: StdRng, rope: &Rope) -> Vec { +fn generate_random_rope_points(rng: &mut StdRng, rope: &Rope) -> Vec { let num_points = rope.len() / 10; let mut points = Vec::new(); @@ -61,14 +64,14 @@ fn rope_benchmarks(c: &mut Criterion) { static SEED: u64 = 9999; static KB: usize = 1024; - let rng = StdRng::seed_from_u64(SEED); let sizes = [4 * KB, 64 * KB]; let mut group = c.benchmark_group("push"); for size in sizes.iter() { group.throughput(Throughput::Bytes(*size as u64)); group.bench_with_input(BenchmarkId::from_parameter(size), &size, |b, &size| { - let text = generate_random_text(rng.clone(), *size); + let mut rng = StdRng::seed_from_u64(SEED); + let text = generate_random_text(&mut rng, *size); b.iter(|| { let mut rope = Rope::new(); @@ -84,9 +87,11 @@ fn rope_benchmarks(c: &mut Criterion) { for size in sizes.iter() { group.throughput(Throughput::Bytes(*size as u64)); group.bench_with_input(BenchmarkId::from_parameter(size), &size, |b, &size| { + let mut rng = StdRng::seed_from_u64(SEED); let mut random_ropes = Vec::new(); for _ in 0..5 { - random_ropes.push(generate_random_rope(rng.clone(), *size)); + let rope = generate_random_rope(&mut rng, *size); + random_ropes.push(rope); } b.iter(|| { @@ -103,10 +108,11 @@ fn rope_benchmarks(c: &mut Criterion) { for size in sizes.iter() { group.throughput(Throughput::Bytes(*size as u64)); group.bench_with_input(BenchmarkId::from_parameter(size), &size, |b, &size| { - let rope = generate_random_rope(rng.clone(), *size); + let mut rng = StdRng::seed_from_u64(SEED); + let rope = generate_random_rope(&mut rng, *size); b.iter_batched( - || generate_random_rope_ranges(rng.clone(), &rope), + || generate_random_rope_ranges(&mut rng, &rope), |ranges| { for range in ranges.iter() { rope.slice(range.clone()); @@ -122,10 +128,11 @@ fn rope_benchmarks(c: &mut Criterion) { for size in sizes.iter() { group.throughput(Throughput::Bytes(*size as u64)); group.bench_with_input(BenchmarkId::from_parameter(size), &size, |b, &size| { - let rope = generate_random_rope(rng.clone(), *size); + let mut rng = StdRng::seed_from_u64(SEED); + let rope = generate_random_rope(&mut rng, *size); b.iter_batched( - || generate_random_rope_ranges(rng.clone(), &rope), + || generate_random_rope_ranges(&mut rng, &rope), |ranges| { for range in ranges.iter() { let bytes = rope.bytes_in_range(range.clone()); @@ -142,7 +149,8 @@ fn rope_benchmarks(c: &mut Criterion) { for size in sizes.iter() { group.throughput(Throughput::Bytes(*size as u64)); group.bench_with_input(BenchmarkId::from_parameter(size), &size, |b, &size| { - let rope = generate_random_rope(rng.clone(), *size); + let mut rng = StdRng::seed_from_u64(SEED); + let rope = generate_random_rope(&mut rng, *size); b.iter(|| { let chars = rope.chars().count(); @@ -156,10 +164,11 @@ fn rope_benchmarks(c: &mut Criterion) { for size in sizes.iter() { group.throughput(Throughput::Bytes(*size as u64)); group.bench_with_input(BenchmarkId::from_parameter(size), &size, |b, &size| { - let rope = generate_random_rope(rng.clone(), *size); + let mut rng = StdRng::seed_from_u64(SEED); + let rope = generate_random_rope(&mut rng, *size); b.iter_batched( - || generate_random_rope_points(rng.clone(), &rope), + || generate_random_rope_points(&mut rng, &rope), |offsets| { for offset in offsets.iter() { black_box(rope.clip_point(*offset, Bias::Left)); @@ -176,10 +185,11 @@ fn rope_benchmarks(c: &mut Criterion) { for size in sizes.iter() { group.throughput(Throughput::Bytes(*size as u64)); group.bench_with_input(BenchmarkId::from_parameter(size), &size, |b, &size| { - let rope = generate_random_rope(rng.clone(), *size); + let mut rng = StdRng::seed_from_u64(SEED); + let rope = generate_random_rope(&mut rng, *size); b.iter_batched( - || generate_random_rope_points(rng.clone(), &rope), + || generate_random_rope_points(&mut rng, &rope), |offsets| { for offset in offsets.iter() { black_box(rope.point_to_offset(*offset)); @@ -195,11 +205,11 @@ fn rope_benchmarks(c: &mut Criterion) { for size in sizes.iter() { group.throughput(Throughput::Bytes(*size as u64)); group.bench_with_input(BenchmarkId::from_parameter(size), &size, |b, &size| { - let rope = generate_random_rope(rng.clone(), *size); + let mut rng = StdRng::seed_from_u64(SEED); + let rope = generate_random_rope(&mut rng, *size); b.iter_batched( || { - let mut rng = rng.clone(); let num_points = rope.len() / 10; let mut points = Vec::new();