Skip to content

Commit

Permalink
EventBase::runAfterLoop()
Browse files Browse the repository at this point in the history
Summary:
There is a pattern of using `runInLoop()` with `thisIteration=true` to schedule a callback as late as possible in the loop and use it to batch-flush operations that were deferred by other callbacks.

This pattern may not behave as intended with `loopCallbacksTimeslice` because the scheduled callback is not guaranteed anymore to run within the same iteration.

This diff adds first-class support for the pattern with `runAfterLoop()` (symmetrical to `runBeforeLoop()`), which is guaranteed to run after the `runInLoop()` callbacks (so no rescheduling hacks needed), and is not subject to the timeslice (so it must be used with care).

It nicely complements `loopCallbacksTimeslice` for the aforementioned flush pattern, since it is possible to bound how long operations are deferred (modulo individually long-running callbacks).

Differential Revision: D58610940

fbshipit-source-id: ac7ba212f18fc3750d42242daf6ba699cdb5db87
  • Loading branch information
ot authored and facebook-github-bot committed Jun 16, 2024
1 parent 553668b commit 7e04ec7
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 2 deletions.
17 changes: 16 additions & 1 deletion folly/io/async/EventBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ EventBase::~EventBase() {
clearCobTimeouts();

DCHECK_EQ(0u, runBeforeLoopCallbacks_.size());
DCHECK_EQ(0u, runAfterLoopCallbacks_.size());

runLoopCallbacks();

Expand Down Expand Up @@ -587,7 +588,8 @@ EventBase::LoopStatus EventBase::loopMain(int flags, LoopOptions options) {
applyLoopKeepAlive();
}
++nextLoopCnt_;
// Run the before loop callbacks

// Run the before-loop callbacks
LoopCallbackList callbacks;
callbacks.swap(runBeforeLoopCallbacks_);
// Before-loop callbacks must by definition all run regardless of
Expand Down Expand Up @@ -625,6 +627,13 @@ EventBase::LoopStatus EventBase::loopMain(int flags, LoopOptions options) {

bool ranLoopCallbacks = runLoopCallbacks();

// Run the after-loop callback. Like the before-loop, no deadline.
{
LoopCallbackList callbacks;
callbacks.swap(runAfterLoopCallbacks_);
runLoopCallbackList(callbacks, LoopCallbacksDeadline{});
}

if (enableTimeMeasurement_) {
auto now = std::chrono::steady_clock::now();
auto busy = std::chrono::duration_cast<std::chrono::microseconds>(
Expand Down Expand Up @@ -872,6 +881,12 @@ void EventBase::runBeforeLoop(LoopCallback* callback) {
runBeforeLoopCallbacks_.push_back(*callback);
}

void EventBase::runAfterLoop(LoopCallback* callback) {
dcheckIsInEventBaseThread();
callback->cancelLoopCallback();
runAfterLoopCallbacks_.push_back(*callback);
}

void EventBase::runInEventBaseThread(Func fn) noexcept {
// Send the message.
// It will be received by the FunctionRunner in the EventBase's thread.
Expand Down
10 changes: 9 additions & 1 deletion folly/io/async/EventBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ class EventBase : public TimeoutManager,
* (each gets one timeslice per iteration). This can be used to prevent the
* queues to starve event handling or each other.
*
* Does not apply to runBeforeLoop() callbacks.
* Does not apply to runBeforeLoop() and runAfterLoop() callbacks.
*/
std::chrono::milliseconds loopCallbacksTimeslice{0};

Expand Down Expand Up @@ -612,6 +612,13 @@ class EventBase : public TimeoutManager,
*/
void runBeforeLoop(LoopCallback* callback);

/**
* Adds a callback that will run immediately *after* the event loop.
* This can be used to delay some processing until after all the normal loop
* callback have been processed for this iteration.
*/
void runAfterLoop(LoopCallback* callback);

/**
* Run the specified function in the EventBase's thread.
*
Expand Down Expand Up @@ -1070,6 +1077,7 @@ class EventBase : public TimeoutManager,

LoopCallbackList loopCallbacks_;
LoopCallbackList runBeforeLoopCallbacks_;
LoopCallbackList runAfterLoopCallbacks_;
Synchronized<OnDestructionCallback::List> onDestructionCallbacks_;
Synchronized<OnDestructionCallback::List> preDestructionCallbacks_;

Expand Down
12 changes: 12 additions & 0 deletions folly/io/async/test/EventBaseTestLib.h
Original file line number Diff line number Diff line change
Expand Up @@ -2207,6 +2207,17 @@ TYPED_TEST_P(EventBaseTest, RunBeforeLoopWait) {
ASSERT_EQ(cb.getCount(), 0);
}

TYPED_TEST_P(EventBaseTest, RunAfterLoop) {
auto evb = this->makeEventBase();
bool cb2Ran = false;
// cbRan is set by a callback scheduled after cb, but cb runs last anyway.
CountedLoopCallback cb(evb.get(), 1, [&] { EXPECT_TRUE(cb2Ran); });
evb->runAfterLoop(&cb);
evb->runInLoop([&] { cb2Ran = true; });
evb->loop();
ASSERT_EQ(cb.getCount(), 0);
}

namespace {
class PipeHandler : public EventHandler {
public:
Expand Down Expand Up @@ -2857,6 +2868,7 @@ REGISTER_TYPED_TEST_SUITE_P(
EventBaseThreadName,
RunBeforeLoop,
RunBeforeLoopWait,
RunAfterLoop,
StopBeforeLoop,
RunCallbacksPreDestruction,
RunCallbacksOnDestruction,
Expand Down

0 comments on commit 7e04ec7

Please sign in to comment.