Skip to content

feat(sdk): Improve ChunkIdentifierGenerator #3256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 21, 2024
Merged

feat(sdk): Improve ChunkIdentifierGenerator #3256

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ab2b5bf
feat(sdk): Remove `AtomicU64::load` in `ChunkIdentifierGenerator`.
Hywan Mar 20, 2024
01c5412
feat(sdk): `ChunkIdentifierGenerator::generate_next` panics.
Hywan Mar 20, 2024
199275f
feat(sdk): Rename `ChunkIdentifierGenerator::generate_next` to `next`.
Hywan Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 22 additions & 35 deletions crates/matrix-sdk/src/event_cache/linked_chunk.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl<Item, Gap, const CAP: usize> LinkedChunk<Item, Gap, CAP> {
/// Push a gap at the end of the [`LinkedChunk`], i.e. after the last
/// chunk.
pub fn push_gap_back(&mut self, content: Gap) {
let next_identifier = self.chunk_identifier_generator.generate_next().unwrap();
let next_identifier = self.chunk_identifier_generator.next();

let last_chunk = self.latest_chunk_mut();
last_chunk.insert_next(Chunk::new_gap_leaked(next_identifier, content));
Expand Down Expand Up @@ -210,10 +210,8 @@ impl<Item, Gap, const CAP: usize> LinkedChunk<Item, Gap, CAP> {
// `chunk.previous.is_some()` in the `if` statement.
.expect("Previous chunk must be present");

previous_chunk.insert_next(Chunk::new_gap_leaked(
chunk_identifier_generator.generate_next().unwrap(),
content,
));
previous_chunk
.insert_next(Chunk::new_gap_leaked(chunk_identifier_generator.next(), content));

// We don't need to update `self.last` because we have inserted a new chunk
// before `chunk`.
Expand All @@ -237,14 +235,9 @@ impl<Item, Gap, const CAP: usize> LinkedChunk<Item, Gap, CAP> {

chunk
// Insert a new gap chunk.
.insert_next(Chunk::new_gap_leaked(
chunk_identifier_generator.generate_next().unwrap(),
content,
))
.insert_next(Chunk::new_gap_leaked(chunk_identifier_generator.next(), content))
// Insert a new items chunk.
.insert_next(Chunk::new_items_leaked(
chunk_identifier_generator.generate_next().unwrap(),
))
.insert_next(Chunk::new_items_leaked(chunk_identifier_generator.next()))
// Finally, push the items that have been detached.
.push_items(detached_items.into_iter(), &chunk_identifier_generator)
}
Expand Down Expand Up @@ -295,9 +288,8 @@ impl<Item, Gap, const CAP: usize> LinkedChunk<Item, Gap, CAP> {

let last_inserted_chunk = chunk
// Insert a new items chunk…
.insert_next(Chunk::new_items_leaked(
chunk_identifier_generator.generate_next().unwrap(),
)) // … and insert the items.
.insert_next(Chunk::new_items_leaked(chunk_identifier_generator.next()))
// … and insert the items.
.push_items(items, &chunk_identifier_generator);

(
Expand Down Expand Up @@ -590,18 +582,17 @@ impl ChunkIdentifierGenerator {
/// Generate the next unique identifier.
///
/// Note that it can fail if there is no more unique identifier available.
/// In this case, `Result::Err` contains the previous unique identifier.
pub fn generate_next(&self) -> Result<ChunkIdentifier, ChunkIdentifier> {
/// In this case, this method will panic.
pub fn next(&self) -> ChunkIdentifier {
let previous = self.next.fetch_add(1, Ordering::Relaxed);
let current = self.next.load(Ordering::Relaxed);

// Check for overflows.
// unlikely — TODO: call `std::intrinsics::unlikely` once it's stable.
if current < previous {
return Err(ChunkIdentifier(previous));
if previous == u64::MAX {
panic!("No more chunk identifiers available. Congrats, you did it. 2^64 identifiers have been consumed.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

easter eggs \o/

}

Ok(ChunkIdentifier(current))
ChunkIdentifier(previous + 1)
}
}

Expand Down Expand Up @@ -836,9 +827,7 @@ impl<Item, Gap, const CAPACITY: usize> Chunk<Item, Gap, CAPACITY> {
ChunkContent::Gap(..) => {
self
// Insert a new items chunk.
.insert_next(Self::new_items_leaked(
chunk_identifier_generator.generate_next().unwrap(),
))
.insert_next(Self::new_items_leaked(chunk_identifier_generator.next()))
// Now push the new items on the next chunk, and return the result of
// `push_items`.
.push_items(new_items, chunk_identifier_generator)
Expand All @@ -862,9 +851,7 @@ impl<Item, Gap, const CAPACITY: usize> Chunk<Item, Gap, CAPACITY> {

self
// Insert a new items chunk.
.insert_next(Self::new_items_leaked(
chunk_identifier_generator.generate_next().unwrap(),
))
.insert_next(Self::new_items_leaked(chunk_identifier_generator.next()))
// Now push the rest of the new items on the next chunk, and return the
// result of `push_items`.
.push_items(new_items, chunk_identifier_generator)
Expand Down Expand Up @@ -1041,18 +1028,18 @@ mod tests {
fn test_chunk_identifier_generator() {
let generator = ChunkIdentifierGenerator::new_from_scratch();

assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(1)));
assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(2)));
assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(3)));
assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(4)));
assert_eq!(generator.next(), ChunkIdentifier(1));
assert_eq!(generator.next(), ChunkIdentifier(2));
assert_eq!(generator.next(), ChunkIdentifier(3));
assert_eq!(generator.next(), ChunkIdentifier(4));

let generator =
ChunkIdentifierGenerator::new_from_previous_chunk_identifier(ChunkIdentifier(42));

assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(43)));
assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(44)));
assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(45)));
assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(46)));
assert_eq!(generator.next(), ChunkIdentifier(43));
assert_eq!(generator.next(), ChunkIdentifier(44));
assert_eq!(generator.next(), ChunkIdentifier(45));
assert_eq!(generator.next(), ChunkIdentifier(46));
}

#[test]
Expand Down