Fix use of PRNG in Rope benchmarks (#39949)

Martin Pool created 2 months ago

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

Change summary

crates/rope/benches/rope_benchmark.rs | 48 +++++++++++++++++-----------
1 file changed, 29 insertions(+), 19 deletions(-)

Detailed changes

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<Range<usize>> {
+fn generate_random_rope_ranges(rng: &mut StdRng, rope: &Rope) -> Vec<Range<usize>> {
     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<Range<usize>
     ranges
 }
 
-fn generate_random_rope_points(mut rng: StdRng, rope: &Rope) -> Vec<Point> {
+fn generate_random_rope_points(rng: &mut StdRng, rope: &Rope) -> Vec<Point> {
     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();