Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor ActiveSupport subscriber #2564

Closed
wants to merge 1 commit into from

Conversation

kaylareopelle
Copy link
Contributor

@kaylareopelle kaylareopelle commented Apr 15, 2024

This PR is an experiment to try to speed up the ActiveSupport instrumentation.

Where strings were used, constants are now used instead.

There's a small improvement to use match? + byteslice over =~ + $1

match? performance benchmark:

# frozen_string_literal: true

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'benchmark-ips'
  gem 'benchmark-ipsa'
end

PATTERN = /\Acache_([^\.]*)\.active_support\z/

original_query_names = %w[
  cache_read.active_support
  cache_read_multi.active_support
  cache_generate.active_support
  cache_fetch_hit.active_support
  cache_write.active_support
  cache_write_multi.active_support
  cache_increment.active_support
  cache_decrement.active_support
  cache_delete.active_support
  cache_delete_multi.active_support
  cache_delete_matched.active_support
  cache_cleanup.active_support
  cache_prune.active_support
  cache_exist?.active_support
]

def equal_tilde(k)
  PATTERN =~ k
end

def matches?(k)
  PATTERN.match?(k)
end

def matchy_match(k)
  PATTERN.match(k) do |m|
    true
  end
end

OOPS = 'oops'

Benchmark.ipsa do |x|
  x.report('=~')         { original_query_names.each { |n| raise OOPS unless equal_tilde(n) } }
  x.report('match?') { original_query_names.each { |n| raise OOPS unless matches?(n) } }
  x.report('match')   { original_query_names.each { |n| raise OOPS unless matchy_match(n) } }
  x.compare!
end


# Allocations -------------------------------------
#                   =~      15/0  alloc/ret        0/0  strings/ret
#               match?       0/0  alloc/ret        0/0  strings/ret
#                match      14/0  alloc/ret        0/0  strings/ret
# Warming up --------------------------------------
#                   =~    10.294k i/100ms
#               match?    26.665k i/100ms
#                match    10.279k i/100ms
# Calculating -------------------------------------
#                   =~    119.819k (± 9.7%) i/s -    597.052k
#               match?    282.765k (± 9.0%) i/s -      1.413M
#                match    105.432k (± 8.1%) i/s -    524.229k

# Comparison:
#               match?:   282765.3 i/s
#                   =~:   119818.9 i/s - 2.36x slower
#                match:   105432.3 i/s - 2.68x slower

Comparing match? with byteslice alongside =~ with $1

# frozen_string_literal: true

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'benchmark-ips'
  gem 'benchmark-ipsa'
end

PATTERN = /\Acache_([^\.]*)\.active_support\z/

NAMES = %w[
  cache_read.active_support
  cache_read_multi.active_support
  cache_generate.active_support
  cache_fetch_hit.active_support
  cache_write.active_support
  cache_write_multi.active_support
  cache_increment.active_support
  cache_decrement.active_support
  cache_delete.active_support
  cache_delete_multi.active_support
  cache_delete_matched.active_support
  cache_cleanup.active_support
  cache_prune.active_support
  cache_exist?.active_support
]

Benchmark.ipsa do |x|
  x.report("match? with byteslice") { NAMES.each { |k| PATTERN.match?(k) ? k.byteslice(6..-16) : UNKNOWN } }
  x.report("=~ with $1")            { NAMES.each { |k| PATTERN =~ k ? $1 : UNKNOWN } }

  x.compare!
end

# Allocations -------------------------------------
# match? with byteslice
#                           15/0  alloc/ret       14/0  strings/ret
#           =~ with $1      28/1  alloc/ret       14/0  strings/ret
# Warming up --------------------------------------
# match? with byteslice
#                         18.999k i/100ms
#           =~ with $1    10.607k i/100ms
# Calculating -------------------------------------
# match? with byteslice
#                         203.823k (±10.6%) i/s -      1.007M
#           =~ with $1    101.897k (±18.1%) i/s -    498.529k

# Comparison:
# match? with byteslice:   203823.3 i/s
#           =~ with $1:   101896.7 i/s - 2.00x slower

* Make the string in the metric name a constant
* Use NewRelic::SLASH instead of the string
* Refactor METHOD_NAME_MAPPING to use faster methods
Copy link

SimpleCov Report

Coverage Threshold
Line 93.77% 93%
Branch 71.48% 71%

@kaylareopelle
Copy link
Contributor Author

We're running a different test to improve the subscriber. See #2610.

@kaylareopelle kaylareopelle deleted the active-support-perf-adjustments branch May 2, 2024 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

1 participant