another attempt to reduce allocations, don't know if helpers, certainly didn't eliminate all of them

This commit is contained in:
Christian Schwarz
2024-01-30 14:10:20 +00:00
parent f3e1ae6740
commit 1daeba6d87
8 changed files with 94 additions and 54 deletions

View File

@@ -161,6 +161,14 @@ pub trait BeSer {
be_coder().deserialize_from(r).map_err(|e| e.into())
}
/// Deserialize from a reader
fn des_from_custom<'s, R: bincode::BincodeRead<'s>>(r: R) -> Result<Self, DeserializeError>
where
Self: DeserializeOwned,
{
be_coder().deserialize_from_custom(r).map_err(|e| e.into())
}
/// Compute the serialized size of a data structure
///
/// Note: it may be faster to serialize to a buffer and then measure the

View File

@@ -21,8 +21,8 @@ pub enum Value {
}
#[derive(Deserialize)]
pub enum ValueDe<'a> {
Image(&'a [u8]),
pub enum ValueDe {
Image(Vec<u8>),
WalRecord(NeonWalRecord),
}

View File

@@ -52,6 +52,20 @@ impl<const N: usize> VecIsh for smallvec::SmallVec<[u8; N]> {
}
}
impl VecIsh for bytes::BytesMut {
fn clear(&mut self) {
bytes::BytesMut::clear(self)
}
fn reserve(&mut self, len: usize) {
bytes::BytesMut::reserve(self, len)
}
fn extend_from_slice(&mut self, o: &[u8]) {
bytes::BytesMut::extend_from_slice(self, o)
}
}
impl<'a> BlockCursor<'a> {
/// Read a blob into a new buffer.
pub async fn read_blob(

View File

@@ -10,7 +10,7 @@ mod layer_desc;
use crate::context::{AccessStatsBehavior, RequestContext};
use crate::task_mgr::TaskKind;
use crate::walrecord::NeonWalRecord;
use bytes::Bytes;
use bytes::{Bytes, BytesMut};
use enum_map::EnumMap;
use enumset::EnumSet;
use once_cell::sync::Lazy;
@@ -66,14 +66,14 @@ where
#[derive(Debug)]
pub struct ValueReconstructState {
pub records: smallvec::SmallVec<[(Lsn, NeonWalRecord); 300]>,
pub img: Option<(Lsn, Range<usize>)>,
pub(crate) scratch: smallvec::SmallVec<[u8; 2 * PAGE_SZ]>,
pub img: Option<(Lsn, Vec<u8>)>,
pub(crate) scratch: BytesMut,
}
impl ValueReconstructState {
pub(crate) fn img_ref(&self) -> Option<(Lsn, &[u8])> {
match &self.img {
Some((lsn, range_in_scratch)) => Some((*lsn, &self.scratch[range_in_scratch.clone()])),
Some((lsn, img)) => Some((*lsn, &img)),
None => None,
}
}

View File

@@ -40,6 +40,7 @@ use crate::virtual_file::{self, VirtualFile};
use crate::{walrecord, TEMP_FILE_SUFFIX};
use crate::{DELTA_FILE_MAGIC, STORAGE_FORMAT_VERSION};
use anyhow::{bail, ensure, Context, Result};
use bytes::{Buf, BytesMut};
use camino::{Utf8Path, Utf8PathBuf};
use pageserver_api::models::LayerAccessKind;
use pageserver_api::shard::TenantShardId;
@@ -747,7 +748,7 @@ impl DeltaLayerInner {
);
let search_key = DeltaKey::from_key_lsn(&key, Lsn(lsn_range.end.0 - 1));
let mut offsets = smallvec::SmallVec::<[(Lsn, u64); 4]>::new();
let mut offsets = smallvec::SmallVec::<[(Lsn, u64); 10]>::new();
tree_reader
.visit(
@@ -789,7 +790,57 @@ impl DeltaLayerInner {
// That's on avg 200 allocations.
// Can we re-use the Vec from a buffer pool?
let val = ValueDe::des(&reconstruct_state.scratch).with_context(|| {
struct MoveVecBincodeRead {
buf: bytes::BytesMut,
}
impl std::io::Read for MoveVecBincodeRead {
fn read(&mut self, mut buf: &mut [u8]) -> std::io::Result<usize> {
let prefix = self.buf.split_to(buf.len()); // FIXME: handle buf < len
buf.copy_from_slice(&prefix);
Ok(buf.len())
}
}
impl<'storage> bincode::BincodeRead<'storage> for MoveVecBincodeRead {
#[inline(always)]
fn forward_read_str<V>(
&mut self,
length: usize,
visitor: V,
) -> bincode::Result<V::Value>
where
V: serde::de::Visitor<'storage>,
{
todo!()
}
fn get_byte_buffer(&mut self, length: usize) -> bincode::Result<Vec<u8>> {
let data = if self.buf.len() == length {
// ensure `data` is uniquely owned so Vec::from below is fast
std::mem::take(&mut self.buf)
} else {
self.buf.split_to(length)
};
Ok(Vec::<u8>::from(data))
}
fn forward_read_bytes<V>(
&mut self,
length: usize,
visitor: V,
) -> bincode::Result<V::Value>
where
V: serde::de::Visitor<'storage>,
{
todo!()
}
}
let val = ValueDe::des_from_custom(MoveVecBincodeRead {
buf: std::mem::take(&mut reconstruct_state.scratch),
})
.with_context(|| {
format!(
"Failed to deserialize file blob from virtual file {}",
file.file.path
@@ -797,24 +848,7 @@ impl DeltaLayerInner {
})?;
match val {
ValueDe::Image(img) => {
let range_in_scratch = {
// TODO, we should just make .img reference .scratch, but that's Pin pain
assert!(
reconstruct_state.scratch.as_ptr_range().start
>= img.as_ptr_range().start
);
assert!(
reconstruct_state.scratch.as_ptr_range().end <= img.as_ptr_range().end
);
// SAFETY: TODO; probably technicall some UB in here; we checked same bounds in above assert,
// but other criteria don't hold. Probably fine though.
let start =
unsafe { img.as_ptr().offset_from(reconstruct_state.scratch.as_ptr()) };
assert!(start >= 0);
let start = usize::try_from(start).unwrap();
start..(start.checked_add(img.len()).unwrap())
};
reconstruct_state.img = Some((entry_lsn, range_in_scratch));
reconstruct_state.img = Some((entry_lsn, img));
need_image = false;
break;
}

View File

@@ -445,7 +445,10 @@ impl ImageLayerInner {
)
.await
.with_context(|| format!("failed to read value from offset {}", offset))?;
reconstruct_state.img = Some((self.lsn, 0..reconstruct_state.scratch.len()));
reconstruct_state.img = Some((
self.lsn,
Vec::<u8>::from(std::mem::take(&mut reconstruct_state.scratch)),
));
Ok(ValueReconstructResult::Complete)
} else {
Ok(ValueReconstructResult::Missing)

View File

@@ -173,36 +173,17 @@ impl InMemoryLayer {
let slice = vec_map.slice_range(lsn_range);
for (entry_lsn, pos) in slice.iter().rev() {
reconstruct_state.scratch.clear();
let buf = reader
reader
.read_blob_into_buf(*pos, &mut reconstruct_state.scratch, &ctx)
.await?;
let value = ValueDe::des(&reconstruct_state.scratch)?;
// TODO: avoid creating the bincode::SliceReader copy
let value = Value::des(&reconstruct_state.scratch)?;
match value {
ValueDe::Image(img) => {
let range_in_scratch = {
// TODO, we should just make .img reference .scratch, but that's Pin pain
assert!(
reconstruct_state.scratch.as_ptr_range().start
>= img.as_ptr_range().start
);
assert!(
reconstruct_state.scratch.as_ptr_range().end
<= img.as_ptr_range().end
);
// SAFETY: TODO; probably technicall some UB in here; we checked same bounds in above assert,
// but other criteria don't hold. Probably fine though.
let start = unsafe {
img.as_ptr().offset_from(reconstruct_state.scratch.as_ptr())
};
assert!(start >= 0);
let start = usize::try_from(start).unwrap();
start..(start.checked_add(img.len()).unwrap())
};
reconstruct_state.img = Some((*entry_lsn, range_in_scratch));
Value::Image(img) => {
reconstruct_state.img = Some((*entry_lsn, img.to_vec()));
return Ok(ValueReconstructResult::Complete);
}
ValueDe::WalRecord(rec) => {
Value::WalRecord(rec) => {
let will_init = rec.will_init();
reconstruct_state.records.push((*entry_lsn, rec));
if will_init {

View File

@@ -1,6 +1,6 @@
use std::cell::RefCell;
use crate::tenant::storage_layer::ValueReconstructState;
use crate::{page_cache::PAGE_SZ, tenant::storage_layer::ValueReconstructState};
struct Content(ValueReconstructState);
@@ -9,7 +9,7 @@ impl Content {
Content(ValueReconstructState {
records: smallvec::SmallVec::new(),
img: None,
scratch: smallvec::SmallVec::new(),
scratch: bytes::BytesMut::with_capacity(2 * PAGE_SZ),
})
}
fn reset(&mut self) {