Skip to content

Commit 418753f

Browse files
committed
refactor!: Box changeset in StoreErrorWithDump
As mentioned in the corresponding issue, `Box`ing helps reduce memory overhead while passing the enum around and also to limit the size of enums with `StoreErrorWithDump` as a variant. Breaking: The enum `StoreErrorWithDump` has been changed.
1 parent a09f12a commit 418753f

File tree

1 file changed

+16
-12
lines changed

1 file changed

+16
-12
lines changed

crates/file_store/src/store.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ where
118118
/// let mut new_store =
119119
/// Store::create(&MAGIC_BYTES, &new_file_path).expect("must create new file");
120120
/// if let Some(aggregated_changeset) = changeset {
121-
/// new_store.append(&aggregated_changeset)?;
121+
/// new_store.append(aggregated_changeset.as_ref())?;
122122
/// }
123123
/// // The following will overwrite the original file. You will loose the corrupted
124124
/// // portion of the original file forever.
@@ -152,7 +152,7 @@ where
152152
f.read_exact(&mut magic_buf)?;
153153
if magic_buf != magic {
154154
return Err(StoreErrorWithDump {
155-
changeset: Option::<C>::None,
155+
changeset: Option::<Box<C>>::None,
156156
error: StoreError::InvalidMagicBytes {
157157
got: magic_buf,
158158
expected: magic.to_vec(),
@@ -194,7 +194,7 @@ where
194194
Ok(aggregated_changeset)
195195
}
196196
Err(iter_error) => Err(StoreErrorWithDump {
197-
changeset: aggregated_changeset,
197+
changeset: aggregated_changeset.map(Box::new),
198198
error: iter_error,
199199
}),
200200
},
@@ -220,7 +220,7 @@ where
220220
Self::create(magic, file_path)
221221
.map(|store| (store, Option::<C>::None))
222222
.map_err(|err: StoreError| StoreErrorWithDump {
223-
changeset: Option::<C>::None,
223+
changeset: Option::<Box<C>>::None,
224224
error: err,
225225
})
226226
}
@@ -257,7 +257,7 @@ where
257257
#[derive(Debug)]
258258
pub struct StoreErrorWithDump<C> {
259259
/// The partially-aggregated changeset.
260-
pub changeset: Option<C>,
260+
pub changeset: Option<Box<C>>,
261261

262262
/// The [`StoreError`]
263263
pub error: StoreError,
@@ -266,7 +266,7 @@ pub struct StoreErrorWithDump<C> {
266266
impl<C> From<io::Error> for StoreErrorWithDump<C> {
267267
fn from(value: io::Error) -> Self {
268268
Self {
269-
changeset: Option::<C>::None,
269+
changeset: Option::<Box<C>>::None,
270270
error: StoreError::Io(value),
271271
}
272272
}
@@ -371,7 +371,7 @@ mod test {
371371
changeset,
372372
error: StoreError::Bincode(_),
373373
}) => {
374-
assert_eq!(changeset, Some(test_changesets))
374+
assert_eq!(changeset, Some(Box::new(test_changesets)))
375375
}
376376
unexpected_res => panic!("unexpected result: {unexpected_res:?}"),
377377
}
@@ -399,7 +399,7 @@ mod test {
399399
changeset,
400400
error: StoreError::Bincode(_),
401401
}) => {
402-
assert_eq!(changeset, Some(test_changesets))
402+
assert_eq!(changeset, Some(Box::new(test_changesets)))
403403
}
404404
unexpected_res => panic!("unexpected result: {unexpected_res:?}"),
405405
}
@@ -500,10 +500,14 @@ mod test {
500500
.expect_err("should fail to aggregate");
501501
assert_eq!(
502502
err.changeset,
503-
changesets.iter().cloned().reduce(|mut acc, cs| {
504-
Merge::merge(&mut acc, cs);
505-
acc
506-
}),
503+
changesets
504+
.iter()
505+
.cloned()
506+
.reduce(|mut acc, cs| {
507+
Merge::merge(&mut acc, cs);
508+
acc
509+
})
510+
.map(Box::new),
507511
"should recover all changesets that are written in full",
508512
);
509513
// Remove file and start again

0 commit comments

Comments
 (0)