From 6c64bcce8e932aacf66e8aac98c0613c4b3804a0 Mon Sep 17 00:00:00 2001 From: daladim Date: Fri, 24 Dec 2021 11:07:45 +0100 Subject: [PATCH] [optim] Download 30 added items at once in a single HTTP request from the server --- Cargo.lock | 16 +++++++++ Cargo.toml | 1 + src/provider/mod.rs | 85 ++++++++++++++++++++++++++++----------------- tests/sync.rs | 2 +- 4 files changed, 72 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c3d5283..f457d62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -113,6 +113,12 @@ dependencies = [ "serde", ] +[[package]] +name = "either" +version = "1.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e78d4f1cc4ae33bbfc157ed5d5a5ef3bc29227303d595861deb238fcec4e9457" + [[package]] name = "encoding_rs" version = "0.8.29" @@ -381,6 +387,15 @@ version = "2.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68f2d64f2edebec4ce84ad108148e67e1064789bee435edc5b60ad398714a3a9" +[[package]] +name = "itertools" +version = "0.10.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a9a9d19fa1e79b6215ff29b9d6880b706147f16e9b1dbb1e4e5947b5b02bc5e3" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "0.4.8" @@ -407,6 +422,7 @@ dependencies = [ "env_logger", "ical-daladim", "ics", + "itertools", "log", "minidom", "once_cell", diff --git a/Cargo.toml b/Cargo.toml index 4f4f61d..264f861 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,3 +35,4 @@ ics = "0.5" chrono = { version = "0.4", features = ["serde"] } csscolorparser = { version = "0.5", features = ["serde"] } once_cell = "1.8" +itertools = "0.10" diff --git a/src/provider/mod.rs b/src/provider/mod.rs index b1eb24a..b5a213c 100644 --- a/src/provider/mod.rs +++ b/src/provider/mod.rs @@ -8,6 +8,8 @@ use std::marker::PhantomData; use std::sync::{Arc, Mutex}; use url::Url; +use itertools::Itertools; + use crate::traits::{BaseCalendar, CalDavSource, DavCalendar}; use crate::traits::CompleteCalendar; use crate::item::SyncStatus; @@ -16,6 +18,14 @@ pub mod sync_progress; use sync_progress::SyncProgress; use sync_progress::{FeedbackSender, SyncEvent}; +/// How many items will be batched in a single HTTP request when downloading from the server +#[cfg(not(test))] +const DOWNLOAD_BATCH_SIZE: usize = 30; +/// How many items will be batched in a single HTTP request when downloading from the server +#[cfg(test)] +const DOWNLOAD_BATCH_SIZE: usize = 3; + + /// A data source that combines two `CalDavSource`s, which is able to sync both sources. /// /// Usually, you will only need to use a provider between a server and a local cache, that is to say a [`CalDavProvider`](crate::CalDavProvider), i.e. a `Provider`. \ @@ -388,45 +398,58 @@ where } async fn apply_remote_additions( - remote_additions: HashSet, + mut remote_additions: HashSet, cal_local: &mut T, cal_remote: &mut U, progress: &mut SyncProgress, cal_name: &str ) { - // - // - // - // - // - // TODO: OPTIM: in the server -> local way, download all the content at once - // - for url_add in remote_additions { - progress.debug(&format!("> Applying remote addition {} locally", url_add)); - progress.feedback(SyncEvent::InProgress{ - calendar: cal_name.to_string(), - details: Self::item_name(&cal_local, &url_add).await, - }); - match cal_remote.get_item_by_url(&url_add).await { - Err(err) => { - progress.warn(&format!("Unable to get remote item {}: {}. Skipping it.", url_add, err)); - continue; - }, - Ok(item) => match item { - None => { - progress.error(&format!("Inconsistency: new item {} has vanished from the remote end", url_add)); - continue; - }, - Some(new_item) => { - if let Err(err) = cal_local.add_item(new_item.clone()).await { - progress.error(&format!("Not able to add item {} to local calendar: {}", url_add, err)); - } - }, - }, - } + for batch in remote_additions.drain().chunks(DOWNLOAD_BATCH_SIZE).into_iter() { + Self::apply_some_remote_additions(batch, cal_local, cal_remote, progress, cal_name).await; } } + async fn apply_some_remote_additions>( + remote_additions: I, + cal_local: &mut T, + cal_remote: &mut U, + progress: &mut SyncProgress, + cal_name: &str + ) { + progress.debug(&format!("> Applying a batch of remote additions locally") /* too bad Chunks does not implement ExactSizeIterator, that could provide useful debug info. See https://github.com/rust-itertools/itertools/issues/171 */); + + let list_of_additions: Vec = remote_additions.map(|url| url.clone()).collect(); + match cal_remote.get_items_by_url(&list_of_additions).await { + Err(err) => { + progress.warn(&format!("Unable to get a batch of items {:?}: {}. Skipping them.", list_of_additions, err)); + }, + Ok(items) => { + for item in items { + match item { + None => { + progress.error(&format!("Inconsistency: an item from the batch has vanished from the remote end")); + continue; + }, + Some(new_item) => { + if let Err(err) = cal_local.add_item(new_item.clone()).await { + progress.error(&format!("Not able to add item {} to local calendar: {}", new_item.url(), err)); + } + }, + } + } + + // Notifying every item at the same time would not make sense. Let's notify only one of them + let one_item_name = match list_of_additions.get(0) { + Some(url) => Self::item_name(&cal_local, &url).await, + None => String::from(""), + }; + progress.feedback(SyncEvent::InProgress{ + calendar: cal_name.to_string(), + details: one_item_name, + }); + }, + } + } } diff --git a/tests/sync.rs b/tests/sync.rs index 37e92ac..cc46469 100644 --- a/tests/sync.rs +++ b/tests/sync.rs @@ -145,7 +145,7 @@ impl TestFlavour { scenarii: scenarii::scenarii_basic(), mock_behaviour: Arc::new(Mutex::new(MockBehaviour{ add_item_behaviour: (2,3), - get_item_by_url_behaviour: (1,4), + get_item_by_url_behaviour: (1,12), ..MockBehaviour::default() })), }