Rewrite ConnectionPool for efficiency

[mattbrictson]

This is a rewrite of SSHKit's ConnectionPool. The advantages of this version are:

• Cleaner API
• Less mutex locking
• Asynchronous connection eviction/closing
• Faster (hopefully)

Cleaner API

# Old API
entry = pool.checkout(host, username, &Net::SSH.method(:start))
begin
  # do stuff with entry.connection
ensure
  pool.checkin(entry)
end

# New API
pool.with(Net::SSH.method(:start), host, username) do |connection|
  # do stuff with connection
end

Less mutex locking

Previously, every checkin and checkout would use a mutex to lock the entire pool every time. Now, each unique connection key has its own separate mutex, so any locking is done per host, and not for the entire pool.

Asynchronous connection eviction/closing

Previously, every checkin and checkout would trigger an expiration check for all connections in the pool. Thus every thread must potentially wait for connections used by other threads to be cleaned up and closed.

Now this logic is removed. Eviction happens in a separate background thread periodically sweeps the connections to identify and close stale ones.

Faster (hopefully)

In my testing the new pool is faster, but only marginally so. However this is in a artificial environment using sleep instead of real SSH commands.

@byroot – Can you test this branch and let me know how it performs against real servers?

TODO:

• Fix skipped unit tests
• CHANGELOG

[mattbrictson]

This PR is related to #326.

[byroot]

So we're back to either disable the pooling entirely, or to have to prune connection?

I don't know if I'm alone, but in our case a full deploy take about 4 to 5 minutes and we do it from inside the datacenter, so pruning idle connections is really not a concern.

Any way we could disable it without disabling pooling?

[mattbrictson]

My goal was to improve efficiency across the board, rather than add conditionals as workarounds. The point is that If you just set idle_timeout to a high value, you shouldn't pay any performance penalty, because a mutex is no longer being used for pruning.

But, like I said, the proof will be in the numbers.

[byroot]

Yeah, we'll see. I didn't have time to test yet.

[byroot]

So the benchmark is fairly similar to before the change:

As for the profiles, cold:

