Skip to content
9 changes: 9 additions & 0 deletions include/envoy/event/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,15 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "scaled_range_timer_manager_interface",
hdrs = ["scaled_range_timer_manager.h"],
deps = [
":range_timer_interface",
":timer_interface",
],
)

envoy_cc_library(
name = "schedulable_cb_interface",
hdrs = ["schedulable_cb.h"],
Expand Down
88 changes: 88 additions & 0 deletions include/envoy/event/scaled_range_timer_manager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
#pragma once

#include "envoy/common/pure.h"
#include "envoy/event/range_timer.h"
#include "envoy/event/timer.h"

#include "absl/types/variant.h"

namespace Envoy {
namespace Event {

/**
* Describes a minimum timer value that is equal to a scale factor applied to the maximum.
*/
struct ScaledMinimum {
explicit ScaledMinimum(double scale_factor) : scale_factor_(scale_factor) {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the wrapper I was suggesting in the previous PR? Do we assume that this is between 0.0 and 1.0? Can we document that and ASSERT that?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned below, I'm going to leave this for a refactoring PR.

const double scale_factor_;
};

/**
* Describes a minimum timer value that is an absolute duration.
*/
struct AbsoluteMinimum {
explicit AbsoluteMinimum(std::chrono::milliseconds value) : value_(value) {}
const std::chrono::milliseconds value_;
};

class ScaledTimerMinimum : public absl::variant<ScaledMinimum, AbsoluteMinimum> {
Comment thread
akonradi marked this conversation as resolved.
Outdated
public:
using absl::variant<ScaledMinimum, AbsoluteMinimum>::variant;
Comment thread
akonradi marked this conversation as resolved.

std::chrono::milliseconds computeMinimum(std::chrono::milliseconds maximum) const {
Comment thread
akonradi marked this conversation as resolved.
struct Visitor {
explicit Visitor(std::chrono::milliseconds value) : value_(value) {}
std::chrono::milliseconds operator()(ScaledMinimum scale_factor) {
return std::chrono::duration_cast<std::chrono::milliseconds>(scale_factor.scale_factor_ *
value_);
}
std::chrono::milliseconds operator()(AbsoluteMinimum absolute_value) {
return absolute_value.value_;
}
const std::chrono::milliseconds value_;
};
return absl::visit(Visitor(maximum), *this);
}
};

/**
* Class for creating RangeTimer objects that can be adjusted towards either the minimum or maximum
* of their range by the owner of the manager object. Users of this class can call createTimer() to
* receive a new RangeTimer object that they can then enable or disable at will (but only on the
* same dispatcher), and setScaleFactor() to change the scaling factor. The current scale factor is
* applied to all timers, including those that are created later.
*/
class ScaledRangeTimerManager {
public:
virtual ~ScaledRangeTimerManager() = default;

/**
* Creates a new range timer backed by the manager. The returned timer will be subject to the
* current and future scale factor values set on the manager. All returned timers must be deleted
* before the manager.
*/
virtual RangeTimerPtr createRangeTimer(TimerCb callback) PURE;

/**
* Creates a new range timer backed by the manager that mimics the Timer interface. Calling
* enableTimer on the returned object sets the maximum duration, while the first argument here
* controls the minimum. Passing a value of ScaleFactor(x) sets the min to (x * max) when the
* timer is enabled, while AbsoluteValue(y) sets the min to the duration y.
*/
virtual TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) PURE;
Comment thread
akonradi marked this conversation as resolved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can RangeTimer just derive from Timer? It seems like we could avoid by allowing a RangeTimer to also be stored as a normal timer if needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the RangeTimer interface is destined for the bit bucket; we don't need it for any consumer code now that we have a createTimer method and it's not used anywhere. I think we should just remove it. Thoughts on doing that in this PR or another one?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just do it!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, PTAL.


/**
* Sets the scale factor for all timers created through this manager. The value should be between
* 0 and 1, inclusive. The scale factor affects the amount of time timers spend in their target
* range. The RangeTimers returned by createTimer will fire after (min + (max - min) *
* scale_factor). This means that a scale factor of 0 causes timers to fire immediately at the min
* duration, a factor of 0.5 causes firing halfway between min and max, and a factor of 1 causes
* firing at max.
*/
virtual void setScaleFactor(double scale_factor) PURE;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this take a ScaledMinimum (or similar) which can better ASSERT that it's between 0.0 and 1.0 and then clamp if needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Between here and the OverloadAction class, I think it would be a good idea to have a single simple wrapper class that enforces 0.0 and 1.0 bounds. I'm going to do that in a future PR.

};

using ScaledRangeTimerManagerPtr = std::unique_ptr<ScaledRangeTimerManager>;

} // namespace Event
} // namespace Envoy
7 changes: 4 additions & 3 deletions source/common/event/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,13 @@ envoy_cc_library(
)

envoy_cc_library(
name = "scaled_range_timer_manager",
srcs = ["scaled_range_timer_manager.cc"],
hdrs = ["scaled_range_timer_manager.h"],
name = "scaled_range_timer_manager_lib",
srcs = ["scaled_range_timer_manager_impl.cc"],
hdrs = ["scaled_range_timer_manager_impl.h"],
deps = [
"//include/envoy/event:dispatcher_interface",
"//include/envoy/event:range_timer_interface",
"//include/envoy/event:scaled_range_timer_manager_interface",
"//source/common/common:scope_tracker",
],
)
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include "common/event/scaled_range_timer_manager.h"
#include "common/event/scaled_range_timer_manager_impl.h"

#include <chrono>
#include <cmath>
Expand Down Expand Up @@ -31,9 +31,9 @@ namespace Event {
* [scaling-max -> inactive -> waiting-for-min -> scaling-max] in a single
* method call. The waiting-for-min transitions are elided for efficiency.
*/
class ScaledRangeTimerManager::RangeTimerImpl final : public RangeTimer {
class ScaledRangeTimerManagerImpl::RangeTimerImpl final : public RangeTimer {
public:
RangeTimerImpl(TimerCb callback, ScaledRangeTimerManager& manager)
RangeTimerImpl(TimerCb callback, ScaledRangeTimerManagerImpl& manager)
: manager_(manager), callback_(std::move(callback)),
min_duration_timer_(manager.dispatcher_.createTimer([this] { onMinTimerComplete(); })) {}

Expand Down Expand Up @@ -98,10 +98,10 @@ class ScaledRangeTimerManager::RangeTimerImpl final : public RangeTimer {
};

struct ScalingMax {
ScalingMax(ScaledRangeTimerManager::ScalingTimerHandle handle) : handle_(handle) {}
ScalingMax(ScaledRangeTimerManagerImpl::ScalingTimerHandle handle) : handle_(handle) {}

// A handle that can be used to disable the timer.
ScaledRangeTimerManager::ScalingTimerHandle handle_;
ScaledRangeTimerManagerImpl::ScalingTimerHandle handle_;
};

/**
Expand All @@ -123,60 +123,90 @@ class ScaledRangeTimerManager::RangeTimerImpl final : public RangeTimer {
}
}

ScaledRangeTimerManager& manager_;
ScaledRangeTimerManagerImpl& manager_;
const TimerCb callback_;
const TimerPtr min_duration_timer_;

absl::variant<Inactive, WaitingForMin, ScalingMax> state_;
const ScopeTrackedObject* scope_;
};

ScaledRangeTimerManager::ScaledRangeTimerManager(Dispatcher& dispatcher)
class ScaledRangeTimerManagerImpl::FixedMinimumRangeTimerImpl final : public Event::Timer {
public:
FixedMinimumRangeTimerImpl(ScaledTimerMinimum minimum, TimerCb timer_cb,
ScaledRangeTimerManagerImpl& manager)
: minimum_(minimum), range_timer_(std::move(timer_cb), manager) {}

void enableTimer(std::chrono::milliseconds ms,
const ScopeTrackedObject* object = nullptr) override {
range_timer_.enableTimer(minimum_.computeMinimum(ms), ms, object);
}

void enableHRTimer(std::chrono::microseconds us,
const ScopeTrackedObject* object = nullptr) override {
enableTimer(std::chrono::duration_cast<std::chrono::milliseconds>(us), object);
Comment thread
antoniovicente marked this conversation as resolved.
Outdated
}

void disableTimer() override { range_timer_.disableTimer(); }

bool enabled() override { return range_timer_.enabled(); }

private:
const ScaledTimerMinimum minimum_;
RangeTimerImpl range_timer_;
};

ScaledRangeTimerManagerImpl::ScaledRangeTimerManagerImpl(Dispatcher& dispatcher)
: dispatcher_(dispatcher), scale_factor_(1.0) {}

ScaledRangeTimerManager::~ScaledRangeTimerManager() {
ScaledRangeTimerManagerImpl::~ScaledRangeTimerManagerImpl() {
// Scaled timers created by the manager shouldn't outlive it. This is
// necessary but not sufficient to guarantee that.
ASSERT(queues_.empty());
}

RangeTimerPtr ScaledRangeTimerManager::createTimer(TimerCb callback) {
RangeTimerPtr ScaledRangeTimerManagerImpl::createRangeTimer(TimerCb callback) {
return std::make_unique<RangeTimerImpl>(callback, *this);
}

void ScaledRangeTimerManager::setScaleFactor(double scale_factor) {
TimerPtr ScaledRangeTimerManagerImpl::createTimer(ScaledTimerMinimum minimum, TimerCb callback) {
return std::make_unique<FixedMinimumRangeTimerImpl>(minimum, callback, *this);
}

void ScaledRangeTimerManagerImpl::setScaleFactor(double scale_factor) {
const MonotonicTime now = dispatcher_.approximateMonotonicTime();
scale_factor_ = DurationScaleFactor(scale_factor);
for (auto& queue : queues_) {
resetQueueTimer(*queue, now);
}
}

ScaledRangeTimerManager::Queue::Item::Item(RangeTimerImpl& timer, MonotonicTime active_time)
ScaledRangeTimerManagerImpl::Queue::Item::Item(RangeTimerImpl& timer, MonotonicTime active_time)
: timer_(timer), active_time_(active_time) {}

ScaledRangeTimerManager::Queue::Queue(std::chrono::milliseconds duration,
ScaledRangeTimerManager& manager, Dispatcher& dispatcher)
ScaledRangeTimerManagerImpl::Queue::Queue(std::chrono::milliseconds duration,
ScaledRangeTimerManagerImpl& manager,
Dispatcher& dispatcher)
: duration_(duration),
timer_(dispatcher.createTimer([this, &manager] { manager.onQueueTimerFired(*this); })) {}

ScaledRangeTimerManager::ScalingTimerHandle::ScalingTimerHandle(Queue& queue,
Queue::Iterator iterator)
ScaledRangeTimerManagerImpl::ScalingTimerHandle::ScalingTimerHandle(Queue& queue,
Queue::Iterator iterator)
: queue_(queue), iterator_(iterator) {}

ScaledRangeTimerManager::DurationScaleFactor::DurationScaleFactor(double value)
ScaledRangeTimerManagerImpl::DurationScaleFactor::DurationScaleFactor(double value)
: value_(std::max(0.0, std::min(value, 1.0))) {}

MonotonicTime ScaledRangeTimerManager::computeTriggerTime(const Queue::Item& item,
std::chrono::milliseconds duration,
DurationScaleFactor scale_factor) {
MonotonicTime ScaledRangeTimerManagerImpl::computeTriggerTime(const Queue::Item& item,
std::chrono::milliseconds duration,
DurationScaleFactor scale_factor) {
return item.active_time_ +
std::chrono::duration_cast<MonotonicTime::duration>(duration * scale_factor.value());
}

ScaledRangeTimerManager::ScalingTimerHandle
ScaledRangeTimerManager::activateTimer(std::chrono::milliseconds duration,
RangeTimerImpl& range_timer) {
ScaledRangeTimerManagerImpl::ScalingTimerHandle
ScaledRangeTimerManagerImpl::activateTimer(std::chrono::milliseconds duration,
RangeTimerImpl& range_timer) {
// Ensure this is being called on the same dispatcher.
ASSERT(dispatcher_.isThreadSafe());

Expand All @@ -200,7 +230,7 @@ ScaledRangeTimerManager::activateTimer(std::chrono::milliseconds duration,
return ScalingTimerHandle(queue, --queue.range_timers_.end());
}

void ScaledRangeTimerManager::removeTimer(ScalingTimerHandle handle) {
void ScaledRangeTimerManagerImpl::removeTimer(ScalingTimerHandle handle) {
// Ensure this is being called on the same dispatcher.
ASSERT(dispatcher_.isThreadSafe());

Expand All @@ -219,7 +249,7 @@ void ScaledRangeTimerManager::removeTimer(ScalingTimerHandle handle) {
}
}

void ScaledRangeTimerManager::resetQueueTimer(Queue& queue, MonotonicTime now) {
void ScaledRangeTimerManagerImpl::resetQueueTimer(Queue& queue, MonotonicTime now) {
ASSERT(!queue.range_timers_.empty());
const MonotonicTime trigger_time =
computeTriggerTime(queue.range_timers_.front(), queue.duration_, scale_factor_);
Expand All @@ -231,7 +261,7 @@ void ScaledRangeTimerManager::resetQueueTimer(Queue& queue, MonotonicTime now) {
}
}

void ScaledRangeTimerManager::onQueueTimerFired(Queue& queue) {
void ScaledRangeTimerManagerImpl::onQueueTimerFired(Queue& queue) {
auto& timers = queue.range_timers_;
ASSERT(!timers.empty());
const MonotonicTime now = dispatcher_.approximateMonotonicTime();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "envoy/event/dispatcher.h"
#include "envoy/event/range_timer.h"
#include "envoy/event/scaled_range_timer_manager.h"
#include "envoy/event/timer.h"

#include "absl/container/flat_hash_map.h"
Expand All @@ -11,43 +12,29 @@ namespace Envoy {
namespace Event {

/**
* Class for creating RangeTimer objects that can be adjusted towards either the minimum or maximum
* of their range by the owner of the manager object. Users of this class can call createTimer() to
* receive a new RangeTimer object that they can then enable or disable at will (but only on the
* same dispatcher), and setScaleFactor() to change the scaling factor. The current scale factor is
* applied to all timers, including those that are created later.
*
* Internally, the manager uses a set of queues to track timers. When an enabled timer reaches its
* min duration, it adds a tracker object to the queue corresponding to the duration (max - min).
* Each queue tracks timers of only a single duration, and uses a real Timer object to schedule the
* expiration of the first timer in the queue. The expectation is that the number of (max - min)
* values used to enable timers is small, so the number of queues is tightly bounded. The
* queue-based implementation depends on that expectation for efficient operation.
* Implementation class for ScaledRangeTimerManager. Internally, this uses a set of queues to track
* timers. When an enabled timer reaches its min duration, it adds a tracker object to the queue
* corresponding to the duration (max - min). Each queue tracks timers of only a single duration,
* and uses a real Timer object to schedule the expiration of the first timer in the queue. The
* expectation is that the number of (max - min) values used to enable timers is small, so the
* number of queues is tightly bounded. The queue-based implementation depends on that expectation
* for efficient operation.
*/
class ScaledRangeTimerManager {
class ScaledRangeTimerManagerImpl : public ScaledRangeTimerManager {
public:
explicit ScaledRangeTimerManager(Dispatcher& dispatcher);
~ScaledRangeTimerManager();
explicit ScaledRangeTimerManagerImpl(Dispatcher& dispatcher);
~ScaledRangeTimerManagerImpl() override;

/**
* Creates a new range timer backed by the manager. The returned timer will be subject to the
* current and future scale factor values set on the manager. All returned timers must be deleted
* before the manager.
*/
RangeTimerPtr createTimer(TimerCb callback);
// ScaledRangeTimerManager impl
RangeTimerPtr createRangeTimer(TimerCb callback) override;

/**
* Sets the scale factor for all timers created through this manager. The value should be between
* 0 and 1, inclusive. The scale factor affects the amount of time timers spend in their target
* range. The RangeTimers returned by createTimer will fire after (min + (max - min) *
* scale_factor). This means that a scale factor of 0 causes timers to fire immediately at the min
* duration, a factor of 0.5 causes firing halfway between min and max, and a factor of 1 causes
* firing at max.
*/
void setScaleFactor(double scale_factor);
TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) override;

void setScaleFactor(double scale_factor) override;

private:
class RangeTimerImpl;
class FixedMinimumRangeTimerImpl;

// A queue object that maintains a list of timers with the same (max - min) values.
struct Queue {
Expand All @@ -62,7 +49,7 @@ class ScaledRangeTimerManager {
// Typedef for convenience.
using Iterator = std::list<Item>::iterator;

Queue(std::chrono::milliseconds duration, ScaledRangeTimerManager& manager,
Queue(std::chrono::milliseconds duration, ScaledRangeTimerManagerImpl& manager,
Dispatcher& dispatcher);

// The (max - min) value for all timers in range_timers_.
Expand Down
6 changes: 3 additions & 3 deletions test/common/event/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ envoy_cc_test(
)

envoy_cc_test(
name = "scaled_range_timer_manager_test",
srcs = ["scaled_range_timer_manager_test.cc"],
name = "scaled_range_timer_manager_impl_test",
srcs = ["scaled_range_timer_manager_impl_test.cc"],
deps = [
"//source/common/event:scaled_range_timer_manager",
"//source/common/event:scaled_range_timer_manager_lib",
"//test/mocks/event:wrapped_dispatcher",
"//test/test_common:simulated_time_system_lib",
],
Expand Down
Loading