From 735abca1b6be37515b61dabe7c4dd2594e566713 Mon Sep 17 00:00:00 2001 From: Johannes Schlatow Date: Fri, 1 Jul 2022 16:32:25 +0200 Subject: [PATCH] nic_router: avoid marginal timeout updates The link dissolve timeout is updated for every packet, which leads to trigger_once() RPCs that only marginally change the scheduled timeout but significantly slow down the packet throughput. genodelabs/genode#4555 --- repos/base/include/timer/timeout.h | 2 + repos/base/include/timer_session/connection.h | 2 + .../server/nic_router/lazy_one_shot_timeout.h | 154 ++++++++++++++++++ repos/os/src/server/nic_router/link.cc | 3 +- repos/os/src/server/nic_router/link.h | 3 +- 5 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 repos/os/src/server/nic_router/lazy_one_shot_timeout.h diff --git a/repos/base/include/timer/timeout.h b/repos/base/include/timer/timeout.h index c9fab0c45a..a76178931d 100644 --- a/repos/base/include/timer/timeout.h +++ b/repos/base/include/timer/timeout.h @@ -123,6 +123,8 @@ class Genode::Timeout : private Noncopyable, void discard(); bool scheduled(); + + Microseconds deadline() const { return _deadline; } }; diff --git a/repos/base/include/timer_session/connection.h b/repos/base/include/timer_session/connection.h index cf126ca64b..4f24dea770 100644 --- a/repos/base/include/timer_session/connection.h +++ b/repos/base/include/timer_session/connection.h @@ -117,6 +117,8 @@ class Timer::One_shot_timeout : private Genode::Noncopyable, void discard() { _timeout.discard(); } bool scheduled() { return _timeout.scheduled(); } + + Microseconds deadline() const { return _timeout.deadline(); } }; diff --git a/repos/os/src/server/nic_router/lazy_one_shot_timeout.h b/repos/os/src/server/nic_router/lazy_one_shot_timeout.h new file mode 100644 index 0000000000..f88ae2725b --- /dev/null +++ b/repos/os/src/server/nic_router/lazy_one_shot_timeout.h @@ -0,0 +1,154 @@ +/* + * \brief A wrapper for Timer::One_shot_timeout with lazy re-scheduling + * \author Johannes Schlatow + * \date 2022-07-01 + * + * NOTE: This implementation is not thread safe and should only be used in + * single-threaded components. + * + * This implementation prevents re-scheduling when a timeout is frequently + * updated with only marginal changes. Timeouts within a certain accuracy + * threshold of the existing timeout will be ignored. Otherwise, earlier + * timeouts will always be re-scheduled whereas later timeouts are never + * applied immediately but only when the scheduled timeout occured. + */ + +/* + * Copyright (C) 2022 Genode Labs GmbH + * + * This file is part of the Genode OS framework, which is distributed + * under the terms of the GNU Affero General Public License version 3. + */ + +#ifndef _LAZY_ONE_SHOT_TIMEOUT_H_ +#define _LAZY_ONE_SHOT_TIMEOUT_H_ + +/* Genode includes */ +#include + +namespace Net { + + template class Lazy_one_shot_timeout; +} + + +template +class Net::Lazy_one_shot_timeout +: + private Timer::One_shot_timeout> +{ + private: + + using One_shot_timeout = Timer::One_shot_timeout>; + using Microseconds = Genode::Microseconds; + using Duration = Genode::Duration; + using uint64_t = Genode::uint64_t; + using Handler_method = void (HANDLER::*)(Duration); + + Timer::Connection &_timer; + HANDLER &_object; + Handler_method const _method; + uint64_t const _tolerance_us; + uint64_t _postponed_deadline_us { 0 }; + + void _handle_timeout(Duration curr_time) + { + /* + * If the postponed deadline is set and more than tolerance + * microseconds in the future, skip calling the user handler and + * re-schedule with the postponed deadline instead. + */ + if (_postponed_deadline_us > 0) { + + uint64_t const curr_time_us { + curr_time.trunc_to_plain_us().value }; + + if (curr_time_us + _tolerance_us < _postponed_deadline_us) { + + One_shot_timeout::schedule( + Microseconds { _postponed_deadline_us - curr_time_us }); + + _postponed_deadline_us = 0; + return; + } + } + /* + * If not, call the user handler. + */ + (_object.*_method)(curr_time); + } + + public: + + using One_shot_timeout::discard; + using One_shot_timeout::scheduled; + + Lazy_one_shot_timeout(Timer::Connection &timer, + HANDLER &object, + Handler_method method, + Microseconds tolerance) + : + One_shot_timeout { timer, *this, + &Lazy_one_shot_timeout::_handle_timeout }, + _timer { timer }, + _object { object }, + _method { method }, + _tolerance_us { tolerance.value } + { } + + /** + * In contrast to the original 'schedule' method, this wrapper evalutes + * whether scheduling must be done immediately, can be postponed to + * '_handle_timeout', or can even be skipped. + * + * Scheduling is done immediately if + * the timeout is not active OR + * new deadline < old deadline - tolerance + * + * Scheduling is postponed to '_handle_timeout' if + * new deadline > old deadline + tolerance + * + * Scheduling is skipped if + * new deadline >= old deadline - tolerance AND + * new deadline <= old deadline + tolerance + */ + void schedule(Microseconds duration) + { + /* remove old postponed deadline */ + _postponed_deadline_us = 0; + + /* no special treatment if timeout is not scheduled */ + if (!scheduled()) { + One_shot_timeout::schedule(duration); + return; + } + uint64_t const curr_time_us { + _timer.curr_time().trunc_to_plain_us().value }; + + uint64_t const new_deadline_us { + duration.value <= ~(uint64_t)0 - curr_time_us ? + curr_time_us + duration.value : ~(uint64_t)0 }; + + uint64_t const old_deadline_us { One_shot_timeout::deadline().value }; + + if (new_deadline_us < old_deadline_us) { + /* + * The new deadline is earlier. If the old deadline is not + * accurate enough, re-schedule. Else, drop the new deadline. + */ + if (new_deadline_us < old_deadline_us - _tolerance_us) + One_shot_timeout::schedule(duration); + + } else { + /* + * The new deadline is later. If the old deadline is not + * accurate enough, remember the new deadline and apply it in + * '_handle_timeout'. Else, drop the new deadline. + */ + if (new_deadline_us > old_deadline_us + _tolerance_us) + _postponed_deadline_us = new_deadline_us; + } + } +}; + +#endif /* _LAZY_ONE_SHOT_TIMEOUT_H_ */ diff --git a/repos/os/src/server/nic_router/link.cc b/repos/os/src/server/nic_router/link.cc index 37210359aa..259aaa9a14 100644 --- a/repos/os/src/server/nic_router/link.cc +++ b/repos/os/src/server/nic_router/link.cc @@ -99,7 +99,8 @@ Link::Link(Interface &cln_interface, _config(config), _client_interface(cln_interface), _server_port_alloc(srv_port_alloc), - _dissolve_timeout(timer, *this, &Link::_handle_dissolve_timeout), + _dissolve_timeout(timer, *this, &Link::_handle_dissolve_timeout, + Microseconds { 100 * 1000 }), _dissolve_timeout_us(dissolve_timeout), _protocol(protocol), _client(cln_interface.domain(), cln_id, *this), diff --git a/repos/os/src/server/nic_router/link.h b/repos/os/src/server/nic_router/link.h index e3bf938c7a..50b8ec8d14 100644 --- a/repos/os/src/server/nic_router/link.h +++ b/repos/os/src/server/nic_router/link.h @@ -42,6 +42,7 @@ #include #include #include +#include namespace Net { @@ -187,7 +188,7 @@ class Net::Link : public Link_list::Element Reference _config; Interface &_client_interface; Pointer _server_port_alloc; - Timer::One_shot_timeout _dissolve_timeout; + Lazy_one_shot_timeout _dissolve_timeout; Genode::Microseconds _dissolve_timeout_us; L3_protocol const _protocol; Link_side _client;