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
```
This commit is contained in:
Alan Guo Xiang Tan 2023-01-03 09:00:42 +08:00 committed by GitHub
parent be1ae9411b
commit 24db6fbb73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 39 additions and 9 deletions

View File

@ -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

View File

@ -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