Skip symbolize_keys in CI::Variables::Item when keys are symbols

Furkan Ayhanrequested to merge
512876-track-symbolize_keys-to-optimize-ci-variables-collection-item into master

What does this MR do and why?

Optimizes Gitlab::Ci::Variables::Collection::Item.fabricate by avoiding unnecessary symbolize_keys calls when hash keys are already symbols.

symbolize_keys is called on every variable hash passed to Item.fabricate, which is one of the hottest paths during pipeline creation. In practice, the vast majority of callers already pass hashes with symbol keys, so symbolize_keys creates a redundant copy of the hash each time. This MR introduces fast_symbolize_keys which checks if keys are already symbols and skips the allocation in that case.

When string keys are detected, we log the occurrence to verify our assumption. After monitoring production, if no string keys are found, we can simplify the code further.

Plan

The plan is the fast iteration; deploy->enable->track->remove.

Then, do the same for dup allocation we discussed in !225902 (closed).

References

Memory profiling results 

# RAILS_PROFILE=true GITALY_DISABLE_REQUEST_LIMITS=true rails console

ActiveRecord::Base.logger = nil
@project = Project.find_by_full_path('gitlab-org/gitlab-clone')
@user = @project.first_owner
@merge_request = MergeRequest.find(153)
@merge_request_params = { allow_duplicate: true }

require 'memory_profiler'

def run
  Gitlab::SafeRequestStore.ensure_request_store do
    pipeline = ::MergeRequests::CreatePipelineService.new(
      project: @project, current_user: @user, params: @merge_request_params
    ).execute(@merge_request).payload

    raise "stop!" unless pipeline.created_successfully?
  end
end

run

report = MemoryProfiler.report do
  run
end

2.times do
  Gitlab::SafeRequestStore.ensure_request_store do
    ::Ci::DestroyPipelineService.new(@project, @user).execute(Ci::Pipeline.last)
  end
end

io = StringIO.new
report.pretty_print(io, detailed_report: true, scale_bytes: true, normalize_paths: true)
puts io.string

Overview

Metric Master FF Enabled FF Removed Delta vs Master
Total allocated 1.45 GB (14.27M obj) 1.46 GB (14.62M obj) 1.39 GB (13.91M obj) -60 MB (-4.1%)
Total retained 345.69 MB (2.88M obj) 345.69 MB (2.88M obj) 345.69 MB (2.88M obj) No change

Key improvement: symbolize_keys elimination

activesupport/core_ext/hash/keys.rb allocated memory dropped from 68.68 MB → 12.40 MB (-56.28 MB, -82%). This is the direct result of skipping redundant hash duplication.

Allocated memory by top gems

Gem Master FF Removed Delta
gitlab/lib 826.41 MB 826.41 MB 0
activesupport-7.2.3 239.17 MB 182.87 MB -56.30 MB (-24%)

Allocated memory by top files

File Master FF Removed Delta
variables/collection/item.rb 277.13 MB 277.13 MB 0
variables/collection.rb 174.29 MB 174.29 MB 0
hash/keys.rb 68.68 MB 12.40 MB -56.28 MB

Allocated memory by top classes

Class Master FF Removed Delta
Hash 546.02 MB 489.73 MB -56.29 MB (-10%)
String 273.55 MB 273.53 MB 0
Array 235.43 MB 235.43 MB 0
Collection::Item 106.03 MB 106.03 MB 0

Note on feature flag overhead

With the feature flag enabled, total allocation is 1.42 GB — ~30 MB more than the FF-removed version (1.39 GB). This overhead comes from the FF check itself and the hash.first call creating a temporary [key, value] array per invocation (~28 MB in extra Array allocations). This disappears once the FF is removed.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Furkan Ayhan

Merge request reports