socketry/timers

Algorithmic complexity in merge! function can be improved

WJWH opened this issue · 0 comments

WJWH commented

The merge! function in timers/events.rb uses Array#insert! to insert a new Handle into the (sorted in descending order) @sequence array. This is an O(N) operation, since all subsequent items need to be shifted one position "to the right". (I found a deleted comment in the commit history here that seems to assume inserting is O(log N) but that is not the case for Ruby arrays) In particular for the case where every new Timer has an expiration time later than all the other Timers, such as would be the case when all timers are attached to a web request for example, each Timer will be inserted at the start of the @sequence array and hit the worst case for the array insertion where every other element needs to be shifted.

The current implementation thus has the following algorithmic runtimes for each of the steps:

  • O(1) for enqueueing a new fiber into the @queue array`
  • O(log N) for de binary search to find the index in merge!
  • O(N) for the array insertion (if you are very lucky O(1) if the timer has been canceled, but with_timeout in Async does not cancel the timer AFAICT?)
  • O(1) for checking the time of the next timer.
  • O(1) for popping off the timer from the array during fire.

If we replace the @sequence array from an array to a binary heap, this could change to:

  • O(1) for enqueueing a new fiber into the @queue array`
  • O(log N) for inserting a new timer into the heap during merge!
  • O(1) for checking the time of the next timer.
  • O(log N) for dequeueing the next timer.

Interestingly, the worst case for the array version is the best case for the heap version since implementing an element larger than all the others into a minheap shortcuts immediately and is O(1). I went ahead and implemented a super simple benchmark in this branch with priority heaps implemented in both pure Ruby and one off-the-shelf gem with a C++ extension. The results for implementing 20k new timers into a structure with 20k timers already there are as follows:

              user     system      total        real
array     0.108545   0.000000   0.108545 (  0.108874)
heap      0.033970   0.000000   0.033970 (  0.034054)
heap C++  0.025127   0.000000   0.025127 (  0.025132)

The test script mainly measures the results of the merge! operation and of pushing new items onto it, no timers are popped off the stack. Let me know what you think, I can make the branch into a proper PR if you want.