Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

event: split dispatcherimpl into two classes #14533

Closed
wants to merge 6 commits into from
Closed

event: split dispatcherimpl into two classes #14533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8fbc7c7
Split DispatcherImpl into two classes
akonradi Dec 29, 2020
14e0485
Use optional instead of owned ptr for registration
akonradi Dec 29, 2020
8b0ea94
Fix formatting
akonradi Dec 29, 2020
4f5f8c1
Fix naming for runTid()
akonradi Dec 29, 2020
2686d7d
Move initialization to base
akonradi Dec 29, 2020
6883c11
Move post functionality to DispatcherImplBase
akonradi Dec 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions include/envoy/event/timer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class ScopeTrackedObject;

namespace Event {

class Dispatcher;
class DispatcherBase;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking forward to the scaled timer integration:
Do we expect to add scaled timers to the DispatcherBase or Dispatcher interface?

Not having access to scaled timers in the base interface would severely limit its potential use in more places like Network::ConnectionImpl implementation. How do we see Dispatcher interfaces evolving from here? Or is the only use case for DispatcherBase in the implementation of scaled timers?

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 was imagining them being in the Dispatcher interface. Network::ConnectionImpl would continue to depend on Dispatcher.

I don't have another use case yet for DispatcherBase. We could try transitioning some of the higher-level abstractions like the DNS resolver to depend only on DispatcherBase but if we try to migrate too much we'll just end up with DispatcherBase becoming the new Dispatcher.


/**
* Callback invoked when a timer event fires.
Expand Down Expand Up @@ -67,7 +67,7 @@ class Scheduler {
/**
* Creates a timer.
*/
virtual TimerPtr createTimer(const TimerCb& cb, Dispatcher& dispatcher) PURE;
virtual TimerPtr createTimer(const TimerCb& cb, DispatcherBase& dispatcher) PURE;
};

using SchedulerPtr = std::unique_ptr<Scheduler>;
Expand Down
4 changes: 2 additions & 2 deletions source/common/common/scope_tracker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Envoy {
// dispatcher at the previously tracked object.
class ScopeTrackerScopeState {
public:
ScopeTrackerScopeState(const ScopeTrackedObject* object, Event::Dispatcher& dispatcher)
ScopeTrackerScopeState(const ScopeTrackedObject* object, Event::DispatcherBase& dispatcher)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we keep requiring a fully Dispatcher when using this object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It gets called from the Timer, which is limited to the DispatcherBase.

: dispatcher_(dispatcher) {
latched_object_ = dispatcher_.setTrackedObject(object);
}
Expand All @@ -21,7 +21,7 @@ class ScopeTrackerScopeState {

private:
const ScopeTrackedObject* latched_object_;
Event::Dispatcher& dispatcher_;
Event::DispatcherBase& dispatcher_;
};

} // namespace Envoy
Loading