Improve performance (part III)

[denisdefreyne]

Detailed description

This continues the progress made in improving performance.

The main learning: Set#any? is slow -- much better to use Set#empty? and negate.

I also realised that Sets can be avoided if the possible elements are limited and known ahead of time. Set is rather slow, and more verbose code can be a lot faster.

To do

• Tests
  • It’d be nice to have a test case for #props_for returning nil, but by now it’s less and less likely that there’s such old versions of Nanoc around that require this very tiny bit of backwards compatibility.

Related issues

Two other performance-focused PRs:

• Improve performance of outdatedness checker #1602
• Improve performance (part II) #1603

[DivineDominion]

@denisdefreyne I'm curious: do you have a hunch why Set#any? is slow? Once you have everything hashed, which might take a while sure, the hash lookup should be super quick.

[denisdefreyne]

Set#any? is slow because Set does not implement #any? directly, but delegates to Enumerable#any? instead. Enumerable#any? seems to be implemented in a way in which it starts iterating over elements, and then breaks out after the first element, something like this:

module MyEnumerable
  def any?
    each do
      return true
    end
  
    false
  end
end

That is what the CPU profile seems to suggest, at least.

It also explains the significance difference in performance. Here’s a benchmark:

Warming up --------------------------------------
                any?   276.965k i/100ms
             !empty?     3.323M i/100ms
Calculating -------------------------------------
                any?      2.756M (± 2.6%) i/s -     13.848M in   5.027698s
             !empty?     33.343M (± 0.2%) i/s -    169.469M in   5.082677s

Comparison:
             !empty?: 33342764.7 i/s
                any?:  2756310.9 i/s - 12.10x  (± 0.00) slower

Benchmark implementation:

require 'benchmark/ips'
require 'set'

set = Set.new
100.times { set << rand }

Benchmark.ips do |x|
  x.report("any?") do |times|
    i = 0
    while i < times
      set.any?
      i += 1
    end
  end

  x.report("!empty?") do |times|
    i = 0
    while i < times
      !set.empty?
      i += 1
    end
  end

  x.compare!
end

[denisdefreyne]

One more thing: #any? also returns false for non-empty collections that contain falsy elements: [false, nil].any? is false.

So, switching from xyz.any? to !xyz.empty? is not only faster, but also more correct. (Though in Nanoc’s case, the correctness wasn’t an issue.)