From 623ac79120a417ec445ce5c106a907fe46734309 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 7 Oct 2021 15:32:51 -0400 Subject: SL-16024: Add LL::WorkQueue for passing work items between threads. A typical WorkQueue has a string name, which can be used to find it to post work to it. "Work" is a nullary callable. WorkQueue is a multi-producer, multi-consumer thread-safe queue: multiple threads can service the WorkQueue, multiple threads can post work to it. Work can be scheduled in the future by submitting with a timestamp. In addition, a given work item can be scheduled to run on a recurring basis. A requesting thread servicing a WorkQueue of its own, such as the viewer's main thread, can submit work to another WorkQueue along with a callback to be passed the result (of arbitrary type) of the first work item. The callback is posted to the originating WorkQueue, permitting safe data exchange between participating threads. Methods are provided for different kinds of servicing threads. runUntilClose() is useful for a simple worker thread. runFor(duration) devotes no more than a specified time slice to that WorkQueue, e.g. for use by the main thread. --- indra/llcommon/tests/workqueue_test.cpp | 158 ++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 indra/llcommon/tests/workqueue_test.cpp (limited to 'indra/llcommon/tests/workqueue_test.cpp') diff --git a/indra/llcommon/tests/workqueue_test.cpp b/indra/llcommon/tests/workqueue_test.cpp new file mode 100644 index 0000000000..ab1cae6c14 --- /dev/null +++ b/indra/llcommon/tests/workqueue_test.cpp @@ -0,0 +1,158 @@ +/** + * @file workqueue_test.cpp + * @author Nat Goodspeed + * @date 2021-10-07 + * @brief Test for workqueue. + * + * $LicenseInfo:firstyear=2021&license=viewerlgpl$ + * Copyright (c) 2021, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "workqueue.h" +// STL headers +// std headers +#include +#include +// external library headers +// other Linden headers +#include "../test/lltut.h" +#include "llcond.h" +#include "llstring.h" +#include "stringize.h" + +using namespace LL; +using namespace std::literals::chrono_literals; // ms suffix +using namespace std::literals::string_literals; // s suffix + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct workqueue_data + { + WorkQueue queue{"queue"}; + }; + typedef test_group workqueue_group; + typedef workqueue_group::object object; + workqueue_group workqueuegrp("workqueue"); + + template<> template<> + void object::test<1>() + { + set_test_name("name"); + ensure_equals("didn't capture name", queue.getKey(), "queue"); + ensure("not findable", WorkQueue::getInstance("queue") == queue.getWeak().lock()); + WorkQueue q2; + ensure("has no name", LLStringUtil::startsWith(q2.getKey(), "WorkQueue")); + } + + template<> template<> + void object::test<2>() + { + set_test_name("post"); + bool wasRun{ false }; + // We only get away with binding a simple bool because we're running + // the work on the same thread. + queue.post([&wasRun](){ wasRun = true; }); + queue.close(); + ensure("ran too soon", ! wasRun); + queue.runUntilClose(); + ensure("didn't run", wasRun); + } + + template<> template<> + void object::test<3>() + { + set_test_name("postEvery"); + // record of runs + using Shared = std::deque; + // This is an example of how to share data between the originator of + // postEvery(work) and the work item itself, since usually a WorkQueue + // is used to dispatch work to a different thread. Neither of them + // should call any of LLCond's wait methods: you don't want to stall + // either the worker thread or the originating thread (conventionally + // main). Use LLCond or a subclass even if all you want to do is + // signal the work item that it can quit; consider LLOneShotCond. + LLCond data; + auto start = WorkQueue::TimePoint::clock::now(); + auto interval = 100ms; + queue.postEvery( + interval, + [&data, count = 0] + () mutable + { + // record the timestamp at which this instance is running + data.update_one( + [](Shared& data) + { + data.push_back(WorkQueue::TimePoint::clock::now()); + }); + // by the 3rd call, return false to stop + return (++count < 3); + }); + // no convenient way to close() our queue while we've got a + // postEvery() running, so run until we think we should have exhausted + // the iterations + queue.runFor(10*interval); + // Take a copy of the captured deque. + Shared result = data.get(); + ensure_equals("called wrong number of times", result.size(), 3); + // postEvery() assumes you want the first call to happen right away. + // Inject a fake start time that's (interval) earlier than that, to + // make our too early/too late tests uniform for all entries. + result.push_front(start - interval); + for (size_t i = 1; i < result.size(); ++i) + { + auto diff = (result[i] - result[i-1]); + try + { + ensure(STRINGIZE("call " << i << " too soon"), diff >= interval); + ensure(STRINGIZE("call " << i << " too late"), diff < interval*1.5); + } + catch (const tut::failure&) + { + auto interval_ms = interval / 1ms; + auto diff_ms = diff / 1ms; + std::cerr << "interval " << interval_ms + << "ms; diff " << diff_ms << "ms" << std::endl; + throw; + } + } + } + + template<> template<> + void object::test<4>() + { + set_test_name("postTo"); + WorkQueue main("main"); + auto qptr = WorkQueue::getInstance("queue"); + int result = 0; + main.postTo( + qptr, + [](){ return 17; }, + // Note that a postTo() *callback* can safely bind a reference to + // a variable on the invoking thread, because the callback is run + // on the invoking thread. + [&result](int i){ result = i; }); + // this should post the callback to main + qptr->runOne(); + // this should run the callback + main.runOne(); + ensure_equals("failed to run int callback", result, 17); + + std::string alpha; + // postTo() handles arbitrary return types + main.postTo( + qptr, + [](){ return "abc"s; }, + [&alpha](const std::string& s){ alpha = s; }); + qptr->runPending(); + main.runPending(); + ensure_equals("failed to run string callback", alpha, "abc"); + } +} // namespace tut -- cgit v1.2.3 From c585ddb75e383cdd994d0d99fed8f2de8f955e3c Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 7 Oct 2021 16:45:15 -0400 Subject: SL-16024: Defend against two threads making "anonymous" WorkQueues. Also make workqueue_test.cpp more robust. --- indra/llcommon/tests/workqueue_test.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'indra/llcommon/tests/workqueue_test.cpp') diff --git a/indra/llcommon/tests/workqueue_test.cpp b/indra/llcommon/tests/workqueue_test.cpp index ab1cae6c14..d5405400fd 100644 --- a/indra/llcommon/tests/workqueue_test.cpp +++ b/indra/llcommon/tests/workqueue_test.cpp @@ -103,12 +103,13 @@ namespace tut Shared result = data.get(); ensure_equals("called wrong number of times", result.size(), 3); // postEvery() assumes you want the first call to happen right away. - // Inject a fake start time that's (interval) earlier than that, to - // make our too early/too late tests uniform for all entries. - result.push_front(start - interval); - for (size_t i = 1; i < result.size(); ++i) + // Pretend our start time was (interval) earlier than that, to make + // our too early/too late tests uniform for all entries. + start -= interval; + for (size_t i = 0; i < result.size(); ++i) { - auto diff = (result[i] - result[i-1]); + auto diff = result[i] - start; + start += interval; try { ensure(STRINGIZE("call " << i << " too soon"), diff >= interval); -- cgit v1.2.3 From e7b8c27741201528bf78f95c96ba820833923dab Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 25 Oct 2021 15:55:49 -0400 Subject: SL-16220: Specialize WorkQueue for callable with void return. Add a test exercising this feature. --- indra/llcommon/tests/workqueue_test.cpp | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) (limited to 'indra/llcommon/tests/workqueue_test.cpp') diff --git a/indra/llcommon/tests/workqueue_test.cpp b/indra/llcommon/tests/workqueue_test.cpp index d5405400fd..b69df49d33 100644 --- a/indra/llcommon/tests/workqueue_test.cpp +++ b/indra/llcommon/tests/workqueue_test.cpp @@ -138,7 +138,8 @@ namespace tut [](){ return 17; }, // Note that a postTo() *callback* can safely bind a reference to // a variable on the invoking thread, because the callback is run - // on the invoking thread. + // on the invoking thread. (Of course the bound variable must + // survive until the callback is called.) [&result](int i){ result = i; }); // this should post the callback to main qptr->runOne(); @@ -156,4 +157,24 @@ namespace tut main.runPending(); ensure_equals("failed to run string callback", alpha, "abc"); } + + template<> template<> + void object::test<5>() + { + set_test_name("postTo with void return"); + WorkQueue main("main"); + auto qptr = WorkQueue::getInstance("queue"); + std::string observe; + main.postTo( + qptr, + // The ONLY reason we can get away with binding a reference to + // 'observe' in our work callable is because we're directly + // calling qptr->runOne() on this same thread. It would be a + // mistake to do that if some other thread were servicing 'queue'. + [&observe](){ observe = "queue"; }, + [&observe](){ observe.append(";main"); }); + qptr->runOne(); + main.runOne(); + ensure_equals("failed to run both lambdas", observe, "queue;main"); + } } // namespace tut -- cgit v1.2.3 From 8b16ecb9cfb4917fe38e4e5b0e4f40a23dd4ffbf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 27 Oct 2021 15:31:54 -0400 Subject: SL-16220: Add tests for WorkQueue::waitForResult(), void & non-void. --- indra/llcommon/tests/workqueue_test.cpp | 49 +++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) (limited to 'indra/llcommon/tests/workqueue_test.cpp') diff --git a/indra/llcommon/tests/workqueue_test.cpp b/indra/llcommon/tests/workqueue_test.cpp index b69df49d33..bea3ad911b 100644 --- a/indra/llcommon/tests/workqueue_test.cpp +++ b/indra/llcommon/tests/workqueue_test.cpp @@ -20,7 +20,10 @@ // external library headers // other Linden headers #include "../test/lltut.h" +#include "../test/catch_and_store_what_in.h" #include "llcond.h" +#include "llcoros.h" +#include "lleventcoro.h" #include "llstring.h" #include "stringize.h" @@ -177,4 +180,50 @@ namespace tut main.runOne(); ensure_equals("failed to run both lambdas", observe, "queue;main"); } + + template<> template<> + void object::test<6>() + { + set_test_name("waitForResult"); + std::string stored; + // Try to call waitForResult() on this thread's main coroutine. It + // should throw because the main coroutine must service the queue. + auto what{ catch_what( + [this, &stored](){ stored = queue.waitForResult( + [](){ return "should throw"; }); }) }; + ensure("lambda should not have run", stored.empty()); + ensure_not("waitForResult() should have thrown", what.empty()); + ensure(STRINGIZE("should mention waitForResult: " << what), + what.find("waitForResult") != std::string::npos); + + // Call waitForResult() on a coroutine, with a string result. + LLCoros::instance().launch( + "waitForResult string", + [this, &stored]() + { stored = queue.waitForResult( + [](){ return "string result"; }); }); + llcoro::suspend(); + // Nothing will have happened yet because, even if the coroutine did + // run immediately, all it did was to queue the inner lambda on + // 'queue'. Service it. + queue.runOne(); + llcoro::suspend(); + ensure_equals("bad waitForResult return", stored, "string result"); + + // Call waitForResult() on a coroutine, with a void callable. + stored.clear(); + bool done = false; + LLCoros::instance().launch( + "waitForResult void", + [this, &stored, &done]() + { + queue.waitForResult([&stored](){ stored = "ran"; }); + done = true; + }); + llcoro::suspend(); + queue.runOne(); + llcoro::suspend(); + ensure_equals("didn't run coroutine", stored, "ran"); + ensure("void waitForResult() didn't return", done); + } } // namespace tut -- cgit v1.2.3 From 029b41c0419e975bbb28454538b46dc69ce5d2ba Mon Sep 17 00:00:00 2001 From: Dave Houlton Date: Mon, 15 Nov 2021 09:25:35 -0700 Subject: Revert "SL-16220: Merge branch 'origin/DRTVWR-546' into glthread" This reverts commit 5188a26a8521251dda07ac0140bb129f28417e49, reversing changes made to 819088563e13f1d75e048311fbaf0df4a79b7e19. --- indra/llcommon/tests/workqueue_test.cpp | 72 +-------------------------------- 1 file changed, 1 insertion(+), 71 deletions(-) (limited to 'indra/llcommon/tests/workqueue_test.cpp') diff --git a/indra/llcommon/tests/workqueue_test.cpp b/indra/llcommon/tests/workqueue_test.cpp index bea3ad911b..d5405400fd 100644 --- a/indra/llcommon/tests/workqueue_test.cpp +++ b/indra/llcommon/tests/workqueue_test.cpp @@ -20,10 +20,7 @@ // external library headers // other Linden headers #include "../test/lltut.h" -#include "../test/catch_and_store_what_in.h" #include "llcond.h" -#include "llcoros.h" -#include "lleventcoro.h" #include "llstring.h" #include "stringize.h" @@ -141,8 +138,7 @@ namespace tut [](){ return 17; }, // Note that a postTo() *callback* can safely bind a reference to // a variable on the invoking thread, because the callback is run - // on the invoking thread. (Of course the bound variable must - // survive until the callback is called.) + // on the invoking thread. [&result](int i){ result = i; }); // this should post the callback to main qptr->runOne(); @@ -160,70 +156,4 @@ namespace tut main.runPending(); ensure_equals("failed to run string callback", alpha, "abc"); } - - template<> template<> - void object::test<5>() - { - set_test_name("postTo with void return"); - WorkQueue main("main"); - auto qptr = WorkQueue::getInstance("queue"); - std::string observe; - main.postTo( - qptr, - // The ONLY reason we can get away with binding a reference to - // 'observe' in our work callable is because we're directly - // calling qptr->runOne() on this same thread. It would be a - // mistake to do that if some other thread were servicing 'queue'. - [&observe](){ observe = "queue"; }, - [&observe](){ observe.append(";main"); }); - qptr->runOne(); - main.runOne(); - ensure_equals("failed to run both lambdas", observe, "queue;main"); - } - - template<> template<> - void object::test<6>() - { - set_test_name("waitForResult"); - std::string stored; - // Try to call waitForResult() on this thread's main coroutine. It - // should throw because the main coroutine must service the queue. - auto what{ catch_what( - [this, &stored](){ stored = queue.waitForResult( - [](){ return "should throw"; }); }) }; - ensure("lambda should not have run", stored.empty()); - ensure_not("waitForResult() should have thrown", what.empty()); - ensure(STRINGIZE("should mention waitForResult: " << what), - what.find("waitForResult") != std::string::npos); - - // Call waitForResult() on a coroutine, with a string result. - LLCoros::instance().launch( - "waitForResult string", - [this, &stored]() - { stored = queue.waitForResult( - [](){ return "string result"; }); }); - llcoro::suspend(); - // Nothing will have happened yet because, even if the coroutine did - // run immediately, all it did was to queue the inner lambda on - // 'queue'. Service it. - queue.runOne(); - llcoro::suspend(); - ensure_equals("bad waitForResult return", stored, "string result"); - - // Call waitForResult() on a coroutine, with a void callable. - stored.clear(); - bool done = false; - LLCoros::instance().launch( - "waitForResult void", - [this, &stored, &done]() - { - queue.waitForResult([&stored](){ stored = "ran"; }); - done = true; - }); - llcoro::suspend(); - queue.runOne(); - llcoro::suspend(); - ensure_equals("didn't run coroutine", stored, "ran"); - ensure("void waitForResult() didn't return", done); - } } // namespace tut -- cgit v1.2.3 From 0b066539fe68dc5750900c3452189645c40adb45 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 24 Nov 2021 10:47:54 -0500 Subject: DRTVWR-546, SL-16220, SL-16094: Undo previous glthread branch revert. Reverting a merge is sticky: it tells git you never want to see that branch again. Merging the DRTVWR-546 branch, which contained the revert, into the glthread branch undid much of the development work on that branch. To restore it we must revert the revert. This reverts commit 029b41c0419e975bbb28454538b46dc69ce5d2ba. --- indra/llcommon/tests/workqueue_test.cpp | 72 ++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) (limited to 'indra/llcommon/tests/workqueue_test.cpp') diff --git a/indra/llcommon/tests/workqueue_test.cpp b/indra/llcommon/tests/workqueue_test.cpp index d5405400fd..bea3ad911b 100644 --- a/indra/llcommon/tests/workqueue_test.cpp +++ b/indra/llcommon/tests/workqueue_test.cpp @@ -20,7 +20,10 @@ // external library headers // other Linden headers #include "../test/lltut.h" +#include "../test/catch_and_store_what_in.h" #include "llcond.h" +#include "llcoros.h" +#include "lleventcoro.h" #include "llstring.h" #include "stringize.h" @@ -138,7 +141,8 @@ namespace tut [](){ return 17; }, // Note that a postTo() *callback* can safely bind a reference to // a variable on the invoking thread, because the callback is run - // on the invoking thread. + // on the invoking thread. (Of course the bound variable must + // survive until the callback is called.) [&result](int i){ result = i; }); // this should post the callback to main qptr->runOne(); @@ -156,4 +160,70 @@ namespace tut main.runPending(); ensure_equals("failed to run string callback", alpha, "abc"); } + + template<> template<> + void object::test<5>() + { + set_test_name("postTo with void return"); + WorkQueue main("main"); + auto qptr = WorkQueue::getInstance("queue"); + std::string observe; + main.postTo( + qptr, + // The ONLY reason we can get away with binding a reference to + // 'observe' in our work callable is because we're directly + // calling qptr->runOne() on this same thread. It would be a + // mistake to do that if some other thread were servicing 'queue'. + [&observe](){ observe = "queue"; }, + [&observe](){ observe.append(";main"); }); + qptr->runOne(); + main.runOne(); + ensure_equals("failed to run both lambdas", observe, "queue;main"); + } + + template<> template<> + void object::test<6>() + { + set_test_name("waitForResult"); + std::string stored; + // Try to call waitForResult() on this thread's main coroutine. It + // should throw because the main coroutine must service the queue. + auto what{ catch_what( + [this, &stored](){ stored = queue.waitForResult( + [](){ return "should throw"; }); }) }; + ensure("lambda should not have run", stored.empty()); + ensure_not("waitForResult() should have thrown", what.empty()); + ensure(STRINGIZE("should mention waitForResult: " << what), + what.find("waitForResult") != std::string::npos); + + // Call waitForResult() on a coroutine, with a string result. + LLCoros::instance().launch( + "waitForResult string", + [this, &stored]() + { stored = queue.waitForResult( + [](){ return "string result"; }); }); + llcoro::suspend(); + // Nothing will have happened yet because, even if the coroutine did + // run immediately, all it did was to queue the inner lambda on + // 'queue'. Service it. + queue.runOne(); + llcoro::suspend(); + ensure_equals("bad waitForResult return", stored, "string result"); + + // Call waitForResult() on a coroutine, with a void callable. + stored.clear(); + bool done = false; + LLCoros::instance().launch( + "waitForResult void", + [this, &stored, &done]() + { + queue.waitForResult([&stored](){ stored = "ran"; }); + done = true; + }); + llcoro::suspend(); + queue.runOne(); + llcoro::suspend(); + ensure_equals("didn't run coroutine", stored, "ran"); + ensure("void waitForResult() didn't return", done); + } } // namespace tut -- cgit v1.2.3 From 04ebc11a2d8a2e59abda5061e35e504fc30504d2 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 24 Nov 2021 12:56:48 -0500 Subject: SL-16094: Fix WorkQueue test for correct behavior of runFor(). Turns out that one of our WorkQueue integration tests was relying on the incorrect runFor() behavior that we just fixed, so the test broke. Now that runFor() doesn't wait around for work to be posted, use an explicit wait loop instead. To support this, add LLCond::get(functor), where functor must accept a const reference to the stored data. This new get() returns whatever the functor returns, allowing a caller to peek at the stored data. Also use universal references for all remaining LLCond functor arguments. --- indra/llcommon/tests/workqueue_test.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'indra/llcommon/tests/workqueue_test.cpp') diff --git a/indra/llcommon/tests/workqueue_test.cpp b/indra/llcommon/tests/workqueue_test.cpp index bea3ad911b..1d73f7aa0d 100644 --- a/indra/llcommon/tests/workqueue_test.cpp +++ b/indra/llcommon/tests/workqueue_test.cpp @@ -99,9 +99,15 @@ namespace tut return (++count < 3); }); // no convenient way to close() our queue while we've got a - // postEvery() running, so run until we think we should have exhausted - // the iterations - queue.runFor(10*interval); + // postEvery() running, so run until we have exhausted the iterations + // or we time out waiting + for (auto finish = start + 10*interval; + WorkQueue::TimePoint::clock::now() < finish && + data.get([](const Shared& data){ return data.size(); }) < 3; ) + { + queue.runPending(); + std::this_thread::sleep_for(interval/10); + } // Take a copy of the captured deque. Shared result = data.get(); ensure_equals("called wrong number of times", result.size(), 3); -- cgit v1.2.3