Skip to content

Resolve "Improve performance of Gitlab::Utils::InlineHash"

What does this MR do?

Improves performance when an optional prefix param is not passed as an argument to Gitlab::Utils::InlineHash.

Old vs New Code

      # Original
      def merge_keys_(hash, prefix: nil, connector: '.')
        result = {}
        base_prefix = prefix ? "#{prefix}#{connector}" : ''
        pairs = hash.map { |key, value| ["#{base_prefix}#{key}", value] }

        until pairs.empty?
          key, value = pairs.shift

          if value.is_a?(Hash)
            value.each { |k, v| pairs.unshift ["#{key}#{connector}#{k}", v] }
          else
            result[key] = value
          end
        end

        result
      end

      # New
      def merge_keys(hash, prefix: nil, connector: '.')
        result = {}
        base = prefix ? "#{prefix}#{connector}" : ''
        pairs =
          if prefix
            hash.map { |key, value| ["#{base}#{key}", value] }
          else
            hash.to_a
          end

        until pairs.empty?
          key, value = pairs.shift

          if value.is_a?(Hash)
            value.each { |k, v| pairs.unshift ["#{key}#{connector}#{k}", v] }
          else
            result[key] = value
          end
        end

        result
      end

Benchmarks

Benchmarks for small

Calculating -------------------------------------
small iter merge_keys
                        25.688k i/100ms
small iter merge_keys_
                        20.931k i/100ms
-------------------------------------------------
small iter merge_keys
                        330.485k (±18.4%) i/s -      1.618M
small iter merge_keys_
                        258.005k (±16.3%) i/s -      1.277M

Comparison:
small iter merge_keys:   330484.7 i/s
small iter merge_keys_:   258005.3 i/s - 1.28x slower

================================================================================

Benchmarks for medium

Calculating -------------------------------------
medium iter merge_keys
                         4.206k i/100ms
medium iter merge_keys_
                         1.470k i/100ms
-------------------------------------------------
medium iter merge_keys
                         44.461k (±17.0%) i/s -    218.712k
medium iter merge_keys_
                         14.314k (±13.5%) i/s -     70.560k

Comparison:
medium iter merge_keys:    44461.1 i/s
medium iter merge_keys_:    14314.5 i/s - 3.11x slower

================================================================================

Benchmarks for large

Calculating -------------------------------------
large iter merge_keys
                       249.000  i/100ms
large iter merge_keys_
                        76.000  i/100ms
-------------------------------------------------
large iter merge_keys
                          2.521k (± 7.5%) i/s -     12.699k
large iter merge_keys_
                        765.837  (± 8.5%) i/s -      3.876k

Comparison:
large iter merge_keys:     2521.0 i/s
large iter merge_keys_:      765.8 i/s - 3.29x slower

================================================================================

Benchmarks for huge

Calculating -------------------------------------
huge iter merge_keys    49.000  i/100ms
huge iter merge_keys_
                        14.000  i/100ms
-------------------------------------------------
huge iter merge_keys    481.732  (± 6.2%) i/s -      2.401k
huge iter merge_keys_
                        146.943  (± 8.8%) i/s -    742.000

Comparison:
huge iter merge_keys:      481.7 i/s
huge iter merge_keys_:      146.9 i/s - 3.28x slower

================================================================================

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team

Part of Issue #32490 (closed)

Edited by Allison Browne

Merge request reports