From 72d35a3b3239a470cfd7830d8e654f1bdefbee05 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Thu, 2 Feb 2023 12:17:40 +0100 Subject: [PATCH] use heapless::HistoryBuffer instead of VecDeque --- Cargo.lock | 54 +++++++++++++++++++++++++- Cargo.toml | 1 + pageserver/Cargo.toml | 1 + pageserver/src/tenant/storage_layer.rs | 42 +++++++------------- 4 files changed, 69 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c649d2504e..e7ea75fe8d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -141,6 +141,15 @@ dependencies = [ "syn", ] +[[package]] +name = "atomic-polyfill" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3ff7eb3f316534d83a8a2c3d1674ace8a5a71198eba31e2e2b597833f699b28" +dependencies = [ + "critical-section", +] + [[package]] name = "atty" version = "0.2.14" @@ -986,6 +995,12 @@ dependencies = [ "itertools", ] +[[package]] +name = "critical-section" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6548a0ad5d2549e111e1f6a11a6c2e2d00ce6a3dafe22948d67c2b443f775e52" + [[package]] name = "crossbeam-channel" version = "0.5.6" @@ -1551,6 +1566,15 @@ version = "1.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eabb4a44450da02c90444cf74558da904edde8fb4e9035a9a6a4e15445af0bd7" +[[package]] +name = "hash32" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47d60b12902ba28e2730cd37e95b8c9223af2808df9e902d4df49588d1470606" +dependencies = [ + "byteorder", +] + [[package]] name = "hashbrown" version = "0.12.3" @@ -1566,6 +1590,18 @@ dependencies = [ "ahash", ] +[[package]] +name = "heapless" +version = "0.8.0" +source = "git+https://github.com/japaric/heapless.git?rev=340e0cec99a73759a5704f726c2fa3eed8fc3af6#340e0cec99a73759a5704f726c2fa3eed8fc3af6" +dependencies = [ + "atomic-polyfill", + "hash32", + "rustc_version", + "spin 0.9.4", + "stable_deref_trait", +] + [[package]] name = "heck" version = "0.4.0" @@ -2366,6 +2402,7 @@ dependencies = [ "fail", "futures", "git-version", + "heapless", "hex", "hex-literal", "humantime", @@ -3042,7 +3079,7 @@ dependencies = [ "cc", "libc", "once_cell", - "spin", + "spin 0.5.2", "untrusted", "web-sys", "winapi", @@ -3589,6 +3626,21 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" +[[package]] +name = "spin" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f6002a767bff9e83f8eeecf883ecb8011875a21ae8da43bffb817a57e78cc09" +dependencies = [ + "lock_api", +] + +[[package]] +name = "stable_deref_trait" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" + [[package]] name = "static_assertions" version = "1.1.0" diff --git a/Cargo.toml b/Cargo.toml index d1c0138104..6538771fd7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,6 +46,7 @@ futures-core = "0.3" futures-util = "0.3" git-version = "0.3" hashbrown = "0.13" +heapless = { default-features=false, features=[], git = "https://github.com/japaric/heapless.git", rev = "340e0cec99a73759a5704f726c2fa3eed8fc3af6" } # upstream release pending hex = "0.4" hex-literal = "0.3" hmac = "0.12.1" diff --git a/pageserver/Cargo.toml b/pageserver/Cargo.toml index f3ad2c5de6..9b0f624e9f 100644 --- a/pageserver/Cargo.toml +++ b/pageserver/Cargo.toml @@ -71,6 +71,7 @@ enum-map.workspace = true enumset.workspace = true strum.workspace = true strum_macros.workspace = true +heapless.workspace = true [dev-dependencies] criterion.workspace = true diff --git a/pageserver/src/tenant/storage_layer.rs b/pageserver/src/tenant/storage_layer.rs index d40e87114a..e155303e57 100644 --- a/pageserver/src/tenant/storage_layer.rs +++ b/pageserver/src/tenant/storage_layer.rs @@ -13,9 +13,9 @@ use anyhow::Result; use bytes::Bytes; use enum_map::EnumMap; use enumset::EnumSet; +use heapless::HistoryBuffer; use pageserver_api::models::LayerAccessKind; use pageserver_api::models::{HistoricLayerInfo, Key}; -use std::collections::VecDeque; use std::ops::Range; use std::path::PathBuf; use std::sync::atomic::{self, AtomicBool}; @@ -91,20 +91,20 @@ pub enum ValueReconstructResult { #[derive(Debug)] pub struct LayerAccessStats(Mutex); -#[derive(Debug, Clone)] -struct LayerAccessStatFullDetails { - when: SystemTime, - task_kind: TaskKind, - access_kind: LayerAccessKind, -} - #[derive(Debug, Clone)] struct LayerAccessStatsInner { first_access: Option, count_by_access_kind: EnumMap, task_kind_flag: EnumSet, - last_accesses: VecDeque, - last_residence_changes: VecDeque, + last_accesses: HistoryBuffer, + last_residence_changes: HistoryBuffer, +} + +#[derive(Debug, Clone)] +struct LayerAccessStatFullDetails { + when: SystemTime, + task_kind: TaskKind, + access_kind: LayerAccessKind, } #[derive(Debug, Clone)] @@ -200,18 +200,13 @@ impl Default for LayerAccessStatsInner { first_access: None, count_by_access_kind: EnumMap::default(), task_kind_flag: EnumSet::default(), - last_accesses: VecDeque::with_capacity(LayerAccessStats::LAST_ACCESSES_MAX_LEN), - last_residence_changes: VecDeque::with_capacity( - LayerAccessStats::LAST_RESIDENCE_CHANGES_MAX_LEN, - ), + last_accesses: HistoryBuffer::default(), + last_residence_changes: HistoryBuffer::default(), } } } impl LayerAccessStats { - const LAST_ACCESSES_MAX_LEN: usize = 16; - const LAST_RESIDENCE_CHANGES_MAX_LEN: usize = 16; - pub(crate) fn for_loading_layer(residence_status: LayerResidenceStatus) -> Self { let new = LayerAccessStats(Mutex::new(LayerAccessStatsInner::default())); new.record_residence_change(residence_status); @@ -244,12 +239,7 @@ impl LayerAccessStats { return; } let mut inner = self.0.lock().unwrap(); - - // make room first to avoid reallocs - while inner.last_residence_changes.len() >= Self::LAST_RESIDENCE_CHANGES_MAX_LEN { - inner.last_residence_changes.pop_back(); - } - inner.last_residence_changes.push_front(new_status); + inner.last_residence_changes.write(new_status); } fn record_access(&self, access_kind: LayerAccessKind, task_kind: TaskKind) { @@ -267,11 +257,7 @@ impl LayerAccessStats { .get_or_insert_with(|| this_access.clone()); inner.count_by_access_kind[access_kind] += 1; inner.task_kind_flag |= task_kind; - // make room first to avoid reallocs - while inner.last_accesses.len() >= Self::LAST_ACCESSES_MAX_LEN { - inner.last_accesses.pop_back(); - } - inner.last_accesses.push_front(this_access); + inner.last_accesses.write(this_access); } fn reset(&self, what: LayerAccessStatsReset) { let mut inner = self.0.lock().unwrap();