Commit 044deb35 authored by Kirill Smelkov's avatar Kirill Smelkov

time: Redo timers properly

Background: in 2019 in 9c260fde (time: New package that mirrors Go's
time) and b073f6df (time: Move/Port timers to C++/Pyx nogil) I've added
basic timers - with proper API but with very dumb implementation that
was spawning one thread per each timer. There were just a few timers in
the users and this was working, surprisingly, relatively ok...

... until 2023 where I was working on XLTE that needs to organize 100Hz
polling of Amarisoft eNodeB service to retrieve information about flows
on Data Radio Bearers:

    xlte@2a016d48
    https://lab.nexedi.com/kirr/xlte/-/blob/8e606c64/amari/drb.py

There each request comes with its own deadline - to catch "no reply",
and the deadlines are implemented via timers. So there are 100 threads
created every second which adds visible overhead, consumes a lot of
virtual address space and RSS for threads stacks, and should be all unnecessary.

We was tolerating even that for some time, but recently Joanne approached me
with reports that xamari program, that does the polling, is leaking memory.

With that, and because it was hard to find what is actually leaking,
I've started to remove uncertainties and there are a lot of uncertainty
in what is going on when lots of threads are being created over and over.

In the end the leak turned out to be likely a different thing (see
nexedi/pygolang!24, still
discovered while working on hereby patch), but all of the above was
enough motivation to finally start redoing the timers properly.

--------

So when it comes to do the timers properly more or less, there is
usually queue of armed timers, and a loop that picks entries from that
queue to fire them. I was initially trying to do the simple thing and
use std::priority_queue for that, because priority_queue is internally
heap, and heaps can provide O(log(n)) insertion and removal of arbitrary
element, plus O(1) "pick top element to process". Exactly what would
suit. However I quickly found that even in 2024, std::priority_queue
does not provide removal operation at all, and there is no such thing as
e.g. std::sift_heap, that would help to implement that manually. Which
is surprising, because e.g. libevent implements all that just ok via
sifting up/down upon removal in logarithmic complexity:

https://github.com/libevent/libevent/blob/80e25c02/minheap-internal.h#L96-L115

the lack of efficient removal operation turned out to be a blocker to
use std::priority_queue because most of the timers, that are armed for
timeouts, are never expired and upon successful completion of covered
operation, the timer is stopped. In other words the timer is removed
from the timer queue and the removal is one of the most often
operations.

So, if std::priority_queue cannot work, we would need to either bring in
another implementation of a heap, or, if we are to bring something,
bring and use something else that is more suitable for implementing
timers.

That reminded me that in 2005 for my Navy project, I already implemented
custom timer wheel to handle timeouts after reading https://lwn.net/Articles/152436/ .
Contrary to heaps, such timer wheels provide O(1) insertion and removal
of timers and work generally faster. But this time I did not want to
delve into implementing all that myself again and tried to look around
of what is available out there.

There was an update to kernel timer-wheel implementation described at
https://lwn.net/Articles/646950/ and from that a project called
Timeout.c was also found that provides implementation for such a wheel
for user space: https://25thandclement.com/~william/projects/timeout.c.html .

However when we are to pick third-party code, we should be ready to
understand it and fix bugs there on our own. So the audit of timeout.c
did not went very smoothly - there are many platform-depended places,
and the issue tracker shows signs that sometimes not everything is ok
with the implementation. With that I've looked around a bit more and
found more compact and more portable Ratas library with good structure
and description and whose audit came more well:

    https://www.snellman.net/blog/archive/2016-07-27-ratas-hierarchical-timer-wheel
    https://github.com/jsnell/ratas

Here, after going through the code, I feel to be capable to understand
issues and fix bugs myself if that would become needed.

And the benchmark comparison of Timeout.c and Ratas shows that they
should be of the same order regarding performance:

https://lab.nexedi.com/kirr/misc/-/blob/4f51fd6/bench/time-wheel/ratas-vs-timeout.pdf
ratas@382321d2
timeout@d6f15744

which makes Ratas the winner for me.

