Code review comment for ~johill-lanl/epics-base/+git/epics-base:timer-queue-fix

Revision history for this message
Martin Konrad (info-martin-konrad) wrote :

I haven't looked at the code, yet, but it builds fine on Ubuntu 19.10 with GCC 9.2: no warnings, all tests pass :-) A few compiler/OS combinations fail to build on Travis (https://travis-ci.com/github/mark0n/epics-base/builds/161843145) and AppVeyor (https://ci.appveyor.com/project/MartinKonrad/epics-base/builds/32416944), though.

Before digging into the code, I benchmarked the old and the proposed timer queue implementation. You can find the results here:

https://www.martin-konrad.net/nextcloud/index.php/s/mYHXWQxbJFz4LkL

The old implementation takes more than 100 us to create and start a new timer if there are already 30,000 timers in the queue (this number is based on a real-world scenario with Asyn/Stream at FRIB) and the new timer needs to be inserted at the beginning of the list (30,000 existing timers with 60 s, the new timer has 30 s duration). In this case the old code has to walk a linked list from the back all the way to the front (the code was optimized for inserting timers with the same duration). The new implementation performs two orders of magnitude better in this scenario. Not bad! None of the cases I benchmarked is performing significantly worse than with the old implementation. For comparison I also benchmarked boost::asio's timers - its performance is in the same order of magnitude.

See https://github.com/mark0n/benchmark_timerqueue for my benchmark code.

« Back to merge proposal