Some code cleanups (#4066)

## Motivation

Found some small code cleanups to do while working on the ScyllaDb code

## Proposal

Clean up the code a bit

## Test Plan

CI

## Release Plan

- Nothing to do / These changes follow the usual release cycle.

Author: ndr-ds

linera-views/src/common.rs

@@ -74,9 +74,8 @@ impl DeletionSet {
 pub(crate) fn get_upper_bound_option(key_prefix: &[u8]) -> Option<Vec<u8>> {
     let len = key_prefix.len();
     for i in (0..len).rev() {
-        let val = key_prefix[i];
-        if val < u8::MAX {
-            let mut upper_bound = key_prefix[0..i + 1].to_vec();
+        if key_prefix[i] < u8::MAX {
+            let mut upper_bound = key_prefix[0..=i].to_vec();
             upper_bound[i] += 1;
             return Some(upper_bound);
         }

linera-views/src/random.rs

@@ -1,70 +1,52 @@
 // Copyright (c) Zefchain Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use rand::{Rng, SeedableRng};
+#[cfg(target_arch = "wasm32")]
+use std::sync::{Mutex, MutexGuard, OnceLock};
+
+use rand::{rngs::SmallRng, Rng, SeedableRng};
 
 // The following seed is chosen to have equal numbers of 1s and 0s, as advised by
 // https://docs.rs/rand/latest/rand/rngs/struct.SmallRng.html
 // Specifically, it's "01" × 32 in binary
 const RNG_SEED: u64 = 6148914691236517205;
 
 /// A deterministic RNG.
-pub type DeterministicRng = rand::rngs::SmallRng;
+pub type DeterministicRng = SmallRng;
 
 /// A RNG that is non-deterministic if the platform supports it.
-pub struct NonDeterministicRng(
-    #[cfg(target_arch = "wasm32")] std::sync::MutexGuard<'static, DeterministicRng>,
-    #[cfg(not(target_arch = "wasm32"))] rand::rngs::ThreadRng,
-);
-
-impl NonDeterministicRng {
-    /// Access the internal RNG.
-    pub fn rng_mut(&mut self) -> &mut impl Rng {
-        #[cfg(target_arch = "wasm32")]
-        {
-            &mut *self.0
-        }
-        #[cfg(not(target_arch = "wasm32"))]
-        {
-            &mut self.0
-        }
-    }
-}
+#[cfg(not(target_arch = "wasm32"))]
+pub type NonDeterministicRng = rand::rngs::ThreadRng;
+/// A RNG that is non-deterministic if the platform supports it.
+#[cfg(target_arch = "wasm32")]
+pub type NonDeterministicRng = MutexGuard<'static, DeterministicRng>;
 
 /// Returns a deterministic RNG for testing.
 pub fn make_deterministic_rng() -> DeterministicRng {
-    rand::rngs::SmallRng::seed_from_u64(RNG_SEED)
+    SmallRng::seed_from_u64(RNG_SEED)
 }
 
 /// Returns a non-deterministic RNG where supported.
 pub fn make_nondeterministic_rng() -> NonDeterministicRng {
     #[cfg(target_arch = "wasm32")]
     {
-        use std::sync::{Mutex, OnceLock};
-
-        use rand::rngs::SmallRng;
-
         static RNG: OnceLock<Mutex<SmallRng>> = OnceLock::new();
-        NonDeterministicRng(
-            RNG.get_or_init(|| Mutex::new(make_deterministic_rng()))
-                .lock()
-                .expect("failed to lock RNG mutex"),
-        )
+        RNG.get_or_init(|| Mutex::new(make_deterministic_rng()))
+            .lock()
+            .expect("failed to lock RNG mutex")
     }
 
     #[cfg(not(target_arch = "wasm32"))]
     {
-        NonDeterministicRng(rand::thread_rng())
+        rand::thread_rng()
     }
 }
 
 /// Get a random alphanumeric string that can be used for all tests.
 pub fn generate_random_alphanumeric_string(length: usize, charset: &[u8]) -> String {
     (0..length)
         .map(|_| {
-            let random_index = make_nondeterministic_rng()
-                .rng_mut()
-                .gen_range(0..charset.len());
+            let random_index = make_nondeterministic_rng().gen_range(0..charset.len());
             charset[random_index] as char
         })
         .collect()

linera-views/src/test_utils/mod.rs

@@ -22,32 +22,38 @@ use crate::{
     },
 };
 
+/// The size of the small value used for tests.
+pub const SMALL_BYTE_UPPER_LIMIT: u8 = 3;
+
 /// Returns a random key prefix used for tests
 pub fn get_random_key_prefix() -> Vec<u8> {
     let mut key_prefix = vec![0];
-    let value: usize = make_nondeterministic_rng().rng_mut().gen();
+    let value: usize = make_nondeterministic_rng().gen();
     bcs::serialize_into(&mut key_prefix, &value).unwrap();
     key_prefix
 }
 
-/// Takes a random number generator, a `key_prefix` and extends it by n random bytes.
-pub fn get_random_byte_vector<R: Rng>(rng: &mut R, key_prefix: &[u8], n: usize) -> Vec<u8> {
+fn get_random_byte_vector_with_byte_upper_limit<R: Rng>(
+    rng: &mut R,
+    key_prefix: &[u8],
+    n: usize,
+    byte_upper_limit: u8,
+) -> Vec<u8> {
     let mut v = key_prefix.to_vec();
     for _ in 0..n {
-        let val = rng.gen_range(0..256) as u8;
+        let val = rng.gen_range(0..=byte_upper_limit);
         v.push(val);
     }
     v
 }
 
-/// Appends a small value to a key making collisions likely.
-pub fn get_small_key_space<R: Rng>(rng: &mut R, key_prefix: &[u8], n: usize) -> Vec<u8> {
-    let mut key = key_prefix.to_vec();
-    for _ in 0..n {
-        let byte = rng.gen_range(0..4) as u8;
-        key.push(byte);
-    }
-    key
+fn get_small_key_space<R: Rng>(rng: &mut R, key_prefix: &[u8], n: usize) -> Vec<u8> {
+    get_random_byte_vector_with_byte_upper_limit(rng, key_prefix, n, SMALL_BYTE_UPPER_LIMIT)
+}
+
+/// Takes a random number generator, a `key_prefix` and extends it by n random bytes.
+pub fn get_random_byte_vector<R: Rng>(rng: &mut R, key_prefix: &[u8], n: usize) -> Vec<u8> {
+    get_random_byte_vector_with_byte_upper_limit(rng, key_prefix, n, u8::MAX)
 }
 
 /// Builds a random k element subset of n
@@ -61,36 +67,43 @@ pub fn get_random_kset<R: Rng>(rng: &mut R, n: usize, k: usize) -> Vec<usize> {
 }
 
 /// Takes a random number generator, a `key_prefix` and generates
-/// pairs `(key, value)` with key obtained by appending 8 bytes at random to `key_prefix`
-/// and value obtained by appending 8 bytes to the trivial vector.
-/// We return n such `(key, value)` pairs which are all distinct
+/// pairs `(key, value)` with key obtained by appending `len_key` random bytes to `key_prefix`
+/// and value obtained by creating a vector with `len_value` random bytes.
+/// We return n such `(key, value)` pairs which are all distinct.
 pub fn get_random_key_values_prefix<R: Rng>(
     rng: &mut R,
     key_prefix: Vec<u8>,
     len_key: usize,
     len_value: usize,
     num_entries: usize,
+    key_byte_upper_limit: u8,
 ) -> Vec<(Vec<u8>, Vec<u8>)> {
-    loop {
-        let mut v_ret = Vec::new();
-        let mut vector_set = HashSet::new();
-        for _ in 0..num_entries {
-            let v1 = get_random_byte_vector(rng, &key_prefix, len_key);
-            let v2 = get_random_byte_vector(rng, &Vec::new(), len_value);
-            let v12 = (v1.clone(), v2);
-            vector_set.insert(v1);
-            v_ret.push(v12);
-        }
-        if vector_set.len() == num_entries {
-            return v_ret;
-        }
+    let mut key_value_pairs = Vec::new();
+    let mut unique_keys = HashSet::new();
+    for _ in 0..num_entries {
+        let key = loop {
+            let key = get_random_byte_vector_with_byte_upper_limit(
+                rng,
+                &key_prefix,
+                len_key,
+                key_byte_upper_limit,
+            );
+            if !unique_keys.contains(&key) {
+                unique_keys.insert(key.clone());
+                break key;
+            }
+        };
+        let value = get_random_byte_vector(rng, &Vec::new(), len_value);
+        key_value_pairs.push((key, value));
     }
+
+    key_value_pairs
 }
 
 /// Takes a random number generator `rng`, a number n and returns n random `(key, value)`
 /// which are all distinct with key and value being of length 8.
 pub fn get_random_key_values<R: Rng>(rng: &mut R, num_entries: usize) -> Vec<(Vec<u8>, Vec<u8>)> {
-    get_random_key_values_prefix(rng, Vec::new(), 8, 8, num_entries)
+    get_random_key_values_prefix(rng, Vec::new(), 8, 8, num_entries, u8::MAX)
 }
 
 type VectorPutDelete = (Vec<(Vec<u8>, Vec<u8>)>, usize);
@@ -101,8 +114,7 @@ pub fn get_random_key_value_operations<R: Rng>(
     num_entries: usize,
     k: usize,
 ) -> VectorPutDelete {
-    let key_value_vector = get_random_key_values_prefix(rng, Vec::new(), 8, 8, num_entries);
-    (key_value_vector, k)
+    (get_random_key_values(rng, num_entries), k)
 }
 
 /// A random reordering of the puts and deletes.
@@ -234,31 +246,39 @@ pub async fn run_reads<S: RestrictedKeyValueStore>(store: S, key_values: Vec<(Ve
     }
 }
 
-fn get_random_key_values1(num_entries: usize, len_value: usize) -> Vec<(Vec<u8>, Vec<u8>)> {
+/// Generates a list of random key-values with no duplicates
+pub fn get_random_key_values_with_sizes(
+    num_entries: usize,
+    len_key: usize,
+    len_value: usize,
+) -> Vec<(Vec<u8>, Vec<u8>)> {
     let key_prefix = vec![0];
     let mut rng = make_deterministic_rng();
-    get_random_key_values_prefix(&mut rng, key_prefix, 8, len_value, num_entries)
+    get_random_key_values_prefix(
+        &mut rng,
+        key_prefix,
+        len_key,
+        len_value,
+        num_entries,
+        u8::MAX,
+    )
 }
 
-/// Generates a list of random key-values with no duplicates
-pub fn get_random_key_values2(
+fn get_random_key_values_with_small_keys(
     num_entries: usize,
     len_key: usize,
     len_value: usize,
 ) -> Vec<(Vec<u8>, Vec<u8>)> {
-    let mut rng = make_deterministic_rng();
     let key_prefix = vec![0];
-    let mut key_values = Vec::new();
-    let mut key_set = HashSet::new();
-    for _ in 0..num_entries {
-        let key = get_small_key_space(&mut rng, &key_prefix, len_key);
-        if !key_set.contains(&key) {
-            key_set.insert(key.clone());
-            let value = get_random_byte_vector(&mut rng, &[], len_value);
-            key_values.push((key, value));
-        }
-    }
-    key_values
+    let mut rng = make_deterministic_rng();
+    get_random_key_values_prefix(
+        &mut rng,
+        key_prefix,
+        len_key,
+        len_value,
+        num_entries,
+        SMALL_BYTE_UPPER_LIMIT,
+    )
 }
 
 /// Adds a prefix to a list of key-values
@@ -276,11 +296,11 @@ pub fn add_prefix(prefix: &[u8], key_values: Vec<(Vec<u8>, Vec<u8>)>) -> Vec<(Ve
 /// We build a number of scenarios for testing the reads.
 pub fn get_random_test_scenarios() -> Vec<Vec<(Vec<u8>, Vec<u8>)>> {
     vec![
-        get_random_key_values1(7, 3),
-        get_random_key_values1(150, 3),
-        get_random_key_values1(30, 10),
-        get_random_key_values2(30, 4, 10),
-        get_random_key_values2(30, 4, 100),
+        get_random_key_values_with_sizes(7, 8, 3),
+        get_random_key_values_with_sizes(150, 8, 3),
+        get_random_key_values_with_sizes(30, 8, 10),
+        get_random_key_values_with_small_keys(30, 4, 10),
+        get_random_key_values_with_small_keys(30, 4, 100),
     ]
 }
 

linera-views/src/test_utils/performance.rs

@@ -6,7 +6,7 @@ use std::time::{Duration, Instant};
 use crate::{
     batch::Batch,
     store::{KeyValueStore, TestKeyValueStore},
-    test_utils::{add_prefix, get_random_key_values2},
+    test_utils::{add_prefix, get_random_key_values_with_small_keys},
 };
 
 // We generate about 2000 keys of length 11 with a key of length 10000
@@ -46,7 +46,7 @@ where
     for _ in 0..iterations {
         let key_values = add_prefix(
             PREFIX,
-            get_random_key_values2(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
+            get_random_key_values_with_small_keys(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
         );
         let mut batch = Batch::new();
         for key_value in &key_values[..NUM_INSERT] {
@@ -76,7 +76,7 @@ where
     for _ in 0..iterations {
         let key_values = add_prefix(
             PREFIX,
-            get_random_key_values2(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
+            get_random_key_values_with_small_keys(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
         );
         let mut batch = Batch::new();
         for key_value in &key_values[..NUM_INSERT] {
@@ -108,7 +108,7 @@ where
     for _ in 0..iterations {
         let key_values = add_prefix(
             PREFIX,
-            get_random_key_values2(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
+            get_random_key_values_with_small_keys(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
         );
         let mut batch = Batch::new();
         for key_value in &key_values {
@@ -136,7 +136,7 @@ where
     for _ in 0..iterations {
         let key_values = add_prefix(
             PREFIX,
-            get_random_key_values2(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
+            get_random_key_values_with_small_keys(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
         );
         let mut batch = Batch::new();
         for key_value in &key_values {
@@ -167,7 +167,7 @@ where
     for _ in 0..iterations {
         let key_values = add_prefix(
             PREFIX,
-            get_random_key_values2(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
+            get_random_key_values_with_small_keys(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
         );
         let mut batch = Batch::new();
         for key_value in &key_values {
@@ -197,7 +197,7 @@ where
     for _ in 0..iterations {
         let key_values = add_prefix(
             PREFIX,
-            get_random_key_values2(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
+            get_random_key_values_with_small_keys(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
         );
         let mut batch = Batch::new();
         for key_value in &key_values {
@@ -226,7 +226,7 @@ pub async fn write_batch<S: TestKeyValueStore>(iterations: u64) -> Duration {
     for _ in 0..iterations {
         let key_values = add_prefix(
             PREFIX,
-            get_random_key_values2(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
+            get_random_key_values_with_small_keys(NUM_ENTRIES, LEN_KEY, LEN_VALUE),
         );
         let mut batch = Batch::new();
         for key_value in &key_values {