From 626c2723d750e5cd933d94a06daa727a41716270 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 21 Apr 2020 14:10:55 -0500 Subject: [PATCH] search: fix scan_query performance regression. Fix a severe performance regression on the posts/index page introduced by 6ca42947. Short answer: scan_query dynamically allocated a regex inside an inner loop that was called thousands of times per pageload. Long answer: * The post index page checks each post to see if they're tagged loli/shota, * This triggers a call to Post#tag_array for every post. * Post#tag_array called scan_query to split the tag string. * scan_query loops over the tag string, checking if each tag matches the regex /#{METATAGS.join("|")}:/. * This regex uses string interpolation, which makes Ruby treat as a dynamic value rather than a static value. Ruby doesn't know the interpolation is static here. This causes the regex to be reallocated on every iteration of the loop, or in other words, for every tag in the tag string. * This caused us to do thousands of regex allocations per pageload. On average, a posts/index pageload contains 20 posts with ~35 tags per post, or 7000+ total tags. Doing this many allocations killed performance. The fix: * Don't use scan_query for Post#tag_array. We don't have to fully parse the tag_string here, we can use a simple split. * Use the /o regex flag to tell Ruby to treat the regex as static and only evaluate the interpolation once. --- app/logical/post_query_builder.rb | 6 +++++- app/models/post.rb | 10 +++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 961dfe490..e6a0a7282 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -713,7 +713,7 @@ class PostQueryBuilder until scanner.eos? scanner.skip(/ +/) - if scanner.scan(/(#{METATAGS.join("|")}):/i) + if scanner.scan(/(#{METATAGS.join("|")}):/io) metatag = scanner.captures.first if scanner.scan(/"(.+)"/) @@ -754,6 +754,10 @@ class PostQueryBuilder tags.join(" ") end + def parse_tag_edit(tag_string) + split_query(tag_string) + end + def parse_query(query, options = {}) q = {} diff --git a/app/models/post.rb b/app/models/post.rb index fbda12343..429148ee0 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -529,11 +529,11 @@ class Post < ApplicationRecord module TagMethods def tag_array - @tag_array ||= PostQueryBuilder.split_query(tag_string) + @tag_array ||= tag_string.split end def tag_array_was - @tag_array_was ||= PostQueryBuilder.split_query(tag_string_in_database.presence || tag_string_before_last_save || "") + @tag_array_was ||= (tag_string_in_database.presence || tag_string_before_last_save || "").split end def tags @@ -595,9 +595,9 @@ class Post < ApplicationRecord if old_tag_string # If someone else committed changes to this post before we did, # then try to merge the tag changes together. - current_tags = tag_array_was - new_tags = tag_array - old_tags = PostQueryBuilder.split_query(old_tag_string) + current_tags = tag_string_was.split + new_tags = PostQueryBuilder.parse_tag_edit(tag_string) + old_tags = old_tag_string.split kept_tags = current_tags & new_tags @removed_tags = old_tags - kept_tags