From 63bb7a3d602cacbf93e3e9666b48ee9e586a3e8f Mon Sep 17 00:00:00 2001 From: Vika Date: Sat, 23 Jul 2022 17:38:51 +0300 Subject: FileStorage: save memory by taking children instead of cloning --- kittybox-rs/src/database/file/mod.rs | 38 +++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/kittybox-rs/src/database/file/mod.rs b/kittybox-rs/src/database/file/mod.rs index fb18dc4..f83e682 100644 --- a/kittybox-rs/src/database/file/mod.rs +++ b/kittybox-rs/src/database/file/mod.rs @@ -271,7 +271,7 @@ async fn hydrate_author( #[async_trait] impl Storage for FileStorage { - #[tracing::instrument] + #[tracing::instrument(skip(self))] async fn post_exists(&self, url: &str) -> Result { let path = url_to_path(&self.root_dir, url); debug!("Checking if {:?} exists...", path); @@ -291,7 +291,7 @@ impl Storage for FileStorage { Ok(spawn_blocking(move || path.is_file()).await.unwrap()) } - #[tracing::instrument] + #[tracing::instrument(skip(self))] async fn get_post(&self, url: &str) -> Result> { let path = url_to_path(&self.root_dir, url); // TODO: check that the path actually belongs to the dir of user who requested it @@ -322,7 +322,7 @@ impl Storage for FileStorage { } } - #[tracing::instrument] + #[tracing::instrument(skip(self))] async fn put_post(&self, post: &'_ serde_json::Value, user: &'_ str) -> Result<()> { let key = post["properties"]["uid"][0] .as_str() @@ -433,7 +433,7 @@ impl Storage for FileStorage { Ok(()) } - #[tracing::instrument] + #[tracing::instrument(skip(self))] async fn update_post(&self, url: &'_ str, update: serde_json::Value) -> Result<()> { let path = url_to_path(&self.root_dir, url); let tempfilename = path.with_extension("tmp"); @@ -464,7 +464,7 @@ impl Storage for FileStorage { Ok(()) } - #[tracing::instrument] + #[tracing::instrument(skip(self))] async fn get_channels(&self, user: &'_ str) -> Result> { let mut path = relative_path::RelativePathBuf::new(); path.push( @@ -498,7 +498,7 @@ impl Storage for FileStorage { } } - #[tracing::instrument] + #[tracing::instrument(skip(self))] async fn read_feed_with_limit( &self, url: &'_ str, @@ -509,17 +509,23 @@ impl Storage for FileStorage { if let Some(feed) = self.get_post(url).await? { if let Some(mut feed) = filter_post(feed, user) { if feed["children"].is_array() { - // This code contains several clones. It looks - // like the borrow checker thinks it is preventing - // me from doing something incredibly stupid. The - // borrow checker may or may not be right. - let children = feed["children"].as_array().unwrap().clone(); + // Take this out of the MF2-JSON document to save memory + // + // This uses a clever match with enum destructuring + // to extract the underlying Vec without cloning it + let children: Vec = match feed["children"].take() { + serde_json::Value::Array(children) => children, + // We've already checked it's an array + _ => unreachable!() + }; + let mut posts_iter = children .into_iter() .map(|s: serde_json::Value| s.as_str().unwrap().to_string()); - // Note: we can't actually use skip_while here because we end up emitting `after`. + // Note: we can't actually use `skip_while` here because we end up emitting `after`. // This imperative snippet consumes after instead of emitting it, allowing the - // stream of posts to return only those items that truly come *after* + // stream of posts to return only those items that truly come *after* that one. + // If I would implement an Iter combinator like this, I would call it `skip_until` if let Some(after) = after { for s in posts_iter.by_ref() { if &s == after { @@ -564,7 +570,7 @@ impl Storage for FileStorage { } } - #[tracing::instrument] + #[tracing::instrument(skip(self))] async fn delete_post(&self, url: &'_ str) -> Result<()> { let path = url_to_path(&self.root_dir, url); if let Err(e) = tokio::fs::remove_file(path).await { @@ -575,7 +581,7 @@ impl Storage for FileStorage { } } - #[tracing::instrument] + #[tracing::instrument(skip(self))] async fn get_setting(&self, setting: Settings, user: &'_ str) -> Result { log::debug!("User for getting settings: {}", user); let url = axum::http::Uri::try_from(user).expect("Couldn't parse a URL"); @@ -599,7 +605,7 @@ impl Storage for FileStorage { .ok_or_else(|| StorageError::new(ErrorKind::Backend, "Setting not set")) } - #[tracing::instrument] + #[tracing::instrument(skip(self))] async fn set_setting(&self, setting: Settings, user: &'_ str, value: &'_ str) -> Result<()> { let url = axum::http::Uri::try_from(user).expect("Couldn't parse a URL"); let mut path = relative_path::RelativePathBuf::new(); -- cgit 1.4.1