Don't call Warning.warn unless the category is enabled

[tenderlove]

The warning category should be enabled if we want to call Warning.warn.

This commit speeds up the following benchmark:

eval "def test; " +
  1000.times.map { "'  '.chomp!" }.join(";") + "; end"

def run_benchmark count
  i = 0
  while i < count
    start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
    yield
    ms = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
    puts "itr ##{i}: #{(ms * 1000).to_i}ms"
    i += 1
  end
end

run_benchmark(25) do
  250.times do
    test
  end
end

On master this runs at about 92ms per iteration. With this patch, it is 7ms per iteration.

[jeremyevans]

This should be a safe optimization if Warning.warn is not overridden. However, if Warning.warn is overridden, this silences a warning that Warning.warn would previously get called with. Maybe that isn't a problem, but I think this change would need to be discussed at a developer meeting, unless it was limited to cases where Warning.warn was not overridden.

[tenderlove]

This should be a safe optimization if Warning.warn is not overridden. However, if Warning.warn is overridden, this silences a warning that Warning.warn would previously get called with. Maybe that isn't a problem, but I think this change would need to be discussed at a developer meeting, unless it was limited to cases where Warning.warn was not overridden.

Ya, I think we could do monkey patch detection and handle that. I'll make a redmine ticket for it. As it stands, this is unfortunately causing a performance regression in some of the benchmarks we're watching (see the etanni benchmark here)

[nobu]

Also rb_category_warning?

[byroot]

To me this is a bug fix, if I set Warning[:performance] = false, I don't want my Warning.warn patch called for performance warnings.

[tenderlove]

Also rb_category_warning?

Yes, thanks @nobu

[tenderlove]

According to @byroot, this restores the behavior from Ruby 2.7.8 (see this ticket). In other words, I think this is a bugfix.

[tenderlove]

@jeremyevans this seems like a bugfix to me, but I'd like to know your opinion 😄

[jeremyevans]

Based on the discussion at https://bugs.ruby-lang.org/issues/20573, I agree this is a bug fix.