Commit a11551c2 authored by Stan Hu's avatar Stan Hu

Speed up path generation with build artifacts

Previously it would take seconds to render the pipeline index JSON
(/gitlab-org/gitlab/pipelines.json) in the GitLab project. Apparently,
we render over 5000 paths for `BuildArtifactEntity`, and generation of
these paths dominated the request time.

The Rails path helpers are slow because they check the namespace and
project parameters against large regular expressions, which include
`TOP_LEVEL_ROUTES` and `PROJECT_WILDCARD_ROUTES`, respectively.

We can speed this up by generating the paths directly with
strings. While this isn't ideal, this results in a 10x improvement in
speed:

```
Calculating -------------------------------------
                orig    86.000  i/100ms
                 new     1.272k i/100ms
-------------------------------------------------
                orig      1.343k (±13.8%) i/s -      6.536k
                 new     13.756k (±15.3%) i/s -     67.416k

Comparison:
                 new:    13756.3 i/s
                orig:     1343.3 i/s - 10.24x slower
```

To prevent these helper methods from breaking if the routes ever change,
we add tests to ensure that the results are the same.

Closes https://gitlab.com/gitlab-org/gitlab/issues/121929
parent 7b2927dd
...@@ -153,6 +153,29 @@ module GitlabRoutingHelper ...@@ -153,6 +153,29 @@ module GitlabRoutingHelper
# Artifacts # Artifacts
# Rails path generators are slow because they need to do large regex comparisons
# against the arguments. We can speed this up 10x by generating the strings directly.
# /*namespace_id/:project_id/-/jobs/:job_id/artifacts/download(.:format)
def fast_download_project_job_artifacts_path(project, job)
expose_fast_artifacts_path(project, job, :download)
end
# /*namespace_id/:project_id/-/jobs/:job_id/artifacts/keep(.:format)
def fast_keep_project_job_artifacts_path(project, job)
expose_fast_artifacts_path(project, job, :keep)
end
# /*namespace_id/:project_id/-/jobs/:job_id/artifacts/browse(/*path)
def fast_browse_project_job_artifacts_path(project, job)
expose_fast_artifacts_path(project, job, :browse)
end
def expose_fast_artifacts_path(project, job, action)
path = "#{project.full_path}/-/jobs/#{job.id}/artifacts/#{action}"
Gitlab::Utils.append_path(Gitlab.config.gitlab.relative_url_root, path)
end
def artifacts_action_path(path, project, build) def artifacts_action_path(path, project, build)
action, path_params = path.split('/', 2) action, path_params = path.split('/', 2)
args = [project, build, path_params] args = [project, build, path_params]
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
class BuildArtifactEntity < Grape::Entity class BuildArtifactEntity < Grape::Entity
include RequestAwareEntity include RequestAwareEntity
include GitlabRoutingHelper
expose :name do |job| expose :name do |job|
job.name job.name
...@@ -11,15 +12,15 @@ class BuildArtifactEntity < Grape::Entity ...@@ -11,15 +12,15 @@ class BuildArtifactEntity < Grape::Entity
expose :artifacts_expire_at, as: :expire_at expose :artifacts_expire_at, as: :expire_at
expose :path do |job| expose :path do |job|
download_project_job_artifacts_path(project, job) fast_download_project_job_artifacts_path(project, job)
end end
expose :keep_path, if: -> (*) { job.has_expiring_artifacts? } do |job| expose :keep_path, if: -> (*) { job.has_expiring_artifacts? } do |job|
keep_project_job_artifacts_path(project, job) fast_keep_project_job_artifacts_path(project, job)
end end
expose :browse_path do |job| expose :browse_path do |job|
browse_project_job_artifacts_path(project, job) fast_browse_project_job_artifacts_path(project, job)
end end
private private
......
---
title: Speed up path generation with build artifacts
merge_request: 22257
author:
type: performance
...@@ -113,6 +113,29 @@ describe GitlabRoutingHelper do ...@@ -113,6 +113,29 @@ describe GitlabRoutingHelper do
end end
end end
context 'artifacts' do
let_it_be(:project) { create(:project) }
let_it_be(:job) { create(:ci_build, project: project, name: 'test:job', artifacts_expire_at: 1.hour.from_now) }
describe '#fast_download_project_job_artifacts_path' do
it 'matches the Rails download path' do
expect(fast_download_project_job_artifacts_path(project, job)).to eq(download_project_job_artifacts_path(project, job))
end
end
describe '#fast_keep_project_job_artifacts_path' do
it 'matches the Rails keep path' do
expect(fast_keep_project_job_artifacts_path(project, job)).to eq(keep_project_job_artifacts_path(project, job))
end
end
describe '#fast_browse_project_job_artifacts_path' do
it 'matches the Rails browse path' do
expect(fast_browse_project_job_artifacts_path(project, job)).to eq(browse_project_job_artifacts_path(project, job))
end
end
end
context 'snippets' do context 'snippets' do
let_it_be(:personal_snippet) { create(:personal_snippet) } let_it_be(:personal_snippet) { create(:personal_snippet) }
let_it_be(:project_snippet) { create(:project_snippet) } let_it_be(:project_snippet) { create(:project_snippet) }
......
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