From 77c8520e2ecd70520757aed0fbdf434643b60234 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 8 Aug 2016 16:18:13 +0200 Subject: Added concern for a faster "cache_key" method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This concern provides an optimized/simplified version of the "cache_key" method. This method is about 9 times faster than the default "cache_key" method. The produced cache keys _are_ different from the previous ones but this is worth the performance improvement. To showcase this I set up a benchmark (using benchmark-ips) that compares FasterCacheKeys#cache_key with the regular cache_key. The output of this benchmark was: Calculating ------------------------------------- cache_key 4.825k i/100ms cache_key_fast 21.723k i/100ms ------------------------------------------------- cache_key 59.422k (± 7.2%) i/s - 299.150k cache_key_fast 543.243k (± 9.2%) i/s - 2.694M Comparison: cache_key_fast: 543243.4 i/s cache_key: 59422.0 i/s - 9.14x slower To see the impact on real code I applied these changes and benchmarked Issue#referenced_merge_requests. For an issue referencing 10 merge requests these changes shaved off between 40 and 60 milliseconds. --- app/models/issue.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'app/models/issue.rb') diff --git a/app/models/issue.rb b/app/models/issue.rb index 11f734cfc6d..d62ffb21467 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -7,6 +7,7 @@ class Issue < ActiveRecord::Base include Sortable include Taskable include Spammable + include FasterCacheKeys DueDateStruct = Struct.new(:title, :name).freeze NoDueDate = DueDateStruct.new('No Due Date', '0').freeze -- cgit v1.2.3 From 95419679f23f0628d1885dd9656cc159e9d55ea9 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Wed, 27 Jul 2016 19:03:06 -0500 Subject: Lay the ground works to submit information to Akismet - New concern `AkismetSubmittable` to allow issues and other `Spammable` models to be submitted to Akismet. - New model `UserAgentDetail` to store information needed for Akismet. - Services needed for their creation and tests. --- app/models/issue.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'app/models/issue.rb') diff --git a/app/models/issue.rb b/app/models/issue.rb index d62ffb21467..6c2635498e5 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -8,6 +8,7 @@ class Issue < ActiveRecord::Base include Taskable include Spammable include FasterCacheKeys + include AkismetSubmittable DueDateStruct = Struct.new(:title, :name).freeze NoDueDate = DueDateStruct.new('No Due Date', '0').freeze -- cgit v1.2.3 From 722fc84e3d4785fb3a9db5f1c7d2aabad22e8e01 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Thu, 28 Jul 2016 19:02:56 -0500 Subject: Complete refactor of the `Spammable` concern and tests: - Merged `AkismetSubmittable` into `Spammable` - Clean up `SpamCheckService` - Added tests for `Spammable` - Added submit (ham or spam) options to `AkismetHelper` --- app/models/issue.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'app/models/issue.rb') diff --git a/app/models/issue.rb b/app/models/issue.rb index 6c2635498e5..5408e24b21c 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -37,6 +37,8 @@ class Issue < ActiveRecord::Base scope :order_due_date_asc, -> { reorder('issues.due_date IS NULL, issues.due_date ASC') } scope :order_due_date_desc, -> { reorder('issues.due_date IS NULL, issues.due_date DESC') } + attr_spammable :title, :description + state_machine :state, initial: :opened do event :close do transition [:reopened, :opened] => :closed -- cgit v1.2.3 From 64ab2b3d9f10366249c03a6bcf5e8b1d20010d8f Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Fri, 29 Jul 2016 23:18:32 -0500 Subject: Refactored spam related code even further - Removed unnecessary column from `SpamLog` - Moved creation of SpamLogs out of its own service and into SpamCheckService - Simplified code in SpamCheckService. - Moved move spam related code into Spammable concern --- app/models/issue.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'app/models/issue.rb') diff --git a/app/models/issue.rb b/app/models/issue.rb index 5408e24b21c..40028e56489 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -265,4 +265,17 @@ class Issue < ActiveRecord::Base def overdue? due_date.try(:past?) || false end + + # To allow polymorphism with Spammable + def check_for_spam? + super && project.public? + end + + def spam_title + title + end + + def spam_description + description + end end -- cgit v1.2.3 From 43e756d4eafd79f4d2f366b646ebb94af78b5a4c Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Fri, 5 Aug 2016 17:10:08 -0500 Subject: Refactored AkismetHelper into AkismetService and cleaned up `Spammable` - Refactored SpamCheckService into SpamService --- app/models/issue.rb | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) (limited to 'app/models/issue.rb') diff --git a/app/models/issue.rb b/app/models/issue.rb index 40028e56489..ab98d0cf9df 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -37,7 +37,8 @@ class Issue < ActiveRecord::Base scope :order_due_date_asc, -> { reorder('issues.due_date IS NULL, issues.due_date ASC') } scope :order_due_date_desc, -> { reorder('issues.due_date IS NULL, issues.due_date DESC') } - attr_spammable :title, :description + attr_spammable :title, spam_title: true + attr_spammable :description, spam_description: true state_machine :state, initial: :opened do event :close do @@ -266,16 +267,8 @@ class Issue < ActiveRecord::Base due_date.try(:past?) || false end - # To allow polymorphism with Spammable + # Only issues on public projects should be checked for spam def check_for_spam? super && project.public? end - - def spam_title - title - end - - def spam_description - description - end end -- cgit v1.2.3 From 5994c11910822463faeabb7b5f11d6529036db9d Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Tue, 9 Aug 2016 12:43:47 -0500 Subject: Further refactor and syntax fixes. --- app/models/issue.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'app/models/issue.rb') diff --git a/app/models/issue.rb b/app/models/issue.rb index ab98d0cf9df..788611305fe 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -8,7 +8,6 @@ class Issue < ActiveRecord::Base include Taskable include Spammable include FasterCacheKeys - include AkismetSubmittable DueDateStruct = Struct.new(:title, :name).freeze NoDueDate = DueDateStruct.new('No Due Date', '0').freeze @@ -269,6 +268,6 @@ class Issue < ActiveRecord::Base # Only issues on public projects should be checked for spam def check_for_spam? - super && project.public? + project.public? end end -- cgit v1.2.3