Replace runtime defined? calls with const_defined?

[etehtsea]

Using defined? in runtime is affecting app performance.
ActiveSupport::Dependencies which automatically hooks into const_missing
makes situation even worse.

# jruby-9.0.3.0
require 'benchmark/ips'
# Uncomment for the second bench
# require 'active_support/dependencies'

Benchmark.ips do |x|
  x.config(warmup: 30, time: 20)
  x.report("const_defined?") do
    Object.const_defined?(:ActionDispatch) &&
      ::ActionDispatch.const_defined?(:Http) &&
      ::ActionDispatch::Http.const_defined?(:ParameterFilter)
  end
  x.report("defined?") { defined?(ActionDispatch::Http::ParameterFilter) }
  x.compare!
end

Comparison:
      const_defined?:  5621473.2 i/s
            defined?:    13207.5 i/s - 425.63x slower

Comparison (with ActiveSupport::Dependencies):
    const_defined?:  6011560.2 i/s
            defined?:     4278.5 i/s - 1405.06x slower

Real app flamegraph (https://github.com/SamSaffron/flamegraph) profiling results:

… activesupport-4.2.4/lib/active_support/dependencies.rb:184:in `const_missing'   (40 samples - 51.28%)
… newrelic_rpm-3.14.0.305/lib/new_relic/agent/parameter_filtering.rb:17:in `filter_using_rails'   (12 samples - 15.38%)

… activesupport-4.2.4/lib/active_support/dependencies.rb:184:in `const_missing'   (40 samples - 51.28%)
… newrelic_rpm-3.14.0.305/lib/new_relic/agent/instrumentation/middleware_proxy.rb:37:in `is_sinatra_app?' (27 samples - 34.62%)

[t3hk0d3]

👍

[kwugirl]

Hi @etehtsea @t3hk0d3 -- thanks for submitting those benchmarks, and apologies of the delay in a response! I tried it out and it looks like there is a big difference when run on jruby but not on MRI--has that been your experience as well?

Are these the only two changes that you would need, or were they just a demonstration of the kinds of changes to be made? The agent uses defined? quite a bit throughout the codebase, and as your code shows, using const_defined? requires stepping through the different levels of namespaces, which comes with its own cost.

[kwugirl]

Hi @etehtsea @t3hk0d3 -- wanted to check in again and see if this was still a concern for you? See my comments above.

[etehtsea]

I think this issue should be addressed upstream and/or fixed somehow globally (for the whole rpm codebase).