From: Felix Ableitner <me@nutomic.com>
Date: Sat, 6 Nov 2021 13:25:34 +0000 (+0100)
Subject: Reduce stack memory usage in apub code
X-Git-Url: http://these/git/%7B%60%24%7BghostArchiveUrl%7D/static/%7Bsrc%7D?a=commitdiff_plain;h=8ea21c39b7d25bf39e9d333040674b55cf640b50;p=lemmy.git

Reduce stack memory usage in apub code

- use our own, smaller Endpoints struct
- wrap ObjectId.url in Box
- adjust usage of Box in different places
---

diff --git a/crates/api/src/post.rs b/crates/api/src/post.rs
index c7acb08f..f22104eb 100644
--- a/crates/api/src/post.rs
+++ b/crates/api/src/post.rs
@@ -73,7 +73,7 @@ impl Perform for CreatePostLike {
     .await??;
 
     let community_id = post.community_id;
-    let object = PostOrComment::Post(Box::new(post));
+    let object = PostOrComment::Post(post);
 
     // Only add the like if the score isnt 0
     let do_add = like_form.score != 0 && (like_form.score == 1 || like_form.score == -1);
diff --git a/crates/api_crud/src/post/create.rs b/crates/api_crud/src/post/create.rs
index 248adee6..1147c1a3 100644
--- a/crates/api_crud/src/post/create.rs
+++ b/crates/api_crud/src/post/create.rs
@@ -146,7 +146,7 @@ impl PerformCrud for CreatePost {
       context,
     )
     .await?;
-    let object = PostOrComment::Post(Box::new(apub_post));
+    let object = PostOrComment::Post(apub_post);
     Vote::send(
       &object,
       &local_user_view.person.clone().into(),
diff --git a/crates/apub/src/activities/community/update.rs b/crates/apub/src/activities/community/update.rs
index 68872247..f5d7e565 100644
--- a/crates/apub/src/activities/community/update.rs
+++ b/crates/apub/src/activities/community/update.rs
@@ -9,7 +9,7 @@ use crate::{
   },
   activity_lists::AnnouncableActivities,
   objects::{community::ApubCommunity, person::ApubPerson},
-  protocol::{activities::community::update::UpdateCommunity, objects::group::Group},
+  protocol::activities::community::update::UpdateCommunity,
 };
 use activitystreams::{activity::kind::UpdateType, public};
 use lemmy_api_common::blocking;
@@ -38,14 +38,14 @@ impl UpdateCommunity {
     let update = UpdateCommunity {
       actor: ObjectId::new(actor.actor_id()),
       to: vec![public()],
-      object: community.clone().into_apub(context).await?,
+      object: Box::new(community.clone().into_apub(context).await?),
       cc: vec![community.actor_id()],
       kind: UpdateType::Update,
       id: id.clone(),
       unparsed: Default::default(),
     };
 
-    let activity = AnnouncableActivities::UpdateCommunity(Box::new(update));
+    let activity = AnnouncableActivities::UpdateCommunity(update);
     send_to_community(activity, &id, actor, &community, vec![], context).await
   }
 }
