From 24db6fbb7366e0a9dc3c5e7a047dd902346efe99 Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Tue, 3 Jan 2023 09:00:42 +0800
Subject: [PATCH] PERF: Memoize topic level checks in PostGuardian (#19647)

When loading posts in a topic, the topic level guardian
checks are run multiple times even though all the posts belong to the
same topic. Profiling in production revealed that this accounted for a
significant amount of request time for a user that is not staff or anon.
Therefore, we're optimizing this by adding memoizing the topic level
calls in `PostGuardian`. Speficifally, the result of
`TopicGuardian#can_see_topic?` and `PostGuardian#can_create_post?`
method calls are memoized per topic.

Locally profiling shows a significant improvement for normal users
loading a topic with 100 posts.

Benchmark script command: `ruby script/bench.rb --unicorn --skip-bundle-assets --iterations 100`

Before:

```
topic user:
  50: 114
  75: 117
  90: 122
  99: 209
topic.json user:
  50: 67
  75: 69
  90: 72
  99: 162
```

After:

```
topic user:
  50: 101
  75: 104
  90: 107
  99: 184
topic.json user:
  50: 53
  75: 53
  90: 56
  99: 138
```
---
 app/models/category.rb        |  2 +-
 lib/guardian/post_guardian.rb | 46 +++++++++++++++++++++++++++++------
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/app/models/category.rb b/app/models/category.rb
index 477744f9879..c68ad7f1c9e 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -228,7 +228,7 @@ class Category < ActiveRecord::Base
         staged: guardian.is_staged?,
         permissions: permissions,
         user_id: guardian.user.id,
-        everyone: Group[:everyone].id)
+        everyone: Group::AUTO_GROUPS[:everyone])
     end
   end
 
diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb
index 2f6a10b9f52..49205fcc169 100644
--- a/lib/guardian/post_guardian.rb
+++ b/lib/guardian/post_guardian.rb
@@ -102,14 +102,15 @@ module PostGuardian
       user.post_count <= SiteSetting.delete_all_posts_max.to_i))
   end
 
-  def can_create_post?(parent)
-    return false if !SiteSetting.enable_system_message_replies? && parent.try(:subtype) == "system_message"
+  def can_create_post?(topic)
+    return can_create_post_in_topic?(topic) if !topic
 
-    (!SpamRule::AutoSilence.prevent_posting?(@user) || (!!parent.try(:private_message?) && parent.allowed_users.include?(@user))) && (
-      !parent ||
-      !parent.category ||
-      Category.post_create_allowed(self).where(id: parent.category.id).count == 1
-    )
+    key = topic_memoize_key(topic)
+    @can_create_post ||= {}
+
+    @can_create_post.fetch(key) do
+      @can_create_post[key] = can_create_post_in_topic?(topic)
+    end
   end
 
   def can_edit_post?(post)
@@ -235,7 +236,7 @@ module PostGuardian
   def can_see_post?(post)
     return false if post.blank?
     return true if is_admin?
-    return false unless can_see_topic?(post.topic)
+    return false unless can_see_post_topic?(post)
     return false unless post.user == @user || Topic.visible_post_types(@user).include?(post.post_type)
     return true if is_moderator? || is_category_group_moderator?(post.topic.category)
     return true if post.deleted_at.blank? || (post.deleted_by_id == @user.id && @user.has_trust_level?(TrustLevel[4]))
@@ -303,4 +304,33 @@ module PostGuardian
   def can_skip_bump?
     is_staff? || @user.has_trust_level?(TrustLevel[4])
   end
+
+  private
+
+  def can_create_post_in_topic?(topic)
+    return false if !SiteSetting.enable_system_message_replies? && topic.try(:subtype) == "system_message"
+
+    (!SpamRule::AutoSilence.prevent_posting?(@user) || (!!topic.try(:private_message?) && topic.allowed_users.include?(@user))) && (
+      !topic ||
+      !topic.category ||
+      Category.post_create_allowed(self).where(id: topic.category.id).count == 1
+    )
+  end
+
+  def topic_memoize_key(topic)
+    # Milliseconds precision on Topic#updated_at so that we don't use memoized results after topic has been updated.
+    "#{topic.id}-#{(topic.updated_at.to_f * 1000).to_i}"
+  end
+
+  def can_see_post_topic?(post)
+    topic = post.topic
+    return false if !topic
+
+    key = topic_memoize_key(topic)
+    @can_see_post_topic ||= {}
+
+    @can_see_post_topic.fetch(key) do
+      @can_see_post_topic[key] = can_see_topic?(topic)
+    end
+  end
 end