slow custom facts

[otheus]

The package_provider.rb facts can be optimized simply by moving the require lines into the setcode block. This reduces stand-alone puppet runs by 0.5s on modern systems. (On puppet-agent runs, no such savings are observed, probably because the libraries are already required by the puppet agent itself. Apparently, the facter command evaluates the outer and inner blocks in different contexts.

Interestingly, moving the corresponding lines in service_provider.rb has no measurable performance benefit.

Current version

# bench 3 "facter -p >/dev/null"
Run 1
Run 2
Run 3
1665392723.165300291
1665392730.070952629
2.3018

With recommended change

# bench 3 "facter -p >/dev/null"
Run 1
Run 2
Run 3
1665392652.861524963
1665392661.744424272
2.9609

[ekohl]

Could you come up with a patch?

[otheus]

@ekohl I'm really not interested in setting up an entire development environment to make these simple patches. In the past, I had to add test cases without having any idea how to run test cases. In our case, it's about 3 or 4 different files where those require statements simply need to be moved down.

[kenyon]

All you need to do is fork and clone this repository, make the change, commit, push, create pull request.

[otheus]

Isn't that wishful thinking / bad process design? I do that, then someone says, "where are the test cases"? If I don't do that, who writes the test cases?

[ekohl]

For require statements it's hard to write tests so in this case I'm not sure what could actually be tested anyway.

Having said that: I can't reproduce your results.

First I try with a baseline: just facter.

$ bench 'facter'
benchmarking facter
time                 412.0 ms   (392.5 ms .. 452.3 ms)
                     0.999 R²   (0.998 R² .. 1.000 R²)
mean                 432.4 ms   (419.1 ms .. 452.2 ms)
std dev              19.44 ms   (4.462 ms .. 25.49 ms)
variance introduced by outliers: 19% (moderately inflated)

Then in a checkout of this repo (at e1977c552f8c013f0f3928c246a8e348952dc844):

$ FACTERLIB=lib/facter bench 'facter -p'
benchmarking facter -p
time                 3.043 s    (3.020 s .. 3.058 s)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 3.010 s    (2.991 s .. 3.021 s)
std dev              18.18 ms   (7.728 ms .. 23.97 ms)
variance introduced by outliers: 19% (moderately inflated)

Then I move the code:

diff --git a/lib/facter/package_provider.rb b/lib/facter/package_provider.rb
index c5c1ef85..049fc997 100644
--- a/lib/facter/package_provider.rb
+++ b/lib/facter/package_provider.rb
@@ -9,13 +9,13 @@
 #
 # Caveats:
 #
-require 'puppet/type'
-require 'puppet/type/package'
 
 # These will be nil if Puppet is not available.
 Facter.add(:package_provider) do
   # Instantiates a dummy package resource and return the provider
   setcode do
+    require 'puppet/type'
+    require 'puppet/type/package'
     Puppet::Type.type(:package).newpackage(name: 'dummy', allow_virtual: 'true')[:provider].to_s
   end
 end

And again run the test:

$ FACTERLIB=lib/facter bench 'facter -p'
benchmarking facter -p
time                 3.108 s    (3.096 s .. 3.122 s)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 3.055 s    (3.010 s .. 3.075 s)
std dev              32.59 ms   (3.441 ms .. 41.48 ms)
variance introduced by outliers: 19% (moderately inflated)

From what I see it actually goes slightly up rather than down.

I tested this on Fedora 36, using Linux kernel 5.19.15-201.fc36.x86_64.

If you're not willing to submit a patch and I can't reproduce the benefit, should we then close this issue?

[otheus]

If you're not willing to submit a patch and I can't reproduce the benefit, should we then close this issue?

No, because I am required by my org to push local changes upstream, and the difference here is substantial and reproducible.

Our setup is significantly more complicated than your test scenario, but I imagine there are many variables.

I guess I will submit a patch. :)