Skip to content
Merged
5 changes: 2 additions & 3 deletions include/envoy/event/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,10 @@ envoy_cc_library(
)

envoy_cc_library(
name = "range_timer_interface",
hdrs = ["range_timer.h"],
name = "scaled_range_timer_manager_interface",
hdrs = ["scaled_range_timer_manager.h"],
deps = [
":timer_interface",
"//include/envoy/common:time_interface",
],
)

Expand Down
47 changes: 0 additions & 47 deletions include/envoy/event/range_timer.h

This file was deleted.

91 changes: 91 additions & 0 deletions include/envoy/event/scaled_range_timer_manager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
#pragma once

#include "envoy/common/pure.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
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
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 that describes how to compute a minimum timeout given a maximum timeout value. It wraps
* ScaledMinimum and AbsoluteMinimum and provides a single computeMinimum() method.
*/
class ScaledTimerMinimum : private absl::variant<ScaledMinimum, AbsoluteMinimum> {
public:
// Use base class constructor.
using absl::variant<ScaledMinimum, AbsoluteMinimum>::variant;

// Computes the minimum value for a given maximum timeout. If this object was constructed with a
// - ScaledMinimum value:
// the return value is the scale factor applied to the provided maximum.
// - AbsoluteMinimum:
// the return value is that minimum, and the provided maximum is ignored.
std::chrono::milliseconds computeMinimum(std::chrono::milliseconds maximum) const {
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, const absl::variant<ScaledMinimum, AbsoluteMinimum>&>(
Visitor(maximum), *this);
}
};

/**
* Class for creating Timer 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 Timer 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 timer backed by the manager. 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;
Copy link
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
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
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
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 timers 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
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
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
9 changes: 5 additions & 4 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",
"//include/envoy/event:timer_interface",
"//source/common/common:scope_tracker",
],
)
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
#include "common/event/scaled_range_timer_manager.h"
#include "common/event/scaled_range_timer_manager_impl.h"

#include <chrono>
#include <cmath>
#include <memory>

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

#include "common/common/assert.h"
Expand All @@ -14,7 +13,7 @@ namespace Envoy {
namespace Event {

/**
* Implementation of RangeTimer that can be scaled by the backing manager object.
* Implementation of Timer that can be scaled by the backing manager object.
*
* Instances of this class exist in one of 3 states:
* - inactive: not enabled
Expand All @@ -31,10 +30,10 @@ 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 Timer {
public:
RangeTimerImpl(TimerCb callback, ScaledRangeTimerManager& manager)
: manager_(manager), callback_(std::move(callback)),
RangeTimerImpl(ScaledTimerMinimum minimum, TimerCb callback, ScaledRangeTimerManagerImpl& manager)
: minimum_(minimum), manager_(manager), callback_(std::move(callback)),
min_duration_timer_(manager.dispatcher_.createTimer([this] { onMinTimerComplete(); })) {}

~RangeTimerImpl() override { disableTimer(); }
Expand All @@ -52,23 +51,29 @@ class ScaledRangeTimerManager::RangeTimerImpl final : public RangeTimer {
scope_ = nullptr;
}

void enableTimer(const std::chrono::milliseconds min_ms, const std::chrono::milliseconds max_ms,
void enableTimer(const std::chrono::milliseconds max_ms,
const ScopeTrackedObject* scope) override {
disableTimer();
scope_ = scope;
ENVOY_LOG_MISC(trace, "enableTimer called on {} for ({}ms, {}ms)", static_cast<void*>(this),
min_ms.count(), max_ms.count());
const std::chrono::milliseconds min_ms = std::min(minimum_.computeMinimum(max_ms), max_ms);
ENVOY_LOG_MISC(trace, "enableTimer called on {} for {}ms, min is {}ms",
static_cast<void*>(this), max_ms.count(), min_ms.count());
if (min_ms <= std::chrono::milliseconds::zero()) {
// If the duration spread (max - min) is zero, skip over the waiting-for-min and straight to
// the scaling-max state.
auto handle = manager_.activateTimer(max_ms, *this);
state_.emplace<ScalingMax>(handle);
} else {
state_.emplace<WaitingForMin>(max_ms - min_ms);
min_duration_timer_->enableTimer(min_ms);
min_duration_timer_->enableTimer(std::min(max_ms, min_ms));
}
}

void enableHRTimer(std::chrono::microseconds us,
const ScopeTrackedObject* object = nullptr) override {
enableTimer(std::chrono::duration_cast<std::chrono::milliseconds>(us), object);
}

bool enabled() override { return !absl::holds_alternative<Inactive>(state_); }

void trigger() {
Expand Down Expand Up @@ -98,10 +103,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 +128,62 @@ class ScaledRangeTimerManager::RangeTimerImpl final : public RangeTimer {
}
}

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

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

ScaledRangeTimerManager::ScaledRangeTimerManager(Dispatcher& dispatcher)
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) {
return std::make_unique<RangeTimerImpl>(callback, *this);
TimerPtr ScaledRangeTimerManagerImpl::createTimer(ScaledTimerMinimum minimum, TimerCb callback) {
return std::make_unique<RangeTimerImpl>(minimum, callback, *this);
}

void ScaledRangeTimerManager::setScaleFactor(double scale_factor) {
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 +207,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 +226,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 +238,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
Loading