diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-04-24 16:01:37 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-04-24 16:01:37 +0000 |
commit | 9d17ad1048779c3e8ff5f8af4d6497a1140cfe33 (patch) | |
tree | e1be814be515e32c561f193f6547768e1a8c7b26 | |
parent | 53301454163bba1b887bbbdba9f91420e951bda1 (diff) | |
parent | a0edaa9210642f23ef3ea4984c6d6f77cbbba878 (diff) | |
download | gitlab-ce-9d17ad1048779c3e8ff5f8af4d6497a1140cfe33.tar.gz |
Merge branch 'sh-optimize-duplicate-routable-full-path' into 'master'
Cache Routable#full_path in RequestStore to reduce duplicate route loads
See merge request !10872
-rw-r--r-- | app/models/concerns/routable.rb | 15 | ||||
-rw-r--r-- | changelogs/unreleased/sh-optimize-duplicate-routable-full-path.yml | 4 | ||||
-rw-r--r-- | spec/models/concerns/routable_spec.rb | 18 |
3 files changed, 35 insertions, 2 deletions
diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index aca99feee53..b28e05d0c28 100644 --- a/app/models/concerns/routable.rb +++ b/app/models/concerns/routable.rb @@ -163,7 +163,20 @@ module Routable end end + # Every time `project.namespace.becomes(Namespace)` is called for polymorphic_path, + # a new instance is instantiated, and we end up duplicating the same query to retrieve + # the route. Caching this per request ensures that even if we have multiple instances, + # we will not have to duplicate work, avoiding N+1 queries in some cases. def full_path + return uncached_full_path unless RequestStore.active? + + key = "routable/full_path/#{self.class.name}/#{self.id}" + RequestStore[key] ||= uncached_full_path + end + + private + + def uncached_full_path if route && route.path.present? @full_path ||= route.path else @@ -173,8 +186,6 @@ module Routable end end - private - def full_name_changed? name_changed? || parent_changed? end diff --git a/changelogs/unreleased/sh-optimize-duplicate-routable-full-path.yml b/changelogs/unreleased/sh-optimize-duplicate-routable-full-path.yml new file mode 100644 index 00000000000..b1ef00f09b2 --- /dev/null +++ b/changelogs/unreleased/sh-optimize-duplicate-routable-full-path.yml @@ -0,0 +1,4 @@ +--- +title: Cache Routable#full_path in RequestStore to reduce duplicate route loads +merge_request: +author: diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index f191605dbdb..221647d7a48 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -194,6 +194,24 @@ describe Group, 'Routable' do it { expect(group.full_path).to eq(group.path) } it { expect(nested_group.full_path).to eq("#{group.full_path}/#{nested_group.path}") } + + context 'with RequestStore active' do + before do + RequestStore.begin! + end + + after do + RequestStore.end! + RequestStore.clear! + end + + it 'does not load the route table more than once' do + expect(group).to receive(:uncached_full_path).once.and_call_original + + 3.times { group.full_path } + expect(group.full_path).to eq(group.path) + end + end end describe '#full_name' do |