Merge branch 'faster-cache-keys' into 'master'
Optimize "cache_key" using a concern
## What does this MR do?
This MR adds a concern (used by Issue and Note) that provides an optimized version of Rails' `cache_key` method. See 77c8520e2e
for more details.
## Are there points in the code the reviewer needs to double check?
No, though a spell check would be appreciated.
## Why was this MR needed?
When loading a lot of data from Redis (e.g. an issue with lots of notes) quite a large amount of time is spent in generating cache keys. This is due to multiple reasons such as:
* Rails trying to figure out if it should use `updated_at` or `updated_on` using somewhat inefficient code
* Rails relying on pluralization logic to figure out how to generate a cache namespace using a model name
* Rails calling a whole bunch of methods in general in the process of generating cache keys
In short, Rails is trying to cater to every possible use case, at the cost of performance.
## What are the relevant issue numbers?
https://gitlab.com/gitlab-org/gitlab-ce/issues/13651 is not directly related but I ran into this `cache_key` problem when looking into said issue.
See merge request !5715
This commit is contained in:
commit
fd3a2cf763
5 changed files with 36 additions and 0 deletions
|
@ -26,6 +26,7 @@ v 8.11.0 (unreleased)
|
|||
- Clean up unused routes (Josef Strzibny)
|
||||
- Fix issue on empty project to allow developers to only push to protected branches if given permission
|
||||
- Add green outline to New Branch button. !5447 (winniehell)
|
||||
- Optimize generating of cache keys for issues and notes
|
||||
- Improve performance of syntax highlighting Markdown code blocks
|
||||
- Update to gitlab_git 10.4.1 and take advantage of preserved Ref objects
|
||||
- Remove delay when hitting "Reply..." button on page with a lot of discussions
|
||||
|
|
16
app/models/concerns/faster_cache_keys.rb
Normal file
16
app/models/concerns/faster_cache_keys.rb
Normal file
|
@ -0,0 +1,16 @@
|
|||
module FasterCacheKeys
|
||||
# A faster version of Rails' "cache_key" method.
|
||||
#
|
||||
# Rails' default "cache_key" method uses all kind of complex logic to figure
|
||||
# out the cache key. In many cases this complexity and overhead may not be
|
||||
# needed.
|
||||
#
|
||||
# This method does not do any timestamp parsing as this process is quite
|
||||
# expensive and not needed when generating cache keys. This method also relies
|
||||
# on the table name instead of the cache namespace name as the latter uses
|
||||
# complex logic to generate the exact same value (as when using the table
|
||||
# name) in 99% of the cases.
|
||||
def cache_key
|
||||
"#{self.class.table_name}/#{id}-#{read_attribute_before_type_cast(:updated_at)}"
|
||||
end
|
||||
end
|
|
@ -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
|
||||
|
|
|
@ -5,6 +5,7 @@ class Note < ActiveRecord::Base
|
|||
include Mentionable
|
||||
include Awardable
|
||||
include Importable
|
||||
include FasterCacheKeys
|
||||
|
||||
# Attribute containing rendered and redacted Markdown as generated by
|
||||
# Banzai::ObjectRenderer.
|
||||
|
|
17
spec/models/concerns/faster_cache_keys_spec.rb
Normal file
17
spec/models/concerns/faster_cache_keys_spec.rb
Normal file
|
@ -0,0 +1,17 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe FasterCacheKeys do
|
||||
describe '#cache_key' do
|
||||
it 'returns a String' do
|
||||
# We're using a fixed string here so it's easier to set an expectation for
|
||||
# the resulting cache key.
|
||||
time = '2016-08-08 16:39:00+02'
|
||||
issue = build(:issue, updated_at: time)
|
||||
issue.extend(described_class)
|
||||
|
||||
expect(issue).to receive(:id).and_return(1)
|
||||
|
||||
expect(issue.cache_key).to eq("issues/1-#{time}")
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in a new issue