cache: refactor cache structure and reduce allocations

This commit reworks the cache data structures and operations to
significantly cut down on unnecessary allocations and improve overall
cache management logic.

Key changes include:

- Replacing event-specific CacheEntry types with a generic CacheEntry<T>
  that stores values and timestamps.

- Removing the old Arc references in HashMap entries, reducing
  reference-counted pointer overhead.

- Providing helper methods (new(), maybe(), empty(), value()) on
  CacheEntry for more ergonomic and explicit value handling.

- Simplifying the retrieval functions (get_mute_list, get_relay_list,
  get_contact_list) by using a shared helper (get_cache_entry) that
  checks for expiration and removes stale entries automatically.

- Streamlining add_event and related cache insertion methods to insert
  the correct CacheEntry variant depending on the event kind.

- Removing unnecessary methods (e.g., references_pubkey) and simplifying
  extensions in nostr_event_extensions.rs.

- Enhancing code clarity by separating expiration logic from retrieval
  and insertion, making the cache easier to maintain, debug, and extend.

These improvements should reduce runtime memory overhead and increase
code clarity, while maintaining the existing external behavior of the
cache. Future changes will be easier to implement, as the new generic
CacheEntry abstraction and simplified code paths provide a more
maintainable foundation.

Signed-off-by: William Casarin <jb55@jb55.com>
This commit is contained in:
William Casarin
2024-12-06 14:36:39 -08:00
committed by Daniel D’Aquino
parent 88939ba2d1
commit e3c9e2834e
3 changed files with 74 additions and 111 deletions

View File

