Commit e33235df authored by Igor Drozdov's avatar Igor Drozdov

Cache TreeSummary response for logs_tree

Response of TreeSummary is the same for [project_id, sha, offset]
We can cache it
parent e6f013c1
......@@ -40,29 +40,25 @@ class Projects::RefsController < Projects::ApplicationController
end
def logs_tree
summary = ::Gitlab::TreeSummary.new(
@commit,
@project,
path: @path,
offset: params[:offset],
limit: 25
)
@logs, commits = summary.summarize
@more_log_url = more_url(summary.next_offset) if summary.more?
tree_summary = ::Gitlab::TreeSummary.new(
@commit, @project, path: @path, offset: params[:offset], limit: 25)
respond_to do |format|
format.html { render_404 }
format.json do
response.headers["More-Logs-Url"] = @more_log_url if summary.more?
response.headers["More-Logs-Offset"] = summary.next_offset if summary.more?
render json: @logs
logs, next_offset = tree_summary.fetch_logs
response.headers["More-Logs-Offset"] = next_offset if next_offset
render json: logs
end
# The commit titles must be rendered and redacted before being shown.
# Doing it here allows us to apply performance optimizations that avoid
# N+1 problems
# Deprecated due to https://gitlab.com/gitlab-org/gitlab/-/issues/36863
# Will be removed soon https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29895
format.js do
@logs, commits = tree_summary.summarize
@more_log_url = more_url(tree_summary.next_offset) if tree_summary.more?
prerender_commit_full_titles!(commits)
end
end
......
---
title: Cache TreeSummary response for logs_tree
merge_request: 29828
author:
type: performance
......@@ -6,6 +6,9 @@ module Gitlab
include ::Gitlab::Utils::StrongMemoize
CACHE_EXPIRE_IN = 1.hour
MAX_OFFSET = 2**31
attr_reader :commit, :project, :path, :offset, :limit
attr_reader :resolved_commits
......@@ -16,7 +19,7 @@ module Gitlab
@project = project
@path = params.fetch(:path, nil).presence
@offset = params.fetch(:offset, 0).to_i
@offset = [params.fetch(:offset, 0).to_i, MAX_OFFSET].min
@limit = (params.fetch(:limit, 25) || 25).to_i
# Ensure that if multiple tree entries share the same last commit, they share
......@@ -43,6 +46,17 @@ module Gitlab
[summary, commits]
end
def fetch_logs
cache_key = ['projects', project.id, 'logs', commit.id, path, offset]
Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRE_IN) do
logs, _ = summarize
new_offset = next_offset if more?
[logs.as_json, new_offset]
end
end
# Does the tree contain more entries after the given offset + limit?
def more?
all_contents[next_offset].present?
......
......@@ -12,25 +12,27 @@ describe Projects::RefsController do
end
describe 'GET #logs_tree' do
let(:path) { 'foo/bar/baz.html' }
def default_get(format = :html)
get :logs_tree,
params: {
namespace_id: project.namespace.to_param,
project_id: project,
id: 'master',
path: 'foo/bar/baz.html'
path: path
},
format: format
end
def xhr_get(format = :html)
def xhr_get(format = :html, params = {})
get :logs_tree, params: {
namespace_id: project.namespace.to_param,
project_id: project,
id: 'master',
path: 'foo/bar/baz.html',
path: path,
format: format
}, xhr: true
}.merge(params), xhr: true
end
it 'never throws MissingTemplate' do
......@@ -52,13 +54,27 @@ describe Projects::RefsController do
expect(response).to be_successful
end
it 'renders JSON' do
expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original
context 'when json is requested' do
it 'renders JSON' do
expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original
xhr_get(:json)
xhr_get(:json)
expect(response).to be_successful
expect(json_response).to be_kind_of(Array)
expect(response).to be_successful
expect(json_response).to be_kind_of(Array)
end
it 'caches tree summary data', :use_clean_rails_memory_store_caching do
expect_next_instance_of(::Gitlab::TreeSummary) do |instance|
expect(instance).to receive_messages(summarize: ['logs'], next_offset: 50, more?: true)
end
xhr_get(:json, offset: 25)
cache_key = "projects/#{project.id}/logs/#{project.commit.id}/#{path}/25"
expect(Rails.cache.fetch(cache_key)).to eq(['logs', 50])
expect(response.headers['More-Logs-Offset']).to eq(50)
end
end
end
end
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