Skip to content

Commit de8ba75

Browse files
authored
Use a generic read_context_slots_continguous_inner to avoid reallocation (#1374)
Right now, we read context slots (getting a `Vec<Option<DownstairsBlockContext>>`), then have to do a second allocation to get a `Vec<Option<BlockContext>>`. This PR makes `read_context_slots_contiguous` generic across a `F: Fn(Option<OnDiskDownstairsBlockContext>, u64) -> T`, so we can do that extraction in-place and only allocate one `Vec<T>`.
1 parent e10f879 commit de8ba75

File tree

1 file changed

+66
-18
lines changed

1 file changed

+66
-18
lines changed

downstairs/src/extent_inner_raw.rs

+66-18
Original file line numberDiff line numberDiff line change
@@ -319,18 +319,15 @@ impl ExtentInner for RawInner {
319319
cdt::extent__read__get__contexts__start!(|| {
320320
(job_id.0, self.extent_number.0, num_blocks)
321321
});
322-
let block_contexts =
323-
self.get_block_contexts(req.offset.value, num_blocks)?;
322+
let blocks = self.get_block_contexts_inner(
323+
req.offset.value,
324+
num_blocks,
325+
|ctx, _block| ctx.map(|c| c.block_context),
326+
)?;
324327
cdt::extent__read__get__contexts__done!(|| {
325328
(job_id.0, self.extent_number.0, num_blocks)
326329
});
327330

328-
// Convert from DownstairsBlockContext -> BlockContext
329-
let blocks = block_contexts
330-
.into_iter()
331-
.map(|b| b.map(|b| b.block_context))
332-
.collect();
333-
334331
// To avoid a `memset`, we're reading directly into uninitialized
335332
// memory in the buffer. This is fine; we sized the buffer
336333
// appropriately in advance (and will panic here if we messed up).
@@ -883,7 +880,26 @@ impl RawInner {
883880
block: u64,
884881
count: u64,
885882
) -> Result<Vec<Option<DownstairsBlockContext>>, CrucibleError> {
886-
let mut out = vec![];
883+
self.get_block_contexts_inner(block, count, |ctx, block| {
884+
ctx.map(|c| DownstairsBlockContext {
885+
block,
886+
block_context: c.block_context,
887+
on_disk_hash: c.on_disk_hash,
888+
})
889+
})
890+
}
891+
892+
/// Maps a function across block contexts, return a `Vec<T>`
893+
fn get_block_contexts_inner<F, T>(
894+
&mut self,
895+
block: u64,
896+
count: u64,
897+
f: F,
898+
) -> Result<Vec<T>, CrucibleError>
899+
where
900+
F: Fn(Option<OnDiskDownstairsBlockContext>, u64) -> T,
901+
{
902+
let mut out = Vec::with_capacity(count as usize);
887903
let mut reads = 0u64;
888904
for (slot, group) in (block..block + count)
889905
.group_by(|block| self.active_context[*block as usize])
@@ -892,12 +908,14 @@ impl RawInner {
892908
let mut group = group.peekable();
893909
let start = *group.peek().unwrap();
894910
let count = group.count();
895-
out.extend(self.layout.read_context_slots_contiguous(
911+
self.layout.read_context_slots_contiguous_inner(
896912
&self.file,
897913
start,
898914
count as u64,
899915
slot,
900-
)?);
916+
&f,
917+
&mut out,
918+
)?;
901919
reads += 1;
902920
}
903921
if let Some(reads) = reads.checked_sub(1) {
@@ -1205,6 +1223,40 @@ impl RawLayout {
12051223
block_count: u64,
12061224
slot: ContextSlot,
12071225
) -> Result<Vec<Option<DownstairsBlockContext>>, CrucibleError> {
1226+
let mut out = Vec::with_capacity(block_count as usize);
1227+
self.read_context_slots_contiguous_inner(
1228+
file,
1229+
block_start,
1230+
block_count,
1231+
slot,
1232+
|ctx, block| {
1233+
ctx.map(|c| DownstairsBlockContext {
1234+
block,
1235+
block_context: c.block_context,
1236+
on_disk_hash: c.on_disk_hash,
1237+
})
1238+
},
1239+
&mut out,
1240+
)?;
1241+
Ok(out)
1242+
}
1243+
1244+
/// Low-level function to read context slots
1245+
///
1246+
/// This function takes a generic transform function and writes to a
1247+
/// user-provided array, to minimize allocations.
1248+
fn read_context_slots_contiguous_inner<F, T>(
1249+
&self,
1250+
file: &File,
1251+
block_start: u64,
1252+
block_count: u64,
1253+
slot: ContextSlot,
1254+
f: F,
1255+
out: &mut Vec<T>,
1256+
) -> Result<(), CrucibleError>
1257+
where
1258+
F: Fn(Option<OnDiskDownstairsBlockContext>, u64) -> T,
1259+
{
12081260
let mut buf =
12091261
vec![0u8; (BLOCK_CONTEXT_SLOT_SIZE_BYTES * block_count) as usize];
12101262

@@ -1213,7 +1265,6 @@ impl RawLayout {
12131265
CrucibleError::IoError(format!("reading context slots failed: {e}"))
12141266
})?;
12151267

1216-
let mut out = vec![];
12171268
for (i, chunk) in buf
12181269
.chunks_exact(BLOCK_CONTEXT_SLOT_SIZE_BYTES as usize)
12191270
.enumerate()
@@ -1222,13 +1273,10 @@ impl RawLayout {
12221273
bincode::deserialize(chunk).map_err(|e| {
12231274
CrucibleError::BadContextSlot(e.to_string())
12241275
})?;
1225-
out.push(ctx.map(|c| DownstairsBlockContext {
1226-
block: block_start + i as u64,
1227-
block_context: c.block_context,
1228-
on_disk_hash: c.on_disk_hash,
1229-
}));
1276+
let v = f(ctx, block_start + i as u64);
1277+
out.push(v);
12301278
}
1231-
Ok(out)
1279+
Ok(())
12321280
}
12331281

12341282
/// Write out the active context array and metadata section of the file

0 commit comments

Comments
 (0)