mirror/matrix-rust-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-rust-sdk.git synced 2026-05-06 15:04:11 -04:00
Code Issues Packages Projects Releases Wiki Activity

refactor(common): LinkedChunk can start by a gap.

Browse Source 
This patch removes the invariant stating that a `LinkedChunk` must start
by a chunk of type items. This has never been really useful but it's now
annoying to have this (with iterative loading of a `LinkedChunk` via the
`EventCache`, it's now possible to get a gap as the first chunk). Let's
remove this invariant.
This commit is contained in:
Ivan Enderlin
2025-02-10 17:31:31 +01:00
parent 2999d10fb9
commit 9db137af44
1 changed files with 210 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

319
crates/matrix-sdk-common/src/linked_chunk/mod.rs
View File

@@ -257,7 +257,6 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
pub fn new() -> Self {
Self {
links: Ends {
// INVARIANT: The first chunk must always be an Items, not a Gap.
first: Chunk::new_items_leaked(ChunkIdentifierGenerator::FIRST_IDENTIFIER),
last: None,
},
@@ -283,11 +282,7 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
});
Self {
links: Ends {
// INVARIANT: The first chunk must always be an Items, not a Gap.
first: Chunk::new_items_leaked(first_chunk_identifier),
last: None,
},
links: Ends { first: Chunk::new_items_leaked(first_chunk_identifier), last: None },
chunk_identifier_generator: ChunkIdentifierGenerator::new_from_scratch(),
updates: Some(updates),
marker: PhantomData,
@@ -336,11 +331,12 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
debug_assert!(last_chunk.is_last_chunk(), "`last_chunk` must be… the last chunk");
// We need to update `self.last` if and only if `last_chunk` _is not_ the first
// chunk, and _is_ the last chunk (ensured by the `debug_assert!` above).
// We need to update `self.links.last` if and only if `last_chunk` _is not_ the
// first chunk, and _is_ the last chunk (ensured by the `debug_assert!`
// above).
if last_chunk.is_first_chunk().not() {
// Maybe `last_chunk` is the same as the previous `self.last` chunk, but it's
// OK.
// Maybe `last_chunk` is the same as the previous `self.links.last` chunk, but
// it's OK.
self.links.last = Some(last_chunk.as_ptr());
}
}
@@ -437,10 +433,10 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
}
};
// We need to update `self.last` if and only if `chunk` _is not_ the first
// We need to update `self.links.last` if and only if `chunk` _is not_ the first
// chunk, and _is_ the last chunk.
if chunk.is_first_chunk().not() && chunk.is_last_chunk() {
// Maybe `chunk` is the same as the previous `self.last` chunk, but it's
// Maybe `chunk` is the same as the previous `self.links.last` chunk, but it's
// OK.
self.links.last = Some(chunk.as_ptr());
}
@@ -505,8 +501,8 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
chunk_ptr = Some(chunk.as_ptr());
// We need to update `self.last` if and only if `chunk` _is_ the last chunk. The
// new last chunk is the chunk before `chunk`.
// We need to update `self.links.last` if and only if `chunk` _is_ the last
// chunk. The new last chunk is the chunk before `chunk`.
if chunk.is_last_chunk() {
self.links.last = chunk.previous;
}
@@ -586,36 +582,43 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
.chunk_mut(chunk_identifier)
.ok_or(Error::InvalidChunkIdentifier { identifier: chunk_identifier })?;
// If `item_index` is 0, we don't want to split the current items chunk to
// insert a new gap chunk, otherwise it would create an empty current items
// chunk. Let's handle this case in particular.
//
// Of course this optimisation applies if there is a previous chunk. Remember
// the invariant: a `Gap` cannot be the first chunk.
if item_index == 0 && chunk.is_items() && chunk.previous.is_some() {
let previous_chunk = chunk
.previous_mut()
// SAFETY: The `previous` chunk exists because we have tested
// `chunk.previous.is_some()` in the `if` statement.
.expect("Previous chunk must be present");
previous_chunk.insert_next(
Chunk::new_gap_leaked(self.chunk_identifier_generator.next(), content),
&mut self.updates,
);
// We don't need to update `self.last` because we have inserted a new chunk
// before `chunk`.
return Ok(());
}
let chunk = match &mut chunk.content {
ChunkContent::Gap(..) => {
return Err(Error::ChunkIsAGap { identifier: chunk_identifier });
}
ChunkContent::Items(current_items) => {
// If `item_index` is 0, we don't want to split the current items chunk to
// insert a new gap chunk, otherwise it would create an empty current items
// chunk. Let's handle this case in particular.
if item_index == 0 {
let chunk_was_first = chunk.is_first_chunk();
let chunk_was_last = chunk.is_last_chunk();
let new_chunk = chunk.insert_before(
Chunk::new_gap_leaked(self.chunk_identifier_generator.next(), content),
&mut self.updates,
);
let new_chunk_ptr = new_chunk.as_ptr();
let chunk_ptr = chunk.as_ptr();
// `chunk` was the first: let's update `self.links.first`.
//
// If `chunk` was not the first but was the last, there is nothing to do,
// `self.links.last` is already up-to-date.
if chunk_was_first {
self.links.first = new_chunk_ptr;
// `chunk` was the first __and__ the last: let's set `self.links.last`.
if chunk_was_last {
self.links.last = Some(chunk_ptr);
}
}
return Ok(());
}
let current_items_length = current_items.len();
if item_index >= current_items_length {
@@ -663,10 +666,10 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
}
};
// We need to update `self.last` if and only if `chunk` _is not_ the first
// We need to update `self.links.last` if and only if `chunk` _is not_ the first
// chunk, and _is_ the last chunk.
if chunk.is_first_chunk().not() && chunk.is_last_chunk() {
// Maybe `chunk` is the same as the previous `self.last` chunk, but it's
// Maybe `chunk` is the same as the previous `self.links.last` chunk, but it's
// OK.
self.links.last = Some(chunk.as_ptr());
}
@@ -691,17 +694,26 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
return Err(Error::ChunkIsItems { identifier: chunk_identifier });
};
let next = chunk.next;
let chunk_was_first = chunk.is_first_chunk();
let chunk_was_last = chunk.is_last_chunk();
let next_ptr = chunk.next;
let previous_ptr = chunk.previous;
let position_of_next = chunk.next().map(|next| next.first_position());
chunk.unlink(&mut self.updates);
let chunk_ptr = chunk.as_ptr();
// If this ever changes, we may need to update self.links.first too.
debug_assert!(chunk.is_first_chunk().not(), "A gap cannot be the first chunk");
// If the chunk is the first one, we need to update `self.links.first`…
if chunk_was_first {
// … if and only if there is a next chunk.
if let Some(next_ptr) = next_ptr {
self.links.first = next_ptr;
}
}
if chunk.is_last_chunk() {
self.links.last = chunk.previous;
if chunk_was_last {
self.links.last = previous_ptr;
}
// SAFETY: `chunk` is unlinked and not borrowed anymore. `LinkedChunk` doesn't
@@ -709,10 +721,7 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
let _chunk_boxed = unsafe { Box::from_raw(chunk_ptr.as_ptr()) };
// Return the first position of the next chunk, if any.
Ok(next.map(|next| {
let chunk = unsafe { next.as_ref() };
chunk.first_position()
}))
Ok(position_of_next)
}
/// Replace the gap identified by `chunk_identifier`, by items.
@@ -746,7 +755,7 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
return Err(Error::ChunkIsItems { identifier: chunk_identifier });
};
debug_assert!(chunk.is_first_chunk().not(), "A gap cannot be the first chunk");
let chunk_was_first = chunk.is_first_chunk();
let maybe_last_chunk_ptr = {
let items = items.into_iter();
@@ -774,7 +783,13 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
// Get the pointer to `chunk`.
chunk_ptr = chunk.as_ptr();
// Update `self.last` if the gap chunk was the last chunk.
// Update `self.links.first` if the gap chunk was the first chunk.
if chunk_was_first {
self.links.first = new_chunk_ptr;
}
// Update `self.links.last` if the gap (so the new) chunk was (is) the last
// chunk.
if let Some(last_chunk_ptr) = maybe_last_chunk_ptr {
self.links.last = Some(last_chunk_ptr);
}
@@ -1410,6 +1425,53 @@ impl<const CAPACITY: usize, Item, Gap> Chunk<CAPACITY, Item, Gap> {
new_chunk
}
/// Insert a new chunk before the current one.
///
/// The respective [`Self::previous`] and [`Self::next`] of the current
/// and new chunk will be updated accordingly.
fn insert_before(
&mut self,
mut new_chunk_ptr: NonNull<Self>,
updates: &mut Option<ObservableUpdates<Item, Gap>>,
) -> &mut Self
where
Gap: Clone,
{
let new_chunk = unsafe { new_chunk_ptr.as_mut() };
// Update the previous chunk if any.
if let Some(previous_chunk) = self.previous_mut() {
// Link back to the new chunk.
previous_chunk.next = Some(new_chunk_ptr);
// Link the new chunk to the next chunk.
new_chunk.previous = self.previous;
}
// Link to the new chunk.
self.previous = Some(new_chunk_ptr);
// Link the new chunk to this one.
new_chunk.next = Some(self.as_ptr());
if let Some(updates) = updates.as_mut() {
let previous = new_chunk.previous().map(Chunk::identifier);
let new = new_chunk.identifier();
let next = new_chunk.next().map(Chunk::identifier);
match new_chunk.content() {
ChunkContent::Gap(gap) => {
updates.push(Update::NewGapChunk { previous, new, next, gap: gap.clone() })
}
ChunkContent::Items(..) => {
updates.push(Update::NewItemsChunk { previous, new, next })
}
}
}
new_chunk
}
/// Unlink this chunk.
///
/// Be careful: `self` won't belong to `LinkedChunk` anymore, and should be
@@ -2529,36 +2591,28 @@ mod tests {
);
}
// Insert at the beginning of a chunk + it's the first chunk.
// Insert at the beginning of a chunk. The targeted chunk is the first chunk.
// `Ends::first` and `Ends::last` may be updated differently.
{
let position_of_a = linked_chunk.item_position(|item| *item == 'a').unwrap();
linked_chunk.insert_gap_at((), position_of_a)?;
// A new empty chunk is created as the first chunk.
assert_items_eq!(linked_chunk, [] [-] ['a'] [-] ['b', 'c'] ['d', 'e', 'f']);
// A new empty chunk is NOT created, i.e. `['a']` is not split into `[]` +
// `['a']` because it's a waste of space.
assert_items_eq!(linked_chunk, [-] ['a'] [-] ['b', 'c'] ['d', 'e', 'f']);
assert_eq!(
linked_chunk.updates().unwrap().take(),
&[
DetachLastItems { at: Position(ChunkIdentifier(0), 0) },
NewGapChunk {
previous: Some(ChunkIdentifier(0)),
new: ChunkIdentifier(4),
next: Some(ChunkIdentifier(2)),
gap: (),
},
StartReattachItems,
NewItemsChunk {
previous: Some(ChunkIdentifier(4)),
new: ChunkIdentifier(5),
next: Some(ChunkIdentifier(2)),
},
PushItems { at: Position(ChunkIdentifier(5), 0), items: vec!['a'] },
EndReattachItems,
]
&[NewGapChunk {
previous: None,
new: ChunkIdentifier(4),
next: Some(ChunkIdentifier(0)),
gap: (),
},]
);
}
// Insert at the beginning of a chunk.
// Insert at the beginning of a chunk. The targeted chunk is not the first
// chunk. `Ends::first` and `Ends::last` may be updated differently.
{
let position_of_d = linked_chunk.item_position(|item| *item == 'd').unwrap();
linked_chunk.insert_gap_at((), position_of_d)?;
@@ -2566,56 +2620,47 @@ mod tests {
// A new empty chunk is NOT created, i.e. `['d', 'e', 'f']` is not
// split into `[]` + `['d', 'e', 'f']` because it's a waste of
// space.
assert_items_eq!(linked_chunk, [] [-] ['a'] [-] ['b', 'c'] [-] ['d', 'e', 'f']);
assert_items_eq!(linked_chunk, [-] ['a'] [-] ['b', 'c'] [-] ['d', 'e', 'f']);
assert_eq!(
linked_chunk.updates().unwrap().take(),
&[NewGapChunk {
previous: Some(ChunkIdentifier(3)),
new: ChunkIdentifier(6),
new: ChunkIdentifier(5),
next: Some(ChunkIdentifier(1)),
gap: (),
}]
);
}
// Insert in an empty chunk + it's the first chunk.
{
let position_of_first_empty_chunk = Position(ChunkIdentifier(0), 0);
assert_matches!(
linked_chunk.insert_gap_at((), position_of_first_empty_chunk),
Err(Error::InvalidItemIndex { index: 0 })
);
assert!(linked_chunk.updates().unwrap().take().is_empty());
}
// Insert in an empty chunk.
{
// Replace a gap by empty items.
let gap_identifier = linked_chunk.chunk_identifier(Chunk::is_gap).unwrap();
let position = linked_chunk.replace_gap_at([], gap_identifier)?.first_position();
assert_items_eq!(linked_chunk, [] [-] ['a'] [-] ['b', 'c'] [] ['d', 'e', 'f']);
assert_items_eq!(linked_chunk, [-] ['a'] [-] ['b', 'c'] [] ['d', 'e', 'f']);
assert_eq!(
linked_chunk.updates().unwrap().take(),
&[
NewItemsChunk {
previous: Some(ChunkIdentifier(6)),
new: ChunkIdentifier(7),
previous: Some(ChunkIdentifier(5)),
new: ChunkIdentifier(6),
next: Some(ChunkIdentifier(1)),
},
RemoveChunk(ChunkIdentifier(6)),
RemoveChunk(ChunkIdentifier(5)),
]
);
linked_chunk.insert_gap_at((), position)?;
assert_items_eq!(linked_chunk, [] [-] ['a'] [-] ['b', 'c'] [-] [] ['d', 'e', 'f']);
assert_items_eq!(linked_chunk, [-] ['a'] [-] ['b', 'c'] [-] [] ['d', 'e', 'f']);
assert_eq!(
linked_chunk.updates().unwrap().take(),
&[NewGapChunk {
previous: Some(ChunkIdentifier(3)),
new: ChunkIdentifier(8),
next: Some(ChunkIdentifier(7)),
new: ChunkIdentifier(7),
next: Some(ChunkIdentifier(6)),
gap: (),
}]
);
@@ -2643,10 +2688,10 @@ mod tests {
{
// It is impossible to get the item position inside a gap. It's only possible if
// the item position is crafted by hand or is outdated.
let position_of_a_gap = Position(ChunkIdentifier(4), 0);
let position_of_a_gap = Position(ChunkIdentifier(2), 0);
assert_matches!(
linked_chunk.insert_gap_at((), position_of_a_gap),
Err(Error::ChunkIsAGap { identifier: ChunkIdentifier(4) })
Err(Error::ChunkIsAGap { identifier: ChunkIdentifier(2) })
);
assert!(linked_chunk.updates().unwrap().take().is_empty());
}
@@ -2766,13 +2811,54 @@ mod tests {
);
}
assert_eq!(linked_chunk.num_items(), 13);
// Replace a gap at the beginning of the linked chunk.
{
let position_of_a = linked_chunk.item_position(|item| *item == 'a').unwrap();
linked_chunk.insert_gap_at((), position_of_a).unwrap();
assert_items_eq!(
linked_chunk,
[-] ['a', 'b'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm'] ['w', 'x', 'y'] ['z']
);
assert_eq!(
linked_chunk.updates().unwrap().take(),
&[NewGapChunk {
previous: None,
new: ChunkIdentifier(8),
next: Some(ChunkIdentifier(0)),
gap: (),
}]
);
let gap_identifier = linked_chunk.chunk_identifier(Chunk::is_gap).unwrap();
assert_eq!(gap_identifier, ChunkIdentifier(8));
let new_chunk = linked_chunk.replace_gap_at(['p'], gap_identifier)?;
assert_eq!(new_chunk.identifier(), ChunkIdentifier(9));
assert_items_eq!(
linked_chunk,
['p'] ['a', 'b'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm'] ['w', 'x', 'y'] ['z']
);
assert_eq!(
linked_chunk.updates().unwrap().take(),
&[
NewItemsChunk {
previous: Some(ChunkIdentifier(8)),
new: ChunkIdentifier(9),
next: Some(ChunkIdentifier(0)),
},
PushItems { at: Position(ChunkIdentifier(9), 0), items: vec!['p'] },
RemoveChunk(ChunkIdentifier(8)),
]
);
}
assert_eq!(linked_chunk.num_items(), 14);
Ok(())
}
#[test]
fn test_remove_gap() -> Result<(), Error> {
fn test_remove_gap_at() -> Result<(), Error> {
use super::Update::*;
let mut linked_chunk = LinkedChunk::<3, char, ()>::new_with_update_history();
@@ -2780,31 +2866,38 @@ mod tests {
// Ignore initial update.
let _ = linked_chunk.updates().unwrap().take();
linked_chunk.insert_gap_at((), Position(ChunkIdentifier(0), 0)).unwrap();
linked_chunk.push_items_back(['a', 'b']);
linked_chunk.push_gap_back(());
linked_chunk.push_items_back(['l', 'm']);
linked_chunk.push_gap_back(());
assert_items_eq!(linked_chunk, ['a', 'b'] [-] ['l', 'm'] [-]);
assert_items_eq!(linked_chunk, [-] ['a', 'b'] [-] ['l', 'm'] [-]);
assert_eq!(
linked_chunk.updates().unwrap().take(),
&[
NewGapChunk {
previous: None,
new: ChunkIdentifier(1),
next: Some(ChunkIdentifier(0)),
gap: (),
},
PushItems { at: Position(ChunkIdentifier(0), 0), items: vec!['a', 'b'] },
NewGapChunk {
previous: Some(ChunkIdentifier(0)),
new: ChunkIdentifier(1),
new: ChunkIdentifier(2),
next: None,
gap: (),
},
NewItemsChunk {
previous: Some(ChunkIdentifier(1)),
new: ChunkIdentifier(2),
next: None,
},
PushItems { at: Position(ChunkIdentifier(2), 0), items: vec!['l', 'm'] },
NewGapChunk {
previous: Some(ChunkIdentifier(2)),
new: ChunkIdentifier(3),
next: None,
},
PushItems { at: Position(ChunkIdentifier(3), 0), items: vec!['l', 'm'] },
NewGapChunk {
previous: Some(ChunkIdentifier(3)),
new: ChunkIdentifier(4),
next: None,
gap: (),
},
]
@@ -2819,20 +2912,28 @@ mod tests {
assert_matches!(err, Error::InvalidChunkIdentifier { .. });
// Remove the gap in the middle.
let maybe_next = linked_chunk.remove_gap_at(ChunkIdentifier(1)).unwrap();
let maybe_next = linked_chunk.remove_gap_at(ChunkIdentifier(2)).unwrap();
let next = maybe_next.unwrap();
// The next insert position at the start of the next chunk.
assert_eq!(next.chunk_identifier(), ChunkIdentifier(2));
assert_eq!(next.chunk_identifier(), ChunkIdentifier(3));
assert_eq!(next.index(), 0);
assert_items_eq!(linked_chunk, ['a', 'b'] ['l', 'm'] [-]);
assert_eq!(linked_chunk.updates().unwrap().take(), &[RemoveChunk(ChunkIdentifier(1))]);
assert_items_eq!(linked_chunk, [-] ['a', 'b'] ['l', 'm'] [-]);
assert_eq!(linked_chunk.updates().unwrap().take(), &[RemoveChunk(ChunkIdentifier(2))]);
// Remove the gap at the end.
let next = linked_chunk.remove_gap_at(ChunkIdentifier(3)).unwrap();
let next = linked_chunk.remove_gap_at(ChunkIdentifier(4)).unwrap();
// It was the last chunk, so there's no next insert position.
assert!(next.is_none());
assert_items_eq!(linked_chunk, [-] ['a', 'b'] ['l', 'm']);
assert_eq!(linked_chunk.updates().unwrap().take(), &[RemoveChunk(ChunkIdentifier(4))]);
// Remove the gap at the beginning.
let maybe_next = linked_chunk.remove_gap_at(ChunkIdentifier(1)).unwrap();
let next = maybe_next.unwrap();
assert_eq!(next.chunk_identifier(), ChunkIdentifier(0));
assert_eq!(next.index(), 0);
assert_items_eq!(linked_chunk, ['a', 'b'] ['l', 'm']);
assert_eq!(linked_chunk.updates().unwrap().take(), &[RemoveChunk(ChunkIdentifier(3))]);
assert_eq!(linked_chunk.updates().unwrap().take(), &[RemoveChunk(ChunkIdentifier(1))]);
Ok(())
}