Having timer-wheel implementation, the rest is just technique to glue it
all together. One implementation aspect deserves to be mentioned though:

The timer loop uses Semaphore.acquire, recently modernized to also
accept timeout, to organize sleep in between pauses with also being able
to be simultaneously woken up if new timer is armed with earlier
expiration time.

Other than that the changes are mostly straightforward. Please see the
patch itself for details.

Regarding how the new implementation is more efficient for what we had
before, there are added benchmarks to measure arming timers that do not
fire, and, for symmetry, arming timers that do fire. We are most
interested in the first benchmark, because it shows how cheap or
expensive it is to use timers to implement timeouts, but the second one
is also useful to have to see the overhead of the whole timers machinery.

On my machine under py3.11 they go as after this patch:

    name              time/op
    timer_arm_cancel   805ns ± 0%
    timer_arm_fire    9.63µs ± 0%

and before the patch the benchmarks simply do not run till the end
because they run out of memory due to huge number of threads being
created.

Still with the following test program we can measure the effect new
timers implementation has:

    ---- 8< ----
    from golang import time

    def main():
        δt_rate = 1*time.millisecond

        tprev = time.now()
        tnext = tprev + δt_rate
        while 1:
            timer = time.Timer(5*time.second)
            _ = timer.stop()
            assert _ is True

            t = time.now()
            δtsleep = tnext - t
            #print('sleep %.3f ms' % (δtsleep/time.millisecond))
            time.sleep(δtsleep)
            tprev = tnext
            tnext += δt_rate

    main()
    ---- 8< ----

This program creates/arms and cancels a timer 1000 times per second.

Before hereby patch this program consumes ~ 30% of CPU, while after
hereby patch this program consumes ~ 7-8% of CPU.

For the reference just a sleep part of that program, with all code
related to timers removed consumes ~5% of CPU, while the consumption of
plain sleep(1ms) in C and directly using system calls

    ---- 8< ----
    #include <unistd.h>

    int main() {
        while (1) {
            usleep(1000);
        }
        return 0;
    }
    ---- 8< ----

is ~ 3-4% of CPU on my machine.

