From 79dd92c8189600f24a29fe2da8d391cdb021f8fd Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Sat, 11 May 2019 07:06:44 -0500 Subject: [PATCH] Optimise upload path calls String#underscore isn't particularly slow, but it's possible for us to call it many times in a users autocomplete request, with mostly-static values ('User', 'Group', etc.). We can memoise this and save a surprising amount of time (around 10% of the total request time in some cases). --- app/controllers/uploads_controller.rb | 2 +- app/models/application_record.rb | 4 ++++ app/uploaders/attachment_uploader.rb | 2 +- app/uploaders/avatar_uploader.rb | 2 +- app/uploaders/personal_file_uploader.rb | 2 +- .../make-autocomplete-faster-with-lots-of-results.yml | 5 +++++ spec/controllers/concerns/send_file_upload_spec.rb | 3 ++- spec/factories/uploads.rb | 2 +- spec/models/application_record_spec.rb | 6 ++++++ spec/uploaders/object_storage_spec.rb | 2 +- 10 files changed, 23 insertions(+), 7 deletions(-) create mode 100644 changelogs/unreleased/make-autocomplete-faster-with-lots-of-results.yml diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 060b09f015c..5d28635232b 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -45,7 +45,7 @@ class UploadsController < ApplicationController when Appearance true else - permission = "read_#{model.class.to_s.underscore}".to_sym + permission = "read_#{model.class.underscore}".to_sym can?(current_user, permission, model) end diff --git a/app/models/application_record.rb b/app/models/application_record.rb index d1d01368972..0979d03f6e6 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -41,4 +41,8 @@ class ApplicationRecord < ActiveRecord::Base find_or_create_by(*args) end end + + def self.underscore + Gitlab::SafeRequestStore.fetch("model:#{self}:underscore") { self.to_s.underscore } + end end diff --git a/app/uploaders/attachment_uploader.rb b/app/uploaders/attachment_uploader.rb index 0a166335b4e..b488bba00e9 100644 --- a/app/uploaders/attachment_uploader.rb +++ b/app/uploaders/attachment_uploader.rb @@ -9,6 +9,6 @@ class AttachmentUploader < GitlabUploader private def dynamic_segment - File.join(model.class.to_s.underscore, mounted_as.to_s, model.id.to_s) + File.join(model.class.underscore, mounted_as.to_s, model.id.to_s) end end diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb index c0165759203..9af59b0aceb 100644 --- a/app/uploaders/avatar_uploader.rb +++ b/app/uploaders/avatar_uploader.rb @@ -25,6 +25,6 @@ class AvatarUploader < GitlabUploader private def dynamic_segment - File.join(model.class.to_s.underscore, mounted_as.to_s, model.id.to_s) + File.join(model.class.underscore, mounted_as.to_s, model.id.to_s) end end diff --git a/app/uploaders/personal_file_uploader.rb b/app/uploaders/personal_file_uploader.rb index 272837aa6ce..f4697d898af 100644 --- a/app/uploaders/personal_file_uploader.rb +++ b/app/uploaders/personal_file_uploader.rb @@ -20,7 +20,7 @@ class PersonalFileUploader < FileUploader def self.model_path_segment(model) return 'temp/' unless model - File.join(model.class.to_s.underscore, model.id.to_s) + File.join(model.class.underscore, model.id.to_s) end def object_store diff --git a/changelogs/unreleased/make-autocomplete-faster-with-lots-of-results.yml b/changelogs/unreleased/make-autocomplete-faster-with-lots-of-results.yml new file mode 100644 index 00000000000..daeefd3ffd7 --- /dev/null +++ b/changelogs/unreleased/make-autocomplete-faster-with-lots-of-results.yml @@ -0,0 +1,5 @@ +--- +title: Improve performance of users autocomplete when there are lots of results +merge_request: +author: +type: performance diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index 8408578a7db..a3ce08f736c 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -1,3 +1,4 @@ +# coding: utf-8 # frozen_string_literal: true require 'spec_helper' @@ -13,7 +14,7 @@ describe SendFileUpload do # user/:id def dynamic_segment - File.join(model.class.to_s.underscore, model.id.to_s) + File.join(model.class.underscore, model.id.to_s) end end end diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb index 7256f785e1f..426abdc2a6c 100644 --- a/spec/factories/uploads.rb +++ b/spec/factories/uploads.rb @@ -13,7 +13,7 @@ FactoryBot.define do end # this needs to comply with RecordsUpload::Concern#upload_path - path { File.join("uploads/-/system", model.class.to_s.underscore, mount_point.to_s, 'avatar.jpg') } + path { File.join("uploads/-/system", model.class.underscore, mount_point.to_s, 'avatar.jpg') } trait :personal_snippet_upload do uploader "PersonalFileUploader" diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index cc90a998d3f..74573d0941c 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -52,4 +52,10 @@ describe ApplicationRecord do expect { Suggestion.find_or_create_by!(note: nil) }.to raise_error(ActiveRecord::RecordInvalid) end end + + describe '.underscore' do + it 'returns the underscored value of the class as a string' do + expect(MergeRequest.underscore).to eq('merge_request') + end + end end diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index a62830c35f1..6bad5d49b1c 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -12,7 +12,7 @@ class Implementation < GitlabUploader # user/:id def dynamic_segment - File.join(model.class.to_s.underscore, model.id.to_s) + File.join(model.class.underscore, model.id.to_s) end end -- GitLab