Skip to content

Commit 2120dc8

Browse files
committed
simplify versioned lru
1 parent c4952fc commit 2120dc8

File tree

2 files changed

+74
-129
lines changed

2 files changed

+74
-129
lines changed

src/cache.rs

Lines changed: 60 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use alloy_trie::Nibbles;
33
use std::{
44
collections::HashMap,
55
num::NonZeroUsize,
6-
sync::{Arc, RwLock, RwLockReadGuard},
6+
sync::{Arc, RwLock},
77
};
88

99
/// An entry in the versioned LRU cache with doubly-linked list indices.
@@ -162,7 +162,7 @@ impl VersionedLru {
162162
self.tail = prev_idx;
163163
}
164164

165-
// Mark as removed (we could also compact the list, but this is simpler)
165+
// Mark as removed
166166
self.lru[lru_idx].lru_prev = None;
167167
self.lru[lru_idx].lru_next = None;
168168
}
@@ -184,7 +184,7 @@ impl VersionedLru {
184184
}
185185
}
186186

187-
// Remove obsolete entries from LRU list
187+
// Remove from LRU list
188188
self.lru.retain(|entry| entry.snapshot_id >= min_id);
189189

190190
// Rebuild LRU pointers after retention
@@ -215,68 +215,28 @@ impl CacheManager {
215215
CacheManager { cache: Arc::new(RwLock::new(VersionedLru::new(max_size.get()))) }
216216
}
217217

218-
/// Provides a reader handle to the cache.
219-
/// Multiple readers can exist concurrently.
220-
pub fn read(&self) -> Reader {
221-
Reader { guard: self.cache.read().unwrap() }
222-
}
223-
224-
/// Provides a writer handle to the cache.
225-
/// This briefly acquires a lock to clone the cache, then releases it.
226-
pub fn write(&self) -> Writer {
227-
Writer { cache: Arc::clone(&self.cache) }
228-
}
229-
230-
/// Sets the minimum snapshot ID for proactive cache purging.
231-
pub fn set_min_snapshot_id(&self, min_snapshot_id: SnapshotId) {
232-
let mut guard = self.cache.write().unwrap();
233-
guard.set_min_snapshot_id(min_snapshot_id);
234-
}
235-
}
236-
237-
/// A handle for reading from the cache.
238-
/// Dropping this struct releases the read lock.
239-
#[derive(Debug)]
240-
pub struct Reader<'a> {
241-
guard: RwLockReadGuard<'a, VersionedLru>,
242-
}
243-
244-
impl<'a> Reader<'a> {
245-
/// Gets a value for the given key and snapshot ID without updating LRU state.
218+
/// Gets a value for the given key and snapshot ID and updates LRU state.
246219
pub fn get(&self, snapshot_id: SnapshotId, key: &Nibbles) -> Option<(PageId, u8)> {
247-
let versions = self.guard.entries.get(key)?;
248-
versions
249-
.iter()
250-
.rev()
251-
.find(|entry| entry.snapshot_id <= snapshot_id)
252-
.and_then(|entry| entry.value)
220+
let mut guard = self.cache.write().unwrap();
221+
guard.get(key, snapshot_id)
253222
}
254-
}
255223

256-
/// A handle for writing to the cache.
257-
/// Modifications are made directly to the shared cache under write lock.
258-
#[derive(Debug)]
259-
pub struct Writer {
260-
cache: Arc<RwLock<VersionedLru>>,
261-
}
262-
263-
impl Writer {
264224
/// Inserts or updates an entry in the cache.
265-
pub fn write(&mut self, snapshot_id: SnapshotId, key: Nibbles, value: Option<(PageId, u8)>) {
225+
pub fn write(&self, snapshot_id: SnapshotId, key: Nibbles, value: Option<(PageId, u8)>) {
266226
let mut guard = self.cache.write().unwrap();
267227
guard.set(key, snapshot_id, value);
268228
}
269229

270230
/// Removes an entry from the cache by inserting a None value.
271-
pub fn remove(&mut self, snapshot_id: SnapshotId, key: Nibbles) {
231+
pub fn remove(&self, snapshot_id: SnapshotId, key: Nibbles) {
272232
let mut guard = self.cache.write().unwrap();
273233
guard.set(key, snapshot_id, None);
274234
}
275235

276-
/// Gets a value and updates LRU state.
277-
pub fn get(&mut self, snapshot_id: SnapshotId, key: &Nibbles) -> Option<(PageId, u8)> {
236+
/// Sets the minimum snapshot ID for proactive cache purging.
237+
pub fn set_min_snapshot_id(&self, min_snapshot_id: SnapshotId) {
278238
let mut guard = self.cache.write().unwrap();
279-
guard.get(key, snapshot_id)
239+
guard.set_min_snapshot_id(min_snapshot_id);
280240
}
281241
}
282242

@@ -291,62 +251,64 @@ mod tests {
291251
let shared_cache = Arc::new(cache);
292252

293253
// first writer
294-
let mut writer1 = shared_cache.write();
295-
writer1.write(100, Nibbles::from_nibbles([1]), Some((PageId::new(10).unwrap(), 11)));
296-
writer1.write(100, Nibbles::from_nibbles([2]), Some((PageId::new(12).unwrap(), 13)));
297-
writer1.write(200, Nibbles::from_nibbles([1]), Some((PageId::new(20).unwrap(), 21)));
298-
drop(writer1);
254+
shared_cache.write(100, Nibbles::from_nibbles([1]), Some((PageId::new(10).unwrap(), 11)));
255+
shared_cache.write(100, Nibbles::from_nibbles([2]), Some((PageId::new(12).unwrap(), 13)));
256+
shared_cache.write(200, Nibbles::from_nibbles([1]), Some((PageId::new(20).unwrap(), 21)));
299257

300258
// have some concurrent readers
301259
let cache_reader1 = Arc::clone(&shared_cache);
302260
let reader1 = thread::spawn(move || {
303-
let reader = cache_reader1.read();
304-
let val1 = reader.get(100, &Nibbles::from_nibbles([1]));
305-
let val2 = reader.get(200, &Nibbles::from_nibbles([1]));
261+
let val1 = cache_reader1.get(100, &Nibbles::from_nibbles([1]));
262+
let val2 = cache_reader1.get(200, &Nibbles::from_nibbles([1]));
306263
assert_eq!(val1, Some((PageId::new(10).unwrap(), 11)));
307264
assert_eq!(val2, Some((PageId::new(20).unwrap(), 21)));
308265
thread::sleep(Duration::from_millis(50));
309266
});
310267

311268
let cache_reader2 = Arc::clone(&shared_cache);
312269
let reader2 = thread::spawn(move || {
313-
let reader = cache_reader2.read();
314-
let val = reader.get(100, &Nibbles::from_nibbles([2]));
270+
let val = cache_reader2.get(100, &Nibbles::from_nibbles([2]));
315271
assert_eq!(val, Some((PageId::new(12).unwrap(), 13)));
316272
thread::sleep(Duration::from_millis(100));
317273
});
318274

319275
// writer2 will be blocked until concurrent readers are done
320276
let cache_writer2 = Arc::clone(&shared_cache);
321277
let writer2 = thread::spawn(move || {
322-
let mut writer = cache_writer2.write();
323-
writer.write(101, Nibbles::from_nibbles([3]), Some((PageId::new(14).unwrap(), 15)));
324-
writer.write(300, Nibbles::from_nibbles([1]), Some((PageId::new(30).unwrap(), 31)));
278+
cache_writer2.write(
279+
101,
280+
Nibbles::from_nibbles([3]),
281+
Some((PageId::new(14).unwrap(), 15)),
282+
);
283+
cache_writer2.write(
284+
300,
285+
Nibbles::from_nibbles([1]),
286+
Some((PageId::new(30).unwrap(), 31)),
287+
);
325288
});
326289

327290
reader1.join().unwrap();
328291
reader2.join().unwrap();
329292
writer2.join().unwrap();
330293

331-
let final_reader = shared_cache.read();
332294
assert_eq!(
333-
final_reader.get(100, &Nibbles::from_nibbles([1])),
295+
shared_cache.get(100, &Nibbles::from_nibbles([1])),
334296
Some((PageId::new(10).unwrap(), 11))
335297
);
336298
assert_eq!(
337-
final_reader.get(100, &Nibbles::from_nibbles([2])),
299+
shared_cache.get(100, &Nibbles::from_nibbles([2])),
338300
Some((PageId::new(12).unwrap(), 13))
339301
);
340302
assert_eq!(
341-
final_reader.get(101, &Nibbles::from_nibbles([3])),
303+
shared_cache.get(101, &Nibbles::from_nibbles([3])),
342304
Some((PageId::new(14).unwrap(), 15))
343305
);
344306
assert_eq!(
345-
final_reader.get(200, &Nibbles::from_nibbles([1])),
307+
shared_cache.get(200, &Nibbles::from_nibbles([1])),
346308
Some((PageId::new(20).unwrap(), 21))
347309
);
348310
assert_eq!(
349-
final_reader.get(300, &Nibbles::from_nibbles([1])),
311+
shared_cache.get(300, &Nibbles::from_nibbles([1])),
350312
Some((PageId::new(30).unwrap(), 31))
351313
);
352314
}
@@ -356,41 +318,36 @@ mod tests {
356318
let cache = CacheManager::new(NonZeroUsize::new(10).unwrap());
357319
let shared_cache = Arc::new(cache);
358320

359-
let mut writer = shared_cache.write();
360-
writer.write(100, Nibbles::from_nibbles([1]), Some((PageId::new(10).unwrap(), 11)));
361-
writer.write(200, Nibbles::from_nibbles([1]), Some((PageId::new(20).unwrap(), 21)));
362-
writer.write(300, Nibbles::from_nibbles([1]), Some((PageId::new(30).unwrap(), 31)));
363-
drop(writer);
364-
365-
let reader = shared_cache.read();
321+
shared_cache.write(100, Nibbles::from_nibbles([1]), Some((PageId::new(10).unwrap(), 11)));
322+
shared_cache.write(200, Nibbles::from_nibbles([1]), Some((PageId::new(20).unwrap(), 21)));
323+
shared_cache.write(300, Nibbles::from_nibbles([1]), Some((PageId::new(30).unwrap(), 31)));
366324

367325
// given the exact same snapshot
368326
assert_eq!(
369-
reader.get(100, &Nibbles::from_nibbles([1])),
327+
shared_cache.get(100, &Nibbles::from_nibbles([1])),
370328
Some((PageId::new(10).unwrap(), 11))
371329
);
372330
assert_eq!(
373-
reader.get(200, &Nibbles::from_nibbles([1])),
331+
shared_cache.get(200, &Nibbles::from_nibbles([1])),
374332
Some((PageId::new(20).unwrap(), 21))
375333
);
376334
assert_eq!(
377-
reader.get(300, &Nibbles::from_nibbles([1])),
335+
shared_cache.get(300, &Nibbles::from_nibbles([1])),
378336
Some((PageId::new(30).unwrap(), 31))
379337
);
380338

381339
// given different snapshots, but it should find the latest version <= target snapshot
382340
assert_eq!(
383-
reader.get(150, &Nibbles::from_nibbles([1])),
341+
shared_cache.get(150, &Nibbles::from_nibbles([1])),
384342
Some((PageId::new(10).unwrap(), 11))
385343
);
386344
assert_eq!(
387-
reader.get(250, &Nibbles::from_nibbles([1])),
345+
shared_cache.get(250, &Nibbles::from_nibbles([1])),
388346
Some((PageId::new(20).unwrap(), 21))
389347
);
390348

391349
// given snapshot too small, since snapshot < earliest
392-
assert_eq!(reader.get(50, &Nibbles::from_nibbles([1])), None);
393-
drop(reader);
350+
assert_eq!(shared_cache.get(50, &Nibbles::from_nibbles([1])), None);
394351
}
395352

396353
#[test]
@@ -399,17 +356,13 @@ mod tests {
399356
let shared_cache = Arc::new(cache);
400357

401358
// insert a value
402-
let mut writer = shared_cache.write();
403-
writer.write(100, Nibbles::from_nibbles([1]), Some((PageId::new(10).unwrap(), 11)));
359+
shared_cache.write(100, Nibbles::from_nibbles([1]), Some((PageId::new(10).unwrap(), 11)));
404360

405361
// invalidate it
406-
writer.write(100, Nibbles::from_nibbles([1]), None);
407-
drop(writer);
362+
shared_cache.write(100, Nibbles::from_nibbles([1]), None);
408363

409364
// try reading it
410-
let reader = shared_cache.read();
411-
assert_eq!(reader.get(100, &Nibbles::from_nibbles([1])), None);
412-
drop(reader);
365+
assert_eq!(shared_cache.get(100, &Nibbles::from_nibbles([1])), None);
413366
}
414367

415368
#[test]
@@ -418,65 +371,58 @@ mod tests {
418371
let shared_cache = Arc::new(cache);
419372

420373
// insert entries with different snapshots
421-
let mut writer = shared_cache.write();
422-
writer.write(100, Nibbles::from_nibbles([1]), Some((PageId::new(10).unwrap(), 11)));
423-
writer.write(200, Nibbles::from_nibbles([1]), Some((PageId::new(20).unwrap(), 21)));
424-
writer.write(300, Nibbles::from_nibbles([1]), Some((PageId::new(30).unwrap(), 31)));
425-
drop(writer);
374+
shared_cache.write(100, Nibbles::from_nibbles([1]), Some((PageId::new(10).unwrap(), 11)));
375+
shared_cache.write(200, Nibbles::from_nibbles([1]), Some((PageId::new(20).unwrap(), 21)));
376+
shared_cache.write(300, Nibbles::from_nibbles([1]), Some((PageId::new(30).unwrap(), 31)));
426377

427378
// set minimum snapshot ID to 250
428379
shared_cache.set_min_snapshot_id(250);
429-
let reader = shared_cache.read();
430380

431381
// purged the entries below min snapshot
432-
assert_eq!(reader.get(100, &Nibbles::from_nibbles([1])), None);
433-
assert_eq!(reader.get(200, &Nibbles::from_nibbles([1])), None);
382+
assert_eq!(shared_cache.get(100, &Nibbles::from_nibbles([1])), None);
383+
assert_eq!(shared_cache.get(200, &Nibbles::from_nibbles([1])), None);
434384

435385
// only keep entries above min snapshot
436386
assert_eq!(
437-
reader.get(300, &Nibbles::from_nibbles([1])),
387+
shared_cache.get(300, &Nibbles::from_nibbles([1])),
438388
Some((PageId::new(30).unwrap(), 31))
439389
);
440-
drop(reader);
441390
}
442391

443392
#[test]
444393
fn test_oldest_sibling_eviction() {
445394
let cache = CacheManager::new(NonZeroUsize::new(4).unwrap());
446395
let shared_cache = Arc::new(cache);
447396

448-
let mut writer = shared_cache.write();
449397
// multiple versions of key [1]
450-
writer.write(100, Nibbles::from_nibbles([1]), Some((PageId::new(10).unwrap(), 11)));
451-
writer.write(200, Nibbles::from_nibbles([1]), Some((PageId::new(20).unwrap(), 21)));
452-
writer.write(300, Nibbles::from_nibbles([1]), Some((PageId::new(30).unwrap(), 31)));
398+
shared_cache.write(100, Nibbles::from_nibbles([1]), Some((PageId::new(10).unwrap(), 11)));
399+
shared_cache.write(200, Nibbles::from_nibbles([1]), Some((PageId::new(20).unwrap(), 21)));
400+
shared_cache.write(300, Nibbles::from_nibbles([1]), Some((PageId::new(30).unwrap(), 31)));
453401

454402
// one entry for key [2]
455-
writer.write(150, Nibbles::from_nibbles([2]), Some((PageId::new(15).unwrap(), 16)));
403+
shared_cache.write(150, Nibbles::from_nibbles([2]), Some((PageId::new(15).unwrap(), 16)));
456404

457405
// since the cache is full, should evict oldest sibling of tail entry
458-
writer.write(400, Nibbles::from_nibbles([3]), Some((PageId::new(40).unwrap(), 41)));
459-
drop(writer);
406+
shared_cache.write(400, Nibbles::from_nibbles([3]), Some((PageId::new(40).unwrap(), 41)));
460407

461-
let reader = shared_cache.read();
462408
// the oldest sibling (snapshot 100) should be evicted, NOT the tail entry
463-
assert_eq!(reader.get(100, &Nibbles::from_nibbles([1])), None);
409+
assert_eq!(shared_cache.get(100, &Nibbles::from_nibbles([1])), None);
464410

465411
// test the rest should exist
466412
assert_eq!(
467-
reader.get(200, &Nibbles::from_nibbles([1])),
413+
shared_cache.get(200, &Nibbles::from_nibbles([1])),
468414
Some((PageId::new(20).unwrap(), 21))
469415
);
470416
assert_eq!(
471-
reader.get(300, &Nibbles::from_nibbles([1])),
417+
shared_cache.get(300, &Nibbles::from_nibbles([1])),
472418
Some((PageId::new(30).unwrap(), 31))
473419
);
474420
assert_eq!(
475-
reader.get(150, &Nibbles::from_nibbles([2])),
421+
shared_cache.get(150, &Nibbles::from_nibbles([2])),
476422
Some((PageId::new(15).unwrap(), 16))
477423
);
478424
assert_eq!(
479-
reader.get(400, &Nibbles::from_nibbles([3])),
425+
shared_cache.get(400, &Nibbles::from_nibbles([3])),
480426
Some((PageId::new(40).unwrap(), 41))
481427
);
482428
}

0 commit comments

Comments
 (0)