/cc @jerome
/cc ORS team (@jhuge, @lu.xu, @tomo, @xavier_thompson, @Daetalus)
/proposed-for-review-on nexedi/pygolang!26
parent 82c55254
[submodule "3rdparty/ratas"]
path = 3rdparty/ratas
url = https://github.com/jsnell/ratas.git
Subproject commit becd5fc5c1e9ea600cd8b3b1c24d564794fedac4
...@@ -131,6 +131,7 @@ using internal::_runtime; ...@@ -131,6 +131,7 @@ using internal::_runtime;
namespace internal { namespace atomic { extern void _init(); } } namespace internal { namespace atomic { extern void _init(); } }
namespace os { namespace signal { extern void _init(); } } namespace os { namespace signal { extern void _init(); } }
namespace time { extern void _init(); }
void _libgolang_init(const _libgolang_runtime_ops *runtime_ops) { void _libgolang_init(const _libgolang_runtime_ops *runtime_ops) {
if (_runtime != nil) // XXX better check atomically if (_runtime != nil) // XXX better check atomically
panic("libgolang: double init"); panic("libgolang: double init");
...@@ -138,6 +139,7 @@ void _libgolang_init(const _libgolang_runtime_ops *runtime_ops) { ...@@ -138,6 +139,7 @@ void _libgolang_init(const _libgolang_runtime_ops *runtime_ops) {
internal::atomic::_init(); internal::atomic::_init();
os::signal::_init(); os::signal::_init();
time::_init();
} }
void _taskgo(void (*f)(void *), void *arg) { void _taskgo(void (*f)(void *), void *arg) {
......
This diff is collapsed.
#ifndef _NXD_LIBGOLANG_TIME_H #ifndef _NXD_LIBGOLANG_TIME_H
#define _NXD_LIBGOLANG_TIME_H #define _NXD_LIBGOLANG_TIME_H
// Copyright (C) 2019-2023 Nexedi SA and Contributors. // Copyright (C) 2019-2024 Nexedi SA and Contributors.
// Kirill Smelkov <kirr@nexedi.com> // Kirill Smelkov <kirr@nexedi.com>
// //
// This program is free software: you can Use, Study, Modify and Redistribute // This program is free software: you can Use, Study, Modify and Redistribute
...@@ -118,6 +118,7 @@ private: ...@@ -118,6 +118,7 @@ private:
double _dt; double _dt;
sync::Mutex _mu; sync::Mutex _mu;
bool _stop; bool _stop;
Timer _timer;
// don't new - create only via new_ticker() // don't new - create only via new_ticker()
private: private:
...@@ -147,18 +148,12 @@ LIBGOLANG_API Timer new_timer(double dt); ...@@ -147,18 +148,12 @@ LIBGOLANG_API Timer new_timer(double dt);
struct _Timer : object { struct _Timer : object {
chan<double> c; chan<double> c;
private:
func<void()> _f;
sync::Mutex _mu;
double _dt; // +inf - stopped, otherwise - armed
int _ver; // current timer was armed by n'th reset
// don't new - create only via new_timer() & co // don't new - create only via new_timer() & co
private: private:
_Timer(); _Timer();
~_Timer(); ~_Timer();
friend Timer _new_timer(double dt, func<void()> f); friend Timer _new_timer(double dt, func<void()> f);
friend class _TimerImpl;
public: public:
LIBGOLANG_API void decref(); LIBGOLANG_API void decref();
...@@ -182,9 +177,6 @@ public: ...@@ -182,9 +177,6 @@ public:
// //
// the timer must be either already stopped or expired. // the timer must be either already stopped or expired.
LIBGOLANG_API void reset(double dt); LIBGOLANG_API void reset(double dt);
private:
void _fire(double dt, int ver);
}; };
......
...@@ -23,6 +23,7 @@ from __future__ import print_function, absolute_import ...@@ -23,6 +23,7 @@ from __future__ import print_function, absolute_import
from golang import select, func, defer from golang import select, func, defer
from golang import time, sync from golang import time, sync
from golang.golang_test import panics from golang.golang_test import panics
from six.moves import range as xrange
# all timer tests operate in dt units # all timer tests operate in dt units
dt = 10*time.millisecond dt = 10*time.millisecond
...@@ -222,3 +223,22 @@ def test_timer_reset_armed(): ...@@ -222,3 +223,22 @@ def test_timer_reset_armed():
t = time.Timer(10*dt); defer(t.stop) t = time.Timer(10*dt); defer(t.stop)
with panics("Timer.reset: the timer is armed; must be stopped or expired"): with panics("Timer.reset: the timer is armed; must be stopped or expired"):
t.reset(5*dt) t.reset(5*dt)
# bench_timer_arm_cancel benchmarks arming timers that do not fire.
# it shows how cheap or expensive it is to use timers to implement timeouts.
def bench_timer_arm_cancel(b):
for i in xrange(b.N):
t = time.Timer(10*time.second)
_ = t.stop()
assert _ is True
# bench_timer_arm_fire benchmarks arming timers that do fire.
# it shows what it costs to go through all steps related to timer loop and firing timers.
def bench_timer_arm_fire(b):
wg = sync.WaitGroup()
wg.add(b.N)
for i in xrange(b.N):
t = time.after_func(1*time.millisecond, wg.done)
wg.wait()
...@@ -229,8 +229,11 @@ setup( ...@@ -229,8 +229,11 @@ setup(
'golang/os/signal.h', 'golang/os/signal.h',
'golang/strings.h', 'golang/strings.h',
'golang/sync.h', 'golang/sync.h',
'golang/time.h'], 'golang/time.h',
include_dirs = ['3rdparty/include'], '3rdparty/ratas/src/timer-wheel.h'],
include_dirs = [
'3rdparty/include',
'3rdparty/ratas/src'],
define_macros = [('BUILDING_LIBGOLANG', None)], define_macros = [('BUILDING_LIBGOLANG', None)],
soversion = '0.1'), soversion = '0.1'),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment