Commit 682d744b authored by Patrick Bajao's avatar Patrick Bajao

Avoid N+1 calls for image_path when rendering commits

When rendering `projects/commits/commit` partial, it calls some
helper methods like `sprite_icon` and `clipboard_button`.

These methods call `sprite_icon_path` internally which is not that
slow (averaging around 6ms per call on my local machine). But when
called multiple times (e.g. rendering 100 commits), it adds up.

Some benchmark numbers for `Projects::MergeRequestsController#commits`
(average of 5 requests, with `RAILS_PROFILE` env var set to `true`:

- BEFORE: 1.051528
- AFTER: 0.684848

This shows around 35% improvement.
parent ec44773b
......@@ -28,10 +28,12 @@ module IconsHelper
end
def sprite_icon_path
# SVG Sprites currently don't work across domains, so in the case of a CDN
# we have to set the current path deliberately to prevent addition of asset_host
sprite_base_url = Gitlab.config.gitlab.url if ActionController::Base.asset_host
ActionController::Base.helpers.image_path('icons.svg', host: sprite_base_url)
@sprite_icon_path ||= begin
# SVG Sprites currently don't work across domains, so in the case of a CDN
# we have to set the current path deliberately to prevent addition of asset_host
sprite_base_url = Gitlab.config.gitlab.url if ActionController::Base.asset_host
ActionController::Base.helpers.image_path('icons.svg', host: sprite_base_url)
end
end
def sprite_file_icons_path
......
---
title: Avoid N+1 calls for image_path when rendering commits
merge_request: 36724
author:
type: performance
......@@ -22,8 +22,13 @@ RSpec.describe IconsHelper do
describe 'sprite_icon_path' do
it 'returns relative path' do
expect(sprite_icon_path)
.to eq icons_path
expect(sprite_icon_path).to eq(icons_path)
end
it 'only calls image_path once when called multiple times' do
expect(ActionController::Base.helpers).to receive(:image_path).once.and_call_original
2.times { sprite_icon_path }
end
context 'when an asset_host is set in the config it will return an absolute local URL' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment