Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 24 additions & 37 deletions linera-core/src/unit_tests/value_cache_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@ use linera_base::{
};
use linera_chain::types::Timeout;

use super::{ValueCache, DEFAULT_VALUE_CACHE_SIZE};
use super::ValueCache;

/// Test cache size for unit tests.
const TEST_CACHE_SIZE: usize = 10;

/// Tests attempt to retrieve non-existent value.
#[test]
fn test_retrieve_missing_value() {
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::default();
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::new(TEST_CACHE_SIZE);
let hash = CryptoHash::test_hash("Missing value");

assert!(cache.get(&hash).is_none());
Expand All @@ -26,7 +29,7 @@ fn test_retrieve_missing_value() {
/// Tests inserting a certificate value in the cache.
#[test]
fn test_insert_single_certificate_value() {
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::default();
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::new(TEST_CACHE_SIZE);
let value = create_dummy_certificate_value(0);
let hash = value.hash();

Expand All @@ -39,9 +42,8 @@ fn test_insert_single_certificate_value() {
/// Tests inserting many certificate values in the cache, one-by-one.
#[test]
fn test_insert_many_certificate_values_individually() {
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::default();
let values =
create_dummy_certificate_values(0..(DEFAULT_VALUE_CACHE_SIZE as u64)).collect::<Vec<_>>();
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::new(TEST_CACHE_SIZE);
let values = create_dummy_certificate_values(0..(TEST_CACHE_SIZE as u64)).collect::<Vec<_>>();

for value in &values {
assert!(cache.insert(Cow::Borrowed(value)));
Expand All @@ -61,9 +63,8 @@ fn test_insert_many_certificate_values_individually() {
/// Tests inserting many values in the cache, all-at-once.
#[test]
fn test_insert_many_values_together() {
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::default();
let values =
create_dummy_certificate_values(0..(DEFAULT_VALUE_CACHE_SIZE as u64)).collect::<Vec<_>>();
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::new(TEST_CACHE_SIZE);
let values = create_dummy_certificate_values(0..(TEST_CACHE_SIZE as u64)).collect::<Vec<_>>();

cache.insert_all(values.iter().map(Cow::Borrowed));

Expand All @@ -81,9 +82,8 @@ fn test_insert_many_values_together() {
/// Tests re-inserting many values in the cache, all-at-once.
#[test]
fn test_reinsertion_of_values() {
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::default();
let values =
create_dummy_certificate_values(0..(DEFAULT_VALUE_CACHE_SIZE as u64)).collect::<Vec<_>>();
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::new(TEST_CACHE_SIZE);
let values = create_dummy_certificate_values(0..(TEST_CACHE_SIZE as u64)).collect::<Vec<_>>();

cache.insert_all(values.iter().map(Cow::Borrowed));

Expand All @@ -105,9 +105,8 @@ fn test_reinsertion_of_values() {
/// Tests eviction of one entry.
#[test]
fn test_one_eviction() {
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::default();
let values =
create_dummy_certificate_values(0..=(DEFAULT_VALUE_CACHE_SIZE as u64)).collect::<Vec<_>>();
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::new(TEST_CACHE_SIZE);
let values = create_dummy_certificate_values(0..=(TEST_CACHE_SIZE as u64)).collect::<Vec<_>>();

cache.insert_all(values.iter().map(Cow::Borrowed));

Expand All @@ -128,18 +127,12 @@ fn test_one_eviction() {
/// Tests eviction of the second entry.
#[test]
fn test_eviction_of_second_entry() {
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::default();
let values =
create_dummy_certificate_values(0..=(DEFAULT_VALUE_CACHE_SIZE as u64)).collect::<Vec<_>>();

cache.insert_all(
values
.iter()
.take(DEFAULT_VALUE_CACHE_SIZE)
.map(Cow::Borrowed),
);
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::new(TEST_CACHE_SIZE);
let values = create_dummy_certificate_values(0..=(TEST_CACHE_SIZE as u64)).collect::<Vec<_>>();

cache.insert_all(values.iter().take(TEST_CACHE_SIZE).map(Cow::Borrowed));
cache.get(&values[0].hash());
assert!(cache.insert(Cow::Borrowed(&values[DEFAULT_VALUE_CACHE_SIZE])));
assert!(cache.insert(Cow::Borrowed(&values[TEST_CACHE_SIZE])));

assert!(cache.contains(&values[0].hash()));
assert_eq!(cache.get(&values[0].hash()).as_ref(), Some(&values[0]));
Expand Down Expand Up @@ -167,18 +160,12 @@ fn test_eviction_of_second_entry() {
/// Tests if reinsertion of the first entry promotes it so that it's not evicted so soon.
#[test]
fn test_promotion_of_reinsertion() {
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::default();
let values =
create_dummy_certificate_values(0..=(DEFAULT_VALUE_CACHE_SIZE as u64)).collect::<Vec<_>>();

cache.insert_all(
values
.iter()
.take(DEFAULT_VALUE_CACHE_SIZE)
.map(Cow::Borrowed),
);
let cache = ValueCache::<CryptoHash, Hashed<Timeout>>::new(TEST_CACHE_SIZE);
let values = create_dummy_certificate_values(0..=(TEST_CACHE_SIZE as u64)).collect::<Vec<_>>();

cache.insert_all(values.iter().take(TEST_CACHE_SIZE).map(Cow::Borrowed));
assert!(!cache.insert(Cow::Borrowed(&values[0])));
assert!(cache.insert(Cow::Borrowed(&values[DEFAULT_VALUE_CACHE_SIZE])));
assert!(cache.insert(Cow::Borrowed(&values[TEST_CACHE_SIZE])));

assert!(cache.contains(&values[0].hash()));
assert_eq!(cache.get(&values[0].hash()).as_ref(), Some(&values[0]));
Expand Down
17 changes: 4 additions & 13 deletions linera-core/src/value_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ use std::{borrow::Cow, hash::Hash, num::NonZeroUsize, sync::Mutex};
use linera_base::{crypto::CryptoHash, hashed::Hashed};
use lru::LruCache;

/// The default cache size.
pub const DEFAULT_VALUE_CACHE_SIZE: usize = 10_000;

/// A counter metric for the number of cache hits in the [`ValueCache`].
#[cfg(with_metrics)]
mod metrics {
Expand Down Expand Up @@ -51,24 +48,18 @@ where
cache: Mutex<LruCache<K, V>>,
}

impl<K, V> Default for ValueCache<K, V>
impl<K, V> ValueCache<K, V>
where
K: Hash + Eq + PartialEq + Copy,
{
fn default() -> Self {
let size = NonZeroUsize::try_from(DEFAULT_VALUE_CACHE_SIZE)
.expect("Default cache size is larger than zero");

/// Creates a new `ValueCache` with the given size.
pub fn new(size: usize) -> Self {
let size = NonZeroUsize::try_from(size).expect("Cache size is larger than zero");
ValueCache {
cache: Mutex::new(LruCache::new(size)),
}
}
}

impl<K, V> ValueCache<K, V>
where
K: Hash + Eq + PartialEq + Copy,
{
/// Inserts a `V` into the cache, if it's not already present.
pub fn insert_owned(&self, key: &K, value: V) -> bool {
let mut cache = self.cache.lock().unwrap();
Expand Down
11 changes: 7 additions & 4 deletions linera-core/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ use crate::{
value_cache::ValueCache,
};

const BLOCK_CACHE_SIZE: usize = 5_000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it really OOMed with 10_000 I'd do more than just halve it. I'd go down to 1_000 or even 100 maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In principle I agree, but the more we lower this the more we seem to be penalized in performance... So 5k seems like a good compromise

const EXECUTION_STATE_CACHE_SIZE: usize = 10_000;

#[cfg(test)]
#[path = "unit_tests/worker_tests.rs"]
mod worker_tests;
Expand Down Expand Up @@ -345,8 +348,8 @@ where
nickname,
storage,
chain_worker_config: ChainWorkerConfig::default().with_key_pair(key_pair),
block_cache: Arc::new(ValueCache::default()),
execution_state_cache: Arc::new(ValueCache::default()),
block_cache: Arc::new(ValueCache::new(BLOCK_CACHE_SIZE)),
execution_state_cache: Arc::new(ValueCache::new(EXECUTION_STATE_CACHE_SIZE)),
tracked_chains: None,
delivery_notifiers: Arc::default(),
chain_worker_tasks: Arc::default(),
Expand All @@ -364,8 +367,8 @@ where
nickname,
storage,
chain_worker_config: ChainWorkerConfig::default(),
block_cache: Arc::new(ValueCache::default()),
execution_state_cache: Arc::new(ValueCache::default()),
block_cache: Arc::new(ValueCache::new(BLOCK_CACHE_SIZE)),
execution_state_cache: Arc::new(ValueCache::new(EXECUTION_STATE_CACHE_SIZE)),
tracked_chains: Some(tracked_chains),
delivery_notifiers: Arc::default(),
chain_worker_tasks: Arc::default(),
Expand Down
Loading