From 63bb7a3d602cacbf93e3e9666b48ee9e586a3e8f Mon Sep 17 00:00:00 2001
From: Vika <vika@fireburn.ru>
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(-)

(limited to 'kittybox-rs/src')

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<S: Storage>(
 
 #[async_trait]
 impl Storage for FileStorage {
-    #[tracing::instrument]
+    #[tracing::instrument(skip(self))]
     async fn post_exists(&self, url: &str) -> Result<bool> {
         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<Option<serde_json::Value>> {
         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<Vec<super::MicropubChannel>> {
         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<serde_json::Value> = 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<String> {
         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