Skip to content

Commit 2abbf58

Browse files
committed
feat(common): Implement LinkedChunk::clear.
This patch implements `LinkedChunk::clear`. The code from `impl Drop for LinkedChunk` has been moved inside `Ends::clear`, and replaced by a simple `self.links.clear()`. In addition, `LinkedChunk::clear` indeed calls `self.links.clear()` but also resets all fields. This patch adds the `Clear` variant to `Update`. This patch updates `AsVector` to emit a `VectorDiff::Clear` on `Update::Clear`. Finally, this patch adds the necessary tests.
1 parent b979b2e commit 2abbf58

File tree

3 files changed

+122
-26
lines changed

3 files changed

+122
-26
lines changed

crates/matrix-sdk-common/src/linked_chunk/as_vector.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,11 @@ impl UpdateToVectorDiff {
405405
// Exiting the _detaching_ mode.
406406
detaching = false;
407407
}
408+
409+
Update::Clear => {
410+
// Let's straightforwardly emit a `VectorDiff::Clear`.
411+
diffs.push(VectorDiff::Clear);
412+
}
408413
}
409414
}
410415

@@ -473,6 +478,7 @@ mod tests {
473478
VectorDiff::Remove { index } => {
474479
accumulator.remove(index);
475480
}
481+
VectorDiff::Clear => accumulator.clear(),
476482
diff => unimplemented!("{diff:?}"),
477483
}
478484
}
@@ -686,14 +692,20 @@ mod tests {
686692
&[VectorDiff::Insert { index: 14, value: 'z' }],
687693
);
688694

689-
drop(linked_chunk);
690-
assert!(as_vector.take().is_empty());
691-
692-
// Finally, ensure the “reconstitued” vector is the one expected.
695+
// Ensure the “reconstitued” vector is the one expected.
693696
assert_eq!(
694697
accumulator,
695698
vector!['m', 'a', 'w', 'x', 'y', 'b', 'd', 'i', 'j', 'k', 'l', 'e', 'f', 'g', 'z', 'h']
696699
);
700+
701+
// Let's try to clear the linked chunk now.
702+
linked_chunk.clear();
703+
704+
apply_and_assert_eq(&mut accumulator, as_vector.take(), &[VectorDiff::Clear]);
705+
assert!(accumulator.is_empty());
706+
707+
drop(linked_chunk);
708+
assert!(as_vector.take().is_empty());
697709
}
698710

699711
#[test]

crates/matrix-sdk-common/src/linked_chunk/mod.rs

Lines changed: 102 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,36 @@ impl<const CAP: usize, Item, Gap> Ends<CAP, Item, Gap> {
191191
chunk = chunk.previous_mut()?;
192192
}
193193
}
194+
195+
/// Drop all chunks, and re-create the first one.
196+
fn clear(&mut self) {
197+
// Loop over all chunks, from the last to the first chunk, and drop them.
198+
{
199+
// Take the latest chunk.
200+
let mut current_chunk_ptr = self.last.or(Some(self.first));
201+
202+
// As long as we have another chunk…
203+
while let Some(chunk_ptr) = current_chunk_ptr {
204+
// Fetch the previous chunk pointer.
205+
let previous_ptr = unsafe { chunk_ptr.as_ref() }.previous;
206+
207+
// Re-box the chunk, and let Rust does its job.
208+
let _chunk_boxed = unsafe { Box::from_raw(chunk_ptr.as_ptr()) };
209+
210+
// Update the `current_chunk_ptr`.
211+
current_chunk_ptr = previous_ptr;
212+
}
213+
214+
// At this step, all chunks have been dropped, including
215+
// `self.first`.
216+
}
217+
218+
// Recreate the first chunk.
219+
self.first = Chunk::new_items_leaked(ChunkIdentifierGenerator::FIRST_IDENTIFIER);
220+
221+
// Reset the last chunk.
222+
self.last = None;
223+
}
194224
}
195225

196226
/// The [`LinkedChunk`] structure.
@@ -259,6 +289,24 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
259289
}
260290
}
261291