==================================
  Mode: wall(1000)
  Samples: 2642 (8.61% miss rate)
  GC: 278 (10.52%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
       312  (11.8%)         304  (11.5%)     SSHKit::Runner::Parallel#execute
       228   (8.6%)         228   (8.6%)     block (3 levels) in Net::SSH::KnownHosts#keys_for
       413  (15.6%)         163   (6.2%)     block (2 levels) in Net::SSH::KnownHosts#keys_for
       173   (6.5%)         111   (4.2%)     Net::SSH::Transport::Kex::DiffieHellmanGroup1SHA1#send_kexinit
       193   (7.3%)         106   (4.0%)     OpenSSL::PKey::DH#valid?
        89   (3.4%)          89   (3.4%)     Net::SSH::Compat.io_select
        87   (3.3%)          87   (3.3%)     block in OpenSSL::PKey::DH#valid?
        94   (3.6%)          62   (2.3%)     block in Net::SSH::Config.load
        59   (2.2%)          59   (2.2%)     block (2 levels) in Net::SSH::Transport::ServerVersion#negotiate!
        62   (2.3%)          52   (2.0%)     Net::SSH::Buffer#read
        75   (2.8%)          40   (1.5%)     Net::SSH::BufferedIo#fill
        39   (1.5%)          39   (1.5%)     block (2 levels) in Net::SSH::Connection::Channel#forward_local_env
       127   (4.8%)          33   (1.2%)     Net::SSH::Config.load
        32   (1.2%)          32   (1.2%)     Logger#debug?
        38   (1.4%)          32   (1.2%)     Net::SSH::Transport::State#update_next_iv
        55   (2.1%)          31   (1.2%)     Net::SSH::BufferedIo#send_pending
        29   (1.1%)          29   (1.1%)     Net::SSH::Buffer#initialize
        24   (0.9%)          24   (0.9%)     Net::SSH::Transport::HMAC::Abstract.mac_length
        24   (0.9%)          24   (0.9%)     Net::SSH::Transport::HMAC::Abstract.digest_class
       447  (16.9%)          22   (0.8%)     Net::SSH::KnownHosts#keys_for
        22   (0.8%)          22   (0.8%)     Net::SSH::Buffer#write_long
        22   (0.8%)          22   (0.8%)     Net::SSH::BufferedIo#input
        21   (0.8%)          21   (0.8%)     Net::SSH::Buffer#length
       186   (7.0%)          21   (0.8%)     Net::SSH::Transport::PacketStream#poll_next_packet
        20   (0.8%)          20   (0.8%)     Net::SSH::Config.pattern2regex
        19   (0.7%)          19   (0.7%)     Net::SSH::KnownHosts#known_host_hash?
        97   (3.7%)          19   (0.7%)     Net::SSH::Buffer.from
        19   (0.7%)          19   (0.7%)     Addrinfo#connect_internal
        45   (1.7%)          18   (0.7%)     Net::SSH::Buffer#read_long
        74   (2.8%)          18   (0.7%)     block in Net::SSH::Buffer.from

Warm:

==================================
  Mode: wall(1000)
  Samples: 3260 (1.81% miss rate)
  GC: 350 (10.74%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
       383  (11.7%)         383  (11.7%)     block (2 levels) in Net::SSH::Connection::Channel#forward_local_env
       331  (10.2%)         331  (10.2%)     Net::SSH::Compat.io_select
       116   (3.6%)         102   (3.1%)     Net::SSH::Buffer#read
       100   (3.1%)         100   (3.1%)     Net::SSH::Transport::HMAC::Abstract.digest_class
       100   (3.1%)         100   (3.1%)     Net::SSH::Transport::HMAC::Abstract.mac_length
       133   (4.1%)          80   (2.5%)     Net::SSH::BufferedIo#send_pending
       139   (4.3%)          74   (2.3%)     Net::SSH::BufferedIo#fill
        71   (2.2%)          71   (2.2%)     Net::SSH::Transport::State#update_next_iv
        80   (2.5%)          71   (2.2%)     SSHKit::Runner::Parallel#execute
        57   (1.7%)          57   (1.7%)     Net::SSH::Buffer#initialize
        55   (1.7%)          55   (1.7%)     Net::SSH::BufferedIo#output
        50   (1.5%)          50   (1.5%)     Logger#debug?
       628  (19.3%)          48   (1.5%)     Net::SSH::Transport::PacketStream#poll_next_packet
        94   (2.9%)          47   (1.4%)     Net::SSH::Transport::State#update_cipher
        46   (1.4%)          46   (1.4%)     block in Net::SSH::Transport::PacketStream#enqueue_packet
        45   (1.4%)          45   (1.4%)     Net::SSH::Buffer#write_long
       909  (27.9%)          44   (1.3%)     block in Net::SSH::Transport::Session#poll_message
        43   (1.3%)          43   (1.3%)     Net::SSH::BufferedIo#input
        41   (1.3%)          41   (1.3%)     block (3 levels) in <class:Digest>
        80   (2.5%)          40   (1.2%)     Net::SSH::Buffer#read_long
        40   (1.2%)          40   (1.2%)     Net::SSH::Buffer#length
        38   (1.2%)          38   (1.2%)     #<Module:0x007fad251d6c60>.reporter
       170   (5.2%)          36   (1.1%)     block in Net::SSH::Connection::Session#postprocess
        34   (1.0%)          34   (1.0%)     SSHKit::Color#colorize?
       107   (3.3%)          32   (1.0%)     block in Net::SSH::Buffer.from
        31   (1.0%)          31   (1.0%)     Net::SSH::Buffer#to_s
        31   (1.0%)          31   (1.0%)     Net::SSH::Buffer#append
        30   (0.9%)          30   (0.9%)     Net::SSH::Connection::Channel#active?
      2739  (84.0%)          29   (0.9%)     Net::SSH::Connection::Session#dispatch_incoming_packets
        28   (0.9%)          28   (0.9%)     Net::SSH::Connection::Channel#[]

So I guess there wasn't much contention on that mutex in the first place. I would 👍 that change though, because if someone happen to use cap with an alternative implementation that might make a big difference.

[leehambley]

👍

[mattbrictson]

Thanks for the review! It is nice to see that SSHKit is no longer a hot spot in the profile output, even if this doesn't make much difference in terms of wall time.

There are two skipped unit tests that I need to fix before this can be merged, as well as edits to the CHANGELOG. I'll try to get those wrapped up this week.

[mattbrictson]

I've rebased and squash this to a single commit, added a detailed commit message, and updated CHANGELOG and README.

@leehambley This is ready for final review and merge.

[mattbrictson]

@leehambley This has been rebased and is ready to merge.

[leehambley]

Thanks @mattbrictson