@@ -73,12 +73,10 @@ impl ActivityHandler for UpdateCommunity {
   ) -> Result<(), LemmyError> {
     let community = self.get_community(context, request_counter).await?;
 
-    let updated_community = Group::into_form(
-      self.object,
-      &community.actor_id.clone().into(),
-      &context.settings(),
-    )
-    .await?;
+    let updated_community = self
+      .object
+      .into_form(&community.actor_id.clone().into(), &context.settings())
+      .await?;
     let cf = CommunityForm {
       name: updated_community.name,
       title: updated_community.title,
diff --git a/crates/apub/src/activities/deletion/mod.rs b/crates/apub/src/activities/deletion/mod.rs
index ddd607a7..1d5a3836 100644
--- a/crates/apub/src/activities/deletion/mod.rs
+++ b/crates/apub/src/activities/deletion/mod.rs
@@ -53,9 +53,9 @@ pub async fn send_apub_remove(
 }
 
 pub enum DeletableObjects {
-  Community(Box<ApubCommunity>),
-  Comment(Box<ApubComment>),
-  Post(Box<ApubPost>),
+  Community(ApubCommunity),
+  Comment(ApubComment),
+  Post(ApubPost),
 }
 
 impl DeletableObjects {
@@ -64,13 +64,13 @@ impl DeletableObjects {
     context: &LemmyContext,
   ) -> Result<DeletableObjects, LemmyError> {
     if let Some(c) = ApubCommunity::read_from_apub_id(ap_id.clone(), context).await? {
-      return Ok(DeletableObjects::Community(Box::new(c)));
+      return Ok(DeletableObjects::Community(c));
     }
     if let Some(p) = ApubPost::read_from_apub_id(ap_id.clone(), context).await? {
-      return Ok(DeletableObjects::Post(Box::new(p)));
+      return Ok(DeletableObjects::Post(p));
     }
     if let Some(c) = ApubComment::read_from_apub_id(ap_id.clone(), context).await? {
-      return Ok(DeletableObjects::Comment(Box::new(c)));
+      return Ok(DeletableObjects::Comment(c));
     }
     Err(diesel::NotFound.into())
   }
diff --git a/crates/apub/src/activities/post/create_or_update.rs b/crates/apub/src/activities/post/create_or_update.rs
index 982f90cc..1c71c8ec 100644
--- a/crates/apub/src/activities/post/create_or_update.rs
+++ b/crates/apub/src/activities/post/create_or_update.rs
@@ -61,7 +61,7 @@ impl CreateOrUpdatePost {
     .into();
     let create_or_update = CreateOrUpdatePost::new(post, actor, &community, kind, context).await?;
     let id = create_or_update.id.clone();
-    let activity = AnnouncableActivities::CreateOrUpdatePost(Box::new(create_or_update));
+    let activity = AnnouncableActivities::CreateOrUpdatePost(create_or_update);
     send_to_community(activity, &id, actor, &community, vec![], context).await
   }
 }
diff --git a/crates/apub/src/activities/voting/undo_vote.rs b/crates/apub/src/activities/voting/undo_vote.rs
index 0fe40b62..6d6f3eaf 100644
--- a/crates/apub/src/activities/voting/undo_vote.rs
+++ b/crates/apub/src/activities/voting/undo_vote.rs
@@ -26,7 +26,6 @@ use lemmy_apub_lib::{
 use lemmy_db_schema::{newtypes::CommunityId, source::community::Community, traits::Crud};
 use lemmy_utils::LemmyError;
 use lemmy_websocket::LemmyContext;
-use std::ops::Deref;
 
 impl UndoVote {
   pub async fn send(
@@ -90,7 +89,7 @@ impl ActivityHandler for UndoVote {
       .dereference(context, request_counter)
       .await?;
     match object {
-      PostOrComment::Post(p) => undo_vote_post(actor, p.deref(), context).await,
+      PostOrComment::Post(p) => undo_vote_post(actor, &p, context).await,
       PostOrComment::Comment(c) => undo_vote_comment(actor, &c, context).await,
     }
   }
diff --git a/crates/apub/src/activities/voting/vote.rs b/crates/apub/src/activities/voting/vote.rs
index 6b9bfbd2..89e88fe6 100644
--- a/crates/apub/src/activities/voting/vote.rs
+++ b/crates/apub/src/activities/voting/vote.rs
@@ -26,7 +26,6 @@ use lemmy_db_schema::{
 };
 use lemmy_utils::LemmyError;
 use lemmy_websocket::LemmyContext;
-use std::ops::Deref;
 
 impl Vote {
   pub(in crate::activities::voting) fn new(
@@ -90,7 +89,7 @@ impl ActivityHandler for Vote {
     let actor = self.actor.dereference(context, request_counter).await?;
     let object = self.object.dereference(context, request_counter).await?;
     match object {
-      PostOrComment::Post(p) => vote_post(&self.kind, actor, p.deref(), context).await,
+      PostOrComment::Post(p) => vote_post(&self.kind, actor, &p, context).await,
       PostOrComment::Comment(c) => vote_comment(&self.kind, actor, &c, context).await,
     }
   }
diff --git a/crates/apub/src/activity_lists.rs b/crates/apub/src/activity_lists.rs
index 6c866b83..2b695111 100644
--- a/crates/apub/src/activity_lists.rs
+++ b/crates/apub/src/activity_lists.rs
@@ -38,7 +38,7 @@ pub enum SharedInboxActivities {
   GroupInboxActivities(GroupInboxActivities),
   // Note, pm activities need to be at the end, otherwise comments will end up here. We can probably
   // avoid this problem by replacing createpm.object with our own struct, instead of NoteExt.
-  PersonInboxActivities(PersonInboxActivities),
+  PersonInboxActivities(Box<PersonInboxActivities>),
 }
 
 #[derive(Clone, Debug, Deserialize, Serialize, ActivityHandler)]
@@ -47,7 +47,7 @@ pub enum SharedInboxActivities {
 pub enum GroupInboxActivities {
   FollowCommunity(FollowCommunity),
   UndoFollowCommunity(UndoFollowCommunity),
-  AnnouncableActivities(AnnouncableActivities),
+  AnnouncableActivities(Box<AnnouncableActivities>),
   Report(Report),
 }
 
@@ -61,7 +61,7 @@ pub enum PersonInboxActivities {
   CreateOrUpdatePrivateMessage(CreateOrUpdatePrivateMessage),
   DeletePrivateMessage(DeletePrivateMessage),
   UndoDeletePrivateMessage(UndoDeletePrivateMessage),
-  AnnounceActivity(Box<AnnounceActivity>),
+  AnnounceActivity(AnnounceActivity),
 }
 
 #[derive(Clone, Debug, Deserialize, Serialize, ActivityHandler)]
@@ -69,12 +69,12 @@ pub enum PersonInboxActivities {
 #[activity_handler(LemmyContext)]
 pub enum AnnouncableActivities {
   CreateOrUpdateComment(CreateOrUpdateComment),
-  CreateOrUpdatePost(Box<CreateOrUpdatePost>),
+  CreateOrUpdatePost(CreateOrUpdatePost),
   Vote(Vote),
   UndoVote(UndoVote),
   Delete(Delete),
   UndoDelete(UndoDelete),
-  UpdateCommunity(Box<UpdateCommunity>),
+  UpdateCommunity(UpdateCommunity),
   BlockUserFromCommunity(BlockUserFromCommunity),
   UndoBlockUserFromCommunity(UndoBlockUserFromCommunity),
   AddMod(AddMod),
diff --git a/crates/apub/src/fetcher/post_or_comment.rs b/crates/apub/src/fetcher/post_or_comment.rs
index 075b018e..dc1546f9 100644
--- a/crates/apub/src/fetcher/post_or_comment.rs
+++ b/crates/apub/src/fetcher/post_or_comment.rs
@@ -11,15 +11,15 @@ use url::Url;
 
 #[derive(Clone, Debug)]
 pub enum PostOrComment {
-  Post(Box<ApubPost>),
+  Post(ApubPost),
   Comment(ApubComment),
 }
 
 #[derive(Deserialize)]
 #[serde(untagged)]
 pub enum PageOrNote {
-  Page(Box<Page>),
-  Note(Box<Note>),
+  Page(Page),
+  Note(Note),
 }
 
 #[async_trait::async_trait(?Send)]
@@ -39,7 +39,7 @@ impl ApubObject for PostOrComment {
   ) -> Result<Option<Self>, LemmyError> {
     let post = ApubPost::read_from_apub_id(object_id.clone(), data).await?;
     Ok(match post {
-      Some(o) => Some(PostOrComment::Post(Box::new(o))),
+      Some(o) => Some(PostOrComment::Post(o)),
       None => ApubComment::read_from_apub_id(object_id, data)
         .await?
         .map(PostOrComment::Comment),
@@ -68,11 +68,11 @@ impl ApubObject for PostOrComment {
     request_counter: &mut i32,
   ) -> Result<Self, LemmyError> {
     Ok(match apub {
-      PageOrNote::Page(p) => PostOrComment::Post(Box::new(
-        ApubPost::from_apub(*p, context, expected_domain, request_counter).await?,
-      )),
+      PageOrNote::Page(p) => PostOrComment::Post(
+        ApubPost::from_apub(p, context, expected_domain, request_counter).await?,
+      ),
       PageOrNote::Note(n) => PostOrComment::Comment(
-        ApubComment::from_apub(*n, context, expected_domain, request_counter).await?,
+        ApubComment::from_apub(n, context, expected_domain, request_counter).await?,
       ),
     })
   }
diff --git a/crates/apub/src/http/community.rs b/crates/apub/src/http/community.rs
index 10794d99..1854da98 100644
--- a/crates/apub/src/http/community.rs
+++ b/crates/apub/src/http/community.rs
@@ -85,7 +85,7 @@ pub(in crate::http) async fn receive_group_inbox(
     let community = announcable.get_community(context, &mut 0).await?;
     verify_person_in_community(&actor_id, &community, context, &mut 0).await?;
     if community.local {
-      AnnounceActivity::send(announcable, &community, vec![], context).await?;
+      AnnounceActivity::send(*announcable, &community, vec![], context).await?;
     }
   }
 
diff --git a/crates/apub/src/http/mod.rs b/crates/apub/src/http/mod.rs
index 03b68b94..b3288b0d 100644
--- a/crates/apub/src/http/mod.rs
+++ b/crates/apub/src/http/mod.rs
@@ -52,7 +52,7 @@ pub async fn shared_inbox(
       receive_group_inbox(g, activity_data, request, &context).await
     }
     SharedInboxActivities::PersonInboxActivities(p) => {
-      receive_person_inbox(p, activity_data, request, &context).await
+      receive_person_inbox(*p, activity_data, request, &context).await
     }
   }
 }
diff --git a/crates/apub/src/objects/community.rs b/crates/apub/src/objects/community.rs
index 1eb58995..35a19353 100644
--- a/crates/apub/src/objects/community.rs
+++ b/crates/apub/src/objects/community.rs
@@ -4,15 +4,12 @@ use crate::{
   generate_moderators_url,
   generate_outbox_url,
   protocol::{
-    objects::{group::Group, tombstone::Tombstone},
+    objects::{group::Group, tombstone::Tombstone, Endpoints},
     ImageObject,
     Source,
   },
 };
-use activitystreams::{
-  actor::{kind::GroupType, Endpoints},
-  object::kind::ImageType,
-};
+use activitystreams::{actor::kind::GroupType, object::kind::ImageType};
 use chrono::NaiveDateTime;
 use itertools::Itertools;
 use lemmy_api_common::blocking;
@@ -111,7 +108,6 @@ impl ApubObject for ApubCommunity {
       followers: self.followers_url.clone().into(),
       endpoints: Endpoints {
         shared_inbox: self.shared_inbox_url.clone().map(|s| s.into()),
-        ..Default::default()
       },
       public_key: self.get_public_key()?,
       published: Some(convert_datetime(self.published)),
diff --git a/crates/apub/src/objects/person.rs b/crates/apub/src/objects/person.rs
index dcaa9cd6..5f494b70 100644
--- a/crates/apub/src/objects/person.rs
+++ b/crates/apub/src/objects/person.rs
@@ -3,12 +3,15 @@ use crate::{
   generate_outbox_url,
   objects::get_summary_from_string_or_source,
   protocol::{
-    objects::person::{Person, UserTypes},
+    objects::{
+      person::{Person, UserTypes},
+      Endpoints,
+    },
     ImageObject,
     Source,
   },
 };
-use activitystreams::{actor::Endpoints, object::kind::ImageType};
+use activitystreams::object::kind::ImageType;
 use chrono::NaiveDateTime;
 use lemmy_api_common::blocking;
 use lemmy_apub_lib::{
@@ -109,7 +112,6 @@ impl ApubObject for ApubPerson {
       outbox: generate_outbox_url(&self.actor_id)?.into(),
       endpoints: Endpoints {
         shared_inbox: self.shared_inbox_url.clone().map(|s| s.into()),
-        ..Default::default()
       },
       public_key: self.get_public_key()?,
       updated: self.updated.map(convert_datetime),
diff --git a/crates/apub/src/protocol/activities/community/update.rs b/crates/apub/src/protocol/activities/community/update.rs
index b9877338..9a831a1f 100644
--- a/crates/apub/src/protocol/activities/community/update.rs
+++ b/crates/apub/src/protocol/activities/community/update.rs
@@ -12,7 +12,7 @@ pub struct UpdateCommunity {
   pub(crate) actor: ObjectId<ApubPerson>,
   pub(crate) to: Vec<Url>,
   // TODO: would be nice to use a separate struct here, which only contains the fields updated here
-  pub(crate) object: Group,
+  pub(crate) object: Box<Group>,
   pub(crate) cc: Vec<Url>,
   #[serde(rename = "type")]
   pub(crate) kind: UpdateType,
diff --git a/crates/apub/src/protocol/objects/group.rs b/crates/apub/src/protocol/objects/group.rs
index acaf5909..a761a442 100644
--- a/crates/apub/src/protocol/objects/group.rs
+++ b/crates/apub/src/protocol/objects/group.rs
@@ -5,12 +5,9 @@ use crate::{
     community_outbox::ApubCommunityOutbox,
   },
   objects::{community::ApubCommunity, get_summary_from_string_or_source},
-  protocol::{ImageObject, Source},
-};
-use activitystreams::{
-  actor::{kind::GroupType, Endpoints},
-  unparsed::Unparsed,
+  protocol::{objects::Endpoints, ImageObject, Source},
 };
+use activitystreams::{actor::kind::GroupType, unparsed::Unparsed};
 use chrono::{DateTime, FixedOffset};
 use lemmy_apub_lib::{object_id::ObjectId, signatures::PublicKey, verify::verify_domains_match};
 use lemmy_db_schema::{naive_now, source::community::CommunityForm};
@@ -46,7 +43,7 @@ pub struct Group {
   pub(crate) inbox: Url,
   pub(crate) outbox: ObjectId<ApubCommunityOutbox>,
   pub(crate) followers: Url,
-  pub(crate) endpoints: Endpoints<Url>,
+  pub(crate) endpoints: Endpoints,
   pub(crate) public_key: PublicKey,
   pub(crate) published: Option<DateTime<FixedOffset>>,
   pub(crate) updated: Option<DateTime<FixedOffset>>,
@@ -72,7 +69,6 @@ impl Group {
     check_slurs(&title, slur_regex)?;
     check_slurs_opt(&description, slur_regex)?;
 
-    // TODO: test_parse_lemmy_community_moderators() keeps failing here with stack overflow
     Ok(CommunityForm {
       name,
       title,
diff --git a/crates/apub/src/protocol/objects/mod.rs b/crates/apub/src/protocol/objects/mod.rs
index 529ee637..bf53d5c9 100644
--- a/crates/apub/src/protocol/objects/mod.rs
+++ b/crates/apub/src/protocol/objects/mod.rs
@@ -1,3 +1,6 @@
+use serde::{Deserialize, Serialize};
+use url::Url;
+
 pub(crate) mod chat_message;
 pub(crate) mod group;
 pub(crate) mod note;
@@ -5,6 +8,13 @@ pub(crate) mod page;
 pub(crate) mod person;
 pub(crate) mod tombstone;
 
+#[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct Endpoints {
+  #[serde(skip_serializing_if = "Option::is_none")]
+  pub shared_inbox: Option<Url>,
+}
+
 #[cfg(test)]
 mod tests {
   use crate::protocol::{
diff --git a/crates/apub/src/protocol/objects/person.rs b/crates/apub/src/protocol/objects/person.rs
index cf9c10b1..e45ea78c 100644
--- a/crates/apub/src/protocol/objects/person.rs
+++ b/crates/apub/src/protocol/objects/person.rs
@@ -1,8 +1,8 @@
 use crate::{
   objects::person::ApubPerson,
-  protocol::{ImageObject, Source},
+  protocol::{objects::Endpoints, ImageObject, Source},
 };
-use activitystreams::{actor::Endpoints, unparsed::Unparsed, url::Url};
+use activitystreams::{unparsed::Unparsed, url::Url};
 use chrono::{DateTime, FixedOffset};
 use lemmy_apub_lib::{object_id::ObjectId, signatures::PublicKey};
 use serde::{Deserialize, Serialize};
@@ -35,7 +35,7 @@ pub struct Person {
   pub(crate) inbox: Url,
   /// mandatory field in activitypub, currently empty in lemmy
   pub(crate) outbox: Url,
-  pub(crate) endpoints: Endpoints<Url>,
+  pub(crate) endpoints: Endpoints,
   pub(crate) public_key: PublicKey,
   pub(crate) published: Option<DateTime<FixedOffset>>,
   pub(crate) updated: Option<DateTime<FixedOffset>>,
diff --git a/crates/apub_lib/src/object_id.rs b/crates/apub_lib/src/object_id.rs
index 52fe5b47..8e4b70a2 100644
--- a/crates/apub_lib/src/object_id.rs
+++ b/crates/apub_lib/src/object_id.rs
@@ -28,9 +28,10 @@ lazy_static! {
     .unwrap();
 }
 
+/// We store Url on the heap because it is quite large (88 bytes).
 #[derive(Clone, PartialEq, Serialize, Deserialize, Debug)]
 #[serde(transparent)]
-pub struct ObjectId<Kind>(Url, #[serde(skip)] PhantomData<Kind>)
+pub struct ObjectId<Kind>(Box<Url>, #[serde(skip)] PhantomData<Kind>)
 where
   Kind: ApubObject + Send + 'static,
   for<'de2> <Kind as ApubObject>::ApubType: serde::Deserialize<'de2>;
@@ -44,7 +45,7 @@ where
   where
     T: Into<Url>,
   {
-    ObjectId(url.into(), PhantomData::<Kind>)
+    ObjectId(Box::new(url.into()), PhantomData::<Kind>)
   }
 
   pub fn inner(&self) -> &Url {
@@ -103,7 +104,7 @@ where
     data: &<Kind as ApubObject>::DataType,
   ) -> Result<Option<Kind>, LemmyError> {
     let id = self.0.clone();
-    ApubObject::read_from_apub_id(id, data).await
+    ApubObject::read_from_apub_id(*id, data).await
   }
 
   async fn dereference_from_http(
@@ -181,7 +182,7 @@ where
   for<'de2> <Kind as ApubObject>::ApubType: serde::Deserialize<'de2>,
 {
   fn from(id: ObjectId<Kind>) -> Self {
-    id.0
+    *id.0
   }
 }
 
diff --git a/crates/apub_lib/src/signatures.rs b/crates/apub_lib/src/signatures.rs
index df959068..ccc72081 100644
--- a/crates/apub_lib/src/signatures.rs
+++ b/crates/apub_lib/src/signatures.rs
@@ -91,7 +91,7 @@ pub fn verify_signature(request: &HttpRequest, public_key: &str) -> Result<(), L
 #[derive(Clone, Debug, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct PublicKey {
-  pub id: String,
-  pub owner: Url,
+  pub(crate) id: String,
+  pub(crate) owner: Box<Url>,
   pub public_key_pem: String,
 }
diff --git a/crates/apub_lib/src/traits.rs b/crates/apub_lib/src/traits.rs
index 38b2d685..18b33ba6 100644
--- a/crates/apub_lib/src/traits.rs
+++ b/crates/apub_lib/src/traits.rs
@@ -82,7 +82,7 @@ pub trait ActorType {
   fn get_public_key(&self) -> Result<PublicKey, LemmyError> {
     Ok(PublicKey {
       id: format!("{}#main-key", self.actor_id()),
-      owner: self.actor_id(),
+      owner: Box::new(self.actor_id()),
       public_key_pem: self.public_key().context(location_info!())?,
     })
   }