292+
/// Clear all the chunks.
293+
pub fn clear(&mut self) {
294+
// Clear `self.links`.
295+
self.links.clear();
296+
297+
// Clear `self.length`.
298+
self.length = 0;
299+
300+
// Clear `self.chunk_identifier_generator`.
301+
self.chunk_identifier_generator = ChunkIdentifierGenerator::new_from_scratch();
302+
303+
// “Clear” `self.updates`.
304+
if let Some(updates) = self.updates.as_mut() {
305+
// TODO: Optimisation: Do we want to clear all pending `Update`s in `updates`?
306+
updates.push(Update::Clear);
307+
}
308+
}
309+
262310
/// Get the number of items in this linked chunk.
263311
#[allow(clippy::len_without_is_empty)]
264312
pub fn len(&self) -> usize {
@@ -847,27 +895,11 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
847895

848896
impl<const CAP: usize, Item, Gap> Drop for LinkedChunk<CAP, Item, Gap> {
849897
fn drop(&mut self) {
850-
// Take the latest chunk.
851-
let mut current_chunk_ptr = self.links.last.or(Some(self.links.first));
852-
853-
// As long as we have another chunk…
854-
while let Some(chunk_ptr) = current_chunk_ptr {
855-
// Disconnect the chunk by updating `previous_chunk.next` pointer.
856-
let previous_ptr = unsafe { chunk_ptr.as_ref() }.previous;
857-
858-
if let Some(mut previous_ptr) = previous_ptr {
859-
unsafe { previous_ptr.as_mut() }.next = None;
860-
}
861-
862-
// Re-box the chunk, and let Rust does its job.
863-
let _chunk_boxed = unsafe { Box::from_raw(chunk_ptr.as_ptr()) };
864-
865-
// Update the `current_chunk_ptr`.
866-
current_chunk_ptr = previous_ptr;
867-
}
868-
869-
// At this step, all chunks have been dropped, including
870-
// `self.first`.
898+
// Only clear the links. Calling `Self::clear` would be an error as we don't
899+
// want to emit an `Update::Clear` when `self` is dropped. Instead, we only care
900+
// about freeing memory correctly. Rust can take care of everything except the
901+
// pointers in `self.links`, hence the specific call to `self.links.clear()`.
902+
self.links.clear();
871903
}
872904
}
873905

@@ -1391,7 +1423,10 @@ impl EmptyChunk {
13911423

13921424
#[cfg(test)]
13931425
mod tests {
1394-
use std::ops::Not;
1426+
use std::{
1427+
ops::Not,
1428+
sync::{atomic::Ordering, Arc},
1429+
};
13951430

13961431
use assert_matches::assert_matches;
13971432

@@ -2635,4 +2670,49 @@ mod tests {
26352670
assert!(chunks.next().unwrap().is_last_chunk());
26362671
assert!(chunks.next().is_none());
26372672
}
2673+
2674+
// Test `LinkedChunk::clear`. This test creates a `LinkedChunk` with `new` to
2675+
// avoid creating too much confusion with `Update`s. The next test
2676+
// `test_clear_emit_an_update_clear` uses `new_with_update_history` and only
2677+
// test `Update::Clear`.
2678+
#[test]
2679+
fn test_clear() {
2680+
let mut linked_chunk = LinkedChunk::<3, Arc<char>, Arc<()>>::new();
2681+
2682+
let item = Arc::new('a');
2683+
let gap = Arc::new(());
2684+
2685+
linked_chunk.push_items_back([
2686+
item.clone(),
2687+
item.clone(),
2688+
item.clone(),
2689+
item.clone(),
2690+
item.clone(),
2691+
]);
2692+
linked_chunk.push_gap_back(gap.clone());
2693+
linked_chunk.push_items_back([item.clone()]);
2694+
2695+
assert_eq!(Arc::strong_count(&item), 7);
2696+
assert_eq!(Arc::strong_count(&gap), 2);
2697+
assert_eq!(linked_chunk.len(), 6);
2698+
assert_eq!(linked_chunk.chunk_identifier_generator.next.load(Ordering::SeqCst), 3);
2699+
2700+
// Now, we can clear the linked chunk and see what happens.
2701+
linked_chunk.clear();
2702+
2703+
assert_eq!(Arc::strong_count(&item), 1);
2704+
assert_eq!(Arc::strong_count(&gap), 1);
2705+
assert_eq!(linked_chunk.len(), 0);
2706+
assert_eq!(linked_chunk.chunk_identifier_generator.next.load(Ordering::SeqCst), 0);
2707+
}
2708+
2709+
#[test]
2710+
fn test_clear_emit_an_update_clear() {
2711+
use super::Update::*;
2712+
2713+
let mut linked_chunk = LinkedChunk::<3, char, ()>::new_with_update_history();
2714+
linked_chunk.clear();
2715+
2716+
assert_eq!(linked_chunk.updates().unwrap().take(), &[Clear]);
2717+
}
26382718
}

crates/matrix-sdk-common/src/linked_chunk/updates.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@ pub enum Update<Item, Gap> {
9999

100100
/// Reattaching items (see [`Self::StartReattachItems`]) is finished.
101101
EndReattachItems,
102+
103+
/// All chunks have been cleared, i.e. all items and all gaps have been
104+
/// dropped.
105+
Clear,
102106
}
103107

104108
/// A collection of [`Update`]s that can be observed.

0 commit comments

Comments
 (0)