@ -2,36 +2,75 @@ use crate::{notification_manager::nostr_event_extensions::MaybeConvertibleToTime
use tokio::time::Duration;
use nostr_sdk::prelude::*;
use std::collections::HashMap;
use std::sync::Arc;
use log;
use super::nostr_event_extensions::{MaybeConvertibleToRelayList, RelayList, TimestampedMuteList};
struct CacheEntry {
event: Option<Event>, // `None` means the event does not exist as far as we know (It does NOT mean expired)
struct CacheEntry<T> {
value: Option<T>, // `None` means the event does not exist as far as we know (It does NOT mean expired)
added_at: nostr::Timestamp,
}
impl CacheEntry {
impl<T> CacheEntry<T> {
fn is_expired(&self, max_age: Duration) -> bool {
let time_delta = TimeDelta::subtracting(nostr::Timestamp::now(), self.added_at);
time_delta.negative || (time_delta.delta_abs_seconds > max_age.as_secs())
}
}
impl<T> CacheEntry<T> {
pub fn new(value: T) -> Self {
let added_at = nostr::Timestamp::now();
CacheEntry { value: Some(value), added_at }
}
pub fn maybe(value: Option<T>) -> Self {
let added_at = nostr::Timestamp::now();
CacheEntry { value, added_at }
}
pub fn empty() -> Self {
let added_at = nostr::Timestamp::now();
CacheEntry { value: None, added_at }
}
pub fn value(&self) -> Option<&T> {
self.value.as_ref()
}
}
pub struct Cache {
entries: HashMap<EventId, Arc<CacheEntry>>,
mute_lists: HashMap<PublicKey, Arc<CacheEntry>>,
contact_lists: HashMap<PublicKey, Arc<CacheEntry>>,
relay_lists: HashMap<PublicKey, Arc<CacheEntry>>,
//entries: HashMap<EventId, Event>,
mute_lists: HashMap<PublicKey, CacheEntry<TimestampedMuteList>>,
contact_lists: HashMap<PublicKey, CacheEntry<Event>>,
relay_lists: HashMap<PublicKey, CacheEntry<RelayList>>,
max_age: Duration,
}
fn get_cache_entry<T: Clone>(list: &mut HashMap<PublicKey, CacheEntry<T>>, pubkey: &PublicKey, max_age: Duration, name: &str) -> Result<Option<T>, CacheError> {
let res = if let Some(entry) = list.get(pubkey) {
if !entry.is_expired(max_age) {
Ok(entry.value().cloned())
} else {
log::debug!("{} list for pubkey {} is expired, removing it from the cache", name, pubkey.to_hex());
Err(CacheError::Expired)
}
} else {
Err(CacheError::NotFound)
};
if let Err(CacheError::Expired) = &res {
list.remove(pubkey);
}
res
}
impl Cache {
// MARK: - Initialization
pub fn new(max_age: Duration) -> Self {
Cache {
entries: HashMap::new(),
//entries: HashMap::new(),
mute_lists: HashMap::new(),
contact_lists: HashMap::new(),
relay_lists: HashMap::new(),
@ -41,68 +80,52 @@ impl Cache {
// MARK: - Adding items to the cache
pub fn add_optional_mute_list_with_author(&mut self, author: &PublicKey, mute_list: Option<Event>) {
pub fn add_optional_mute_list_with_author<'a>(&'a mut self, author: &PublicKey, mute_list: Option<&Event>) {
if let Some(mute_list) = mute_list {
self.add_event(mute_list);
} else {
self.mute_lists.insert(
author.clone(),
Arc::new(CacheEntry {
event: None,
added_at: nostr::Timestamp::now(),
}),
author.to_owned(),
CacheEntry::empty(),
);
}
}
pub fn add_optional_relay_list_with_author(&mut self, author: &PublicKey, relay_list_event: Option<Event>) {
pub fn add_optional_relay_list_with_author<'a>(&'a mut self, author: &PublicKey, relay_list_event: Option<&Event>) {
if let Some(relay_list_event) = relay_list_event {
self.add_event(relay_list_event);
} else {
self.relay_lists.insert(
author.clone(),
Arc::new(CacheEntry {
event: None,
added_at: nostr::Timestamp::now(),
}),
author.to_owned(),
CacheEntry::empty(),
);
}
}
pub fn add_optional_contact_list_with_author(&mut self, author: &PublicKey, contact_list: Option<Event>) {
pub fn add_optional_contact_list_with_author<'a>(&'a mut self, author: &PublicKey, contact_list: Option<&Event>) {
if let Some(contact_list) = contact_list {
self.add_event(contact_list);
} else {
self.contact_lists.insert(
author.clone(),
Arc::new(CacheEntry {
event: None,
added_at: nostr::Timestamp::now(),
}),
author.to_owned(),
CacheEntry::empty(),
);
}
}
pub fn add_event(&mut self, event: Event) {
let entry = Arc::new(CacheEntry {
event: Some(event.clone()),
added_at: nostr::Timestamp::now(),
});
self.entries.insert(event.id.clone(), entry.clone());
pub fn add_event(&mut self, event: &Event) {
match event.kind {
Kind::MuteList => {
self.mute_lists.insert(event.pubkey.clone(), entry.clone());
self.mute_lists.insert(event.pubkey.clone(), CacheEntry::maybe(event.to_timestamped_mute_list()));
log::debug!("Added mute list to the cache. Event ID: {}", event.id.to_hex());
}
Kind::ContactList => {
self.contact_lists
.insert(event.pubkey.clone(), entry.clone());
log::debug!("Added contact list to the cache. Event ID: {}", event.id.to_hex());
self.contact_lists.insert(event.pubkey.clone(), CacheEntry::new(event.to_owned()));
},
Kind::RelayList => {
self.relay_lists.insert(event.pubkey.clone(), entry.clone());
log::debug!("Added relay list to the cache. Event ID: {}", event.id.to_hex());
self.relay_lists.insert(event.pubkey.clone(), CacheEntry::maybe(event.to_relay_list()));
},
_ => {
log::debug!("Unknown event kind, not adding to any cache. Event ID: {}", event.id.to_hex());
@ -113,70 +136,15 @@ impl Cache {
// MARK: - Fetching items from the cache
pub fn get_mute_list(&mut self, pubkey: &PublicKey) -> Result<Option<TimestampedMuteList>, CacheError> {
if let Some(entry) = self.mute_lists.get(pubkey) {
let entry = entry.clone(); // Clone the Arc to avoid borrowing issues
if !entry.is_expired(self.max_age) {
match &entry.event {
Some(event) => {
log::debug!("Cached mute list for pubkey {} was found", pubkey.to_hex());
return Ok(event.to_timestamped_mute_list());
}
None => {
log::debug!("Empty mute list cache entry for pubkey {}", pubkey.to_hex());
return Ok(None);
}
}
} else {
log::debug!("Mute list for pubkey {} is expired, removing it from the cache", pubkey.to_hex());
self.mute_lists.remove(pubkey);
self.remove_event_from_all_maps(&entry.event);
}
}
log::debug!("Mute list for pubkey {} not found on cache", pubkey.to_hex());
Err(CacheError::NotFound)
get_cache_entry(&mut self.mute_lists, pubkey, self.max_age, "Mute")
}
pub fn get_relay_list(&mut self, pubkey: &PublicKey) -> Result<Option<RelayList>, CacheError> {
if let Some(entry) = self.relay_lists.get(pubkey) {
let entry = entry.clone(); // Clone the Arc to avoid borrowing issues
if !entry.is_expired(self.max_age) {
if let Some(event) = entry.event.clone() {
return Ok(event.to_relay_list());
}
} else {
log::debug!("Relay list for pubkey {} is expired, removing it from the cache", pubkey.to_hex());
self.mute_lists.remove(pubkey);
self.remove_event_from_all_maps(&entry.event);
}
}
Err(CacheError::NotFound)
get_cache_entry(&mut self.relay_lists, pubkey, self.max_age, "Relay")
}
pub fn get_contact_list(&mut self, pubkey: &PublicKey) -> Result<Option<Event>, CacheError> {
if let Some(entry) = self.contact_lists.get(pubkey) {
let entry = entry.clone(); // Clone the Arc to avoid borrowing issues
if !entry.is_expired(self.max_age) {
return Ok(entry.event.clone());
} else {
log::debug!("Contact list for pubkey {} is expired, removing it from the cache", pubkey.to_hex());
self.contact_lists.remove(pubkey);
self.remove_event_from_all_maps(&entry.event);
}
}
Err(CacheError::NotFound)
}
// MARK: - Removing items from the cache
fn remove_event_from_all_maps(&mut self, event: &Option<Event>) {
if let Some(event) = event {
let event_id = event.id.clone();
let pubkey = event.pubkey.clone();
self.entries.remove(&event_id);
self.mute_lists.remove(&pubkey);
self.contact_lists.remove(&pubkey);
}
// We can't remove an event from all maps if the event does not exist
get_cache_entry(&mut self.contact_lists, pubkey, self.max_age, "Contact")
}
}
@ -184,4 +152,5 @@ impl Cache {
#[derive(Debug)]
pub enum CacheError {
NotFound,
Expired,
}

View File

@ -3,9 +3,6 @@ use nostr_sdk::{EventId, Kind, TagKind};
/// Temporary scaffolding of old methods that have not been ported to use native Event methods
pub trait ExtendedEvent {
/// Checks if the note references a given pubkey
fn references_pubkey(&self, pubkey: &PublicKey) -> bool;
/// Retrieves a set of pubkeys referenced by the note
fn referenced_pubkeys(&self) -> std::collections::HashSet<nostr::PublicKey>;
@ -21,11 +18,6 @@ pub trait ExtendedEvent {
// This is a wrapper around the Event type from strfry-policies, which adds some useful methods
impl ExtendedEvent for nostr::Event {
/// Checks if the note references a given pubkey
fn references_pubkey(&self, pubkey: &PublicKey) -> bool {
self.referenced_pubkeys().contains(pubkey)
}
/// Retrieves a set of pubkeys referenced by the note
fn referenced_pubkeys(&self) -> std::collections::HashSet<nostr::PublicKey> {
self.get_tags_content(SingleLetter(SingleLetterTag::lowercase(Alphabet::P)))
@ -149,6 +141,7 @@ impl MaybeConvertibleToRelayList for nostr::Event {
let extracted_relay_list_owned = extracted_relay_list.into_iter()
.map(|(url, metadata)| (url.clone(), metadata.as_ref().map(|m| m.clone())))
.collect();
Some(extracted_relay_list_owned)
}
}
@ -216,6 +209,7 @@ impl Codable for MuteList {
}
}
#[derive(Clone)]
pub struct TimestampedMuteList {
pub mute_list: MuteList,
pub timestamp: nostr::Timestamp,

View File

@ -1,5 +1,5 @@
use tokio::sync::Mutex;
use super::nostr_event_extensions::{MaybeConvertibleToRelayList, MaybeConvertibleToTimestampedMuteList, RelayList, TimestampedMuteList};
use super::nostr_event_extensions::{RelayList, TimestampedMuteList};
use super::notification_manager::EventSaver;
use super::ExtendedEvent;
use nostr_sdk::prelude::*;
@ -59,8 +59,8 @@ impl NostrNetworkHelper {
// We don't have an answer from the cache, so we need to fetch it
let mute_list_event = self.fetch_single_event(pubkey, Kind::MuteList).await;
let mut cache_mutex_guard = self.cache.lock().await;
cache_mutex_guard.add_optional_mute_list_with_author(pubkey, mute_list_event.clone());
Some(mute_list_event?.to_timestamped_mute_list()?)
cache_mutex_guard.add_optional_mute_list_with_author(pubkey, mute_list_event.as_ref());
cache_mutex_guard.get_mute_list(pubkey).ok()?
}
pub async fn get_relay_list(&self, pubkey: &PublicKey) -> Option<RelayList> {
@ -74,8 +74,8 @@ impl NostrNetworkHelper {
// We don't have an answer from the cache, so we need to fetch it
let relay_list_event = NostrNetworkHelper::fetch_single_event_from_client(pubkey, Kind::RelayList, &self.bootstrap_client).await;
let mut cache_mutex_guard = self.cache.lock().await;
cache_mutex_guard.add_optional_relay_list_with_author(pubkey, relay_list_event.clone());
relay_list_event?.to_relay_list()
cache_mutex_guard.add_optional_relay_list_with_author(pubkey, relay_list_event.as_ref());
cache_mutex_guard.get_relay_list(pubkey).ok()?
}
pub async fn get_contact_list(&self, pubkey: &PublicKey) -> Option<Event> {
@ -89,8 +89,8 @@ impl NostrNetworkHelper {
// We don't have an answer from the cache, so we need to fetch it
let contact_list_event = self.fetch_single_event(pubkey, Kind::ContactList).await;
let mut cache_mutex_guard = self.cache.lock().await;
cache_mutex_guard.add_optional_contact_list_with_author(pubkey, contact_list_event.clone());
contact_list_event
cache_mutex_guard.add_optional_contact_list_with_author(pubkey, contact_list_event.as_ref());
cache_mutex_guard.get_contact_list(pubkey).ok()?
}
// MARK: - Lower level fetching functions