From fc424a0db90fd2d2e44e85a19750ad6eaa57b28a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 9 Dec 2022 13:21:45 -0500 Subject: SL-18809: Add WorkSchedule; remove timestamps from WorkQueue. For work queues that don't need timestamped tasks, eliminate the overhead of a priority queue ordered by timestamp. Timestamped task support moves to WorkSchedule. WorkQueue is a simpler queue that just waits for work. Both WorkQueue and WorkSchedule can be accessed via new WorkQueueBase API. Of course the WorkQueueBase API doesn't deal with timestamps, but a WorkSchedule can be accessed directly to post timestamped tasks and then handled normally (e.g. by ThreadPool) to run them. Most ThreadPool functionality migrates to new ThreadPoolBase class, with template subclass ThreadPoolUsing or ThreadPoolUsing depending on need. ThreadPool is now an alias for ThreadPoolUsing. Importantly, ThreadPoolUsing::getQueue() delivers a reference to the specific queue subclass type, so you can post timestamped tasks on a queue retrieved from ThreadPoolUsing::getQueue(). Since ThreadPool is no longer a simple class but an alias for a particular template specialization, introduce threadpool_fwd.h to forward-declare it. Recast workqueue_test.cpp to exercise WorkSchedule, since some of the tests are time-based. A future todo would be to exercise each applicable test with both WorkQueue and WorkSchedule. --- indra/llcommon/workqueue.cpp | 194 +++++++++++++++++++++++++++++++++---------- 1 file changed, 148 insertions(+), 46 deletions(-) (limited to 'indra/llcommon/workqueue.cpp') diff --git a/indra/llcommon/workqueue.cpp b/indra/llcommon/workqueue.cpp index eb06890468..83e0216ae7 100644 --- a/indra/llcommon/workqueue.cpp +++ b/indra/llcommon/workqueue.cpp @@ -26,83 +26,65 @@ using Mutex = LLCoros::Mutex; using Lock = LLCoros::LockType; -LL::WorkQueue::WorkQueue(const std::string& name, size_t capacity): - super(makeName(name)), - mQueue(capacity) +/***************************************************************************** +* WorkQueueBase +*****************************************************************************/ +LL::WorkQueueBase::WorkQueueBase(const std::string& name): + super(makeName(name)) { // TODO: register for "LLApp" events so we can implicitly close() on // viewer shutdown. } -void LL::WorkQueue::close() -{ - mQueue.close(); -} - -size_t LL::WorkQueue::size() -{ - return mQueue.size(); -} - -bool LL::WorkQueue::isClosed() -{ - return mQueue.isClosed(); -} - -bool LL::WorkQueue::done() -{ - return mQueue.done(); -} - -void LL::WorkQueue::runUntilClose() +void LL::WorkQueueBase::runUntilClose() { try { for (;;) { LL_PROFILE_ZONE_SCOPED_CATEGORY_THREAD; - callWork(mQueue.pop()); + callWork(pop_()); } } - catch (const Queue::Closed&) + catch (const Closed&) { } } -bool LL::WorkQueue::runPending() +bool LL::WorkQueueBase::runPending() { LL_PROFILE_ZONE_SCOPED_CATEGORY_THREAD; - for (Work work; mQueue.tryPop(work); ) + for (Work work; tryPop_(work); ) { callWork(work); } - return ! mQueue.done(); + return ! done(); } -bool LL::WorkQueue::runOne() +bool LL::WorkQueueBase::runOne() { Work work; - if (mQueue.tryPop(work)) + if (tryPop_(work)) { callWork(work); } - return ! mQueue.done(); + return ! done(); } -bool LL::WorkQueue::runUntil(const TimePoint& until) +bool LL::WorkQueueBase::runUntil(const TimePoint& until) { LL_PROFILE_ZONE_SCOPED_CATEGORY_THREAD; // Should we subtract some slop to allow for typical Work execution time? // How much slop? // runUntil() is simply a time-bounded runPending(). - for (Work work; TimePoint::clock::now() < until && mQueue.tryPop(work); ) + for (Work work; TimePoint::clock::now() < until && tryPop_(work); ) { callWork(work); } - return ! mQueue.done(); + return ! done(); } -std::string LL::WorkQueue::makeName(const std::string& name) +std::string LL::WorkQueueBase::makeName(const std::string& name) { if (! name.empty()) return name; @@ -120,14 +102,7 @@ std::string LL::WorkQueue::makeName(const std::string& name) return STRINGIZE("WorkQueue" << num); } -void LL::WorkQueue::callWork(const Queue::DataTuple& work) -{ - // ThreadSafeSchedule::pop() always delivers a tuple, even when - // there's only one data field per item, as for us. - callWork(std::get<0>(work)); -} - -void LL::WorkQueue::callWork(const Work& work) +void LL::WorkQueueBase::callWork(const Work& work) { LL_PROFILE_ZONE_SCOPED_CATEGORY_THREAD; try @@ -142,12 +117,12 @@ void LL::WorkQueue::callWork(const Work& work) } } -void LL::WorkQueue::error(const std::string& msg) +void LL::WorkQueueBase::error(const std::string& msg) { LL_ERRS("WorkQueue") << msg << LL_ENDL; } -void LL::WorkQueue::checkCoroutine(const std::string& method) +void LL::WorkQueueBase::checkCoroutine(const std::string& method) { // By convention, the default coroutine on each thread has an empty name // string. See also LLCoros::logname(). @@ -156,3 +131,130 @@ void LL::WorkQueue::checkCoroutine(const std::string& method) LLTHROW(Error("Do not call " + method + " from a thread's default coroutine")); } } + +/***************************************************************************** +* WorkQueue +*****************************************************************************/ +LL::WorkQueue::WorkQueue(const std::string& name, size_t capacity): + super(name), + mQueue(capacity) +{ +} + +void LL::WorkQueue::close() +{ + mQueue.close(); +} + +size_t LL::WorkQueue::size() +{ + return mQueue.size(); +} + +bool LL::WorkQueue::isClosed() +{ + return mQueue.isClosed(); +} + +bool LL::WorkQueue::done() +{ + return mQueue.done(); +} + +void LL::WorkQueue::post(const Work& callable) +{ + mQueue.push(callable); +} + +bool LL::WorkQueue::postIfOpen(const Work& callable) +{ + return mQueue.pushIfOpen(callable); +} + +bool LL::WorkQueue::tryPost(const Work& callable) +{ + return mQueue.tryPush(callable); +} + +LL::WorkQueue::Work LL::WorkQueue::pop_() +{ + return mQueue.pop(); +} + +bool LL::WorkQueue::tryPop_(Work& work) +{ + return mQueue.tryPop(work); +} + +/***************************************************************************** +* WorkSchedule +*****************************************************************************/ +LL::WorkSchedule::WorkSchedule(const std::string& name, size_t capacity): + super(name), + mQueue(capacity) +{ +} + +void LL::WorkSchedule::close() +{ + mQueue.close(); +} + +size_t LL::WorkSchedule::size() +{ + return mQueue.size(); +} + +bool LL::WorkSchedule::isClosed() +{ + return mQueue.isClosed(); +} + +bool LL::WorkSchedule::done() +{ + return mQueue.done(); +} + +void LL::WorkSchedule::post(const Work& callable) +{ + // Use TimePoint::clock::now() instead of TimePoint's representation of + // the epoch because this WorkSchedule may contain a mix of past-due + // TimedWork items and TimedWork items scheduled for the future. Sift this + // new item into the correct place. + post(callable, TimePoint::clock::now()); +} + +void LL::WorkSchedule::post(const Work& callable, const TimePoint& time) +{ + mQueue.push(TimedWork(time, callable)); +} + +bool LL::WorkSchedule::postIfOpen(const Work& callable) +{ + return postIfOpen(callable, TimePoint::clock::now()); +} + +bool LL::WorkSchedule::postIfOpen(const Work& callable, const TimePoint& time) +{ + return mQueue.pushIfOpen(TimedWork(time, callable)); +} + +bool LL::WorkSchedule::tryPost(const Work& callable) +{ + return tryPost(callable, TimePoint::clock::now()); +} + +bool LL::WorkSchedule::tryPost(const Work& callable, const TimePoint& time) +{ + return mQueue.tryPush(TimedWork(time, callable)); +} + +LL::WorkSchedule::Work LL::WorkSchedule::pop_() +{ + return std::get<0>(mQueue.pop()); +} + +bool LL::WorkSchedule::tryPop_(Work& work) +{ + return mQueue.tryPop(work); +} -- cgit v1.2.3 From 026ef1935dbdb21ab79159d38fb78e126dd6ac95 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 8 May 2023 12:07:31 -0400 Subject: SL-19690: Follow up on Rye Mutt's fix for shutdown crashes. Rather than continuing to propagate try/catch (Closed) (aka LLThreadSafeQueueInterrupt) constructs through the code base, make WorkQueueBase::post() return bool indicating success (i.e. ! isClosed()). This obviates postIfOpen(), which no one was using anyway. In effect, postIfOpen() is renamed post(), bypassing the exception when isClosed(). Review existing try/catch blocks of that sort, changing to test for post() returning false. --- indra/llcommon/workqueue.cpp | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) (limited to 'indra/llcommon/workqueue.cpp') diff --git a/indra/llcommon/workqueue.cpp b/indra/llcommon/workqueue.cpp index 83e0216ae7..5a77d517dd 100644 --- a/indra/llcommon/workqueue.cpp +++ b/indra/llcommon/workqueue.cpp @@ -161,12 +161,7 @@ bool LL::WorkQueue::done() return mQueue.done(); } -void LL::WorkQueue::post(const Work& callable) -{ - mQueue.push(callable); -} - -bool LL::WorkQueue::postIfOpen(const Work& callable) +bool LL::WorkQueue::post(const Work& callable) { return mQueue.pushIfOpen(callable); } @@ -215,26 +210,16 @@ bool LL::WorkSchedule::done() return mQueue.done(); } -void LL::WorkSchedule::post(const Work& callable) +bool LL::WorkSchedule::post(const Work& callable) { // Use TimePoint::clock::now() instead of TimePoint's representation of // the epoch because this WorkSchedule may contain a mix of past-due // TimedWork items and TimedWork items scheduled for the future. Sift this // new item into the correct place. - post(callable, TimePoint::clock::now()); -} - -void LL::WorkSchedule::post(const Work& callable, const TimePoint& time) -{ - mQueue.push(TimedWork(time, callable)); -} - -bool LL::WorkSchedule::postIfOpen(const Work& callable) -{ return postIfOpen(callable, TimePoint::clock::now()); } -bool LL::WorkSchedule::postIfOpen(const Work& callable, const TimePoint& time) +bool LL::WorkSchedule::post(const Work& callable, const TimePoint& time) { return mQueue.pushIfOpen(TimedWork(time, callable)); } -- cgit v1.2.3 From f728808d938666a01f73a039c861358fc4b02a9e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 8 May 2023 12:45:27 -0400 Subject: SL-19690: Fix a lingering reference to WorkSchedule::postIfOpen() --- indra/llcommon/workqueue.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon/workqueue.cpp') diff --git a/indra/llcommon/workqueue.cpp b/indra/llcommon/workqueue.cpp index 5a77d517dd..cf80ce0656 100644 --- a/indra/llcommon/workqueue.cpp +++ b/indra/llcommon/workqueue.cpp @@ -216,7 +216,7 @@ bool LL::WorkSchedule::post(const Work& callable) // the epoch because this WorkSchedule may contain a mix of past-due // TimedWork items and TimedWork items scheduled for the future. Sift this // new item into the correct place. - return postIfOpen(callable, TimePoint::clock::now()); + return post(callable, TimePoint::clock::now()); } bool LL::WorkSchedule::post(const Work& callable, const TimePoint& time) -- cgit v1.2.3