From df41a3a7dfe7f3bc69eabef3a4afe0e9f2a8d8e1 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Aug 2023 09:14:54 -0400 Subject: DRTVWR-588: Finally ditch LL_USE_SYSTEM_RAND code in llrand.cpp. This conditional code hasn't been used since June 2008, possibly even earlier. --- indra/llcommon/llrand.cpp | 104 +++++++++++++++++----------------------------- 1 file changed, 39 insertions(+), 65 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index cb28a8f5c3..33afc50cf7 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -58,46 +58,14 @@ * to restore uniform distribution. */ -// *NOTE: The system rand implementation is probably not correct. -#define LL_USE_SYSTEM_RAND 0 +static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); -#if LL_USE_SYSTEM_RAND -#include -#endif +// no default implementation, only specific F64 and F32 specializations +template +inline REAL ll_internal_random(); -#if LL_USE_SYSTEM_RAND -class LLSeedRand -{ -public: - LLSeedRand() - { -#if LL_WINDOWS - srand(LLUUID::getRandomSeed()); -#else - srand48(LLUUID::getRandomSeed()); -#endif - } -}; -static LLSeedRand sRandomSeeder; -inline F64 ll_internal_random_double() -{ -#if LL_WINDOWS - return (F64)rand() / (F64)RAND_MAX; -#else - return drand48(); -#endif -} -inline F32 ll_internal_random_float() -{ -#if LL_WINDOWS - return (F32)rand() / (F32)RAND_MAX; -#else - return (F32)drand48(); -#endif -} -#else -static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); -inline F64 ll_internal_random_double() +template <> +inline F64 ll_internal_random() { // *HACK: Through experimentation, we have found that dual core // CPUs (or at least multi-threaded processes) seem to @@ -108,15 +76,35 @@ inline F64 ll_internal_random_double() return rv; } +template <> +inline F32 ll_internal_random() +{ + return F32(ll_internal_random()); +} + +/*------------------------------ F64 aliases -------------------------------*/ +inline F64 ll_internal_random_double() +{ + return ll_internal_random(); +} + +F64 ll_drand() +{ + return ll_internal_random_double(); +} + +/*------------------------------ F32 aliases -------------------------------*/ inline F32 ll_internal_random_float() { - // The clamping rules are described above. - F32 rv = (F32)gRandomGenerator(); - if(!((rv >= 0.0f) && (rv < 1.0f))) return fmod(rv, 1.f); - return rv; + return ll_internal_random(); +} + +F32 ll_frand() +{ + return ll_internal_random_float(); } -#endif +/*-------------------------- clamped random range --------------------------*/ S32 ll_rand() { return ll_rand(RAND_MAX); @@ -130,42 +118,28 @@ S32 ll_rand(S32 val) return rv; } -F32 ll_frand() -{ - return ll_internal_random_float(); -} - -F32 ll_frand(F32 val) +template +REAL ll_grand(REAL val) { // The clamping rules are described above. - F32 rv = ll_internal_random_float() * val; + REAL rv = ll_internal_random() * val; if(val > 0) { - if(rv >= val) return 0.0f; + if(rv >= val) return REAL(); } else { - if(rv <= val) return 0.0f; + if(rv <= val) return REAL(); } return rv; } -F64 ll_drand() +F32 ll_frand(F32 val) { - return ll_internal_random_double(); + return ll_grand(val); } F64 ll_drand(F64 val) { - // The clamping rules are described above. - F64 rv = ll_internal_random_double() * val; - if(val > 0) - { - if(rv >= val) return 0.0; - } - else - { - if(rv <= val) return 0.0; - } - return rv; + return ll_grand(val); } -- cgit v1.2.3 From 0f8b8fd7a338272e3464e809b94eb0443d99e275 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Aug 2023 09:28:53 -0400 Subject: DRTVWR-588: Eliminate APR and Boost.Phoenix from NamedTempFile. NamedTempFile used to use APR calls to discover a suitable temp directory, synthesize a temp filename template, generate the unique file, write its content and ultimately delete it. This required a reference to gAPRPoolp as the default value of an optional constructor argument in case some usage demanded an alternative APR memory pool. It also used Boost.Phoenix placeholders to magically synthesize a callable. Replace APR calls with Boost.Filesystem; replace Boost.Phoenix with lambdas. Break out unique path generation logic as static NamedTempFile::temp_path(). In a nod to GitHub Actions builds, honor RUNNER_TEMP environment variable if set. test.cpp's RecordToTempFile need no longer pass an apr_pool_t* to NamedTempFile. NamedTempFile's constructor now accepts an optional suffix, making subclass NamedExtTempFile nearly trivial. It no longer needs to create or remove a symlink, for which it used to use APR calls. llprocess_test.cpp's NamedTempDir used to use Python's tempfile.mkdtemp() to create a temp directory, and apr_dir_remove() to destroy it. Replace both with NamedTempFile::temp_path() and Boost.Filesystem. Also add diagnostic output for LLProcess test failure. If llprocess_test cannot launch a child process, notice the APR_LOG environment variable recognized by our patched apr_suite to engage logging, and report the contents of that file. --- indra/llcommon/tests/llprocess_test.cpp | 70 ++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 19 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 81449b4a42..c48d913069 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -151,8 +151,38 @@ struct PythonProcessLauncher /// Launch Python script; verify that it launched void launch() { - mPy = LLProcess::create(mParams); - tut::ensure(STRINGIZE("Couldn't launch " << mDesc << " script"), bool(mPy)); + try + { + mPy = LLProcess::create(mParams); + tut::ensure(STRINGIZE("Couldn't launch " << mDesc << " script"), bool(mPy)); + } + catch (const tut::failure&) + { + // On Windows, if APR_LOG is set, our version of APR's + // apr_create_proc() logs to the specified file. If this test + // failed, try to report that log. + const char* APR_LOG = getenv("APR_LOG"); + if (APR_LOG && *APR_LOG) + { + std::ifstream inf(APR_LOG); + if (! inf.is_open()) + { + LL_WARNS() << "Couldn't open '" << APR_LOG << "'" << LL_ENDL; + } + else + { + LL_WARNS() << "==============================" << LL_ENDL; + LL_WARNS() << "From '" << APR_LOG << "':" << LL_ENDL; + std::string line; + while (std::getline(inf, line)) + { + LL_WARNS() << line << LL_ENDL; + } + LL_WARNS() << "==============================" << LL_ENDL; + } + } + throw; + } } /// Run Python script and wait for it to complete. @@ -214,30 +244,26 @@ static std::string python_out(const std::string& desc, const CONTENT& script) class NamedTempDir: public boost::noncopyable { public: - // Use python() function to create a temp directory: I've found - // nothing in either Boost.Filesystem or APR quite like Python's - // tempfile.mkdtemp(). - // Special extra bonus: on Mac, mkdtemp() reports a pathname - // starting with /var/folders/something, whereas that's really a - // symlink to /private/var/folders/something. Have to use - // realpath() to compare properly. NamedTempDir(): - mPath(python_out("mkdtemp()", - "from __future__ import with_statement\n" - "import os.path, sys, tempfile\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write(os.path.normcase(os.path.normpath(os.path.realpath(tempfile.mkdtemp()))))\n")) - {} + mPath(NamedTempFile::temp_path()), + mCreated(boost::filesystem::create_directories(mPath)) + { + mPath = boost::filesystem::canonical(mPath); + } ~NamedTempDir() { - aprchk(apr_dir_remove(mPath.c_str(), gAPRPoolp)); + if (mCreated) + { + boost::filesystem::remove_all(mPath); + } } - std::string getName() const { return mPath; } + std::string getName() const { return mPath.string(); } private: - std::string mPath; + boost::filesystem::path mPath; + bool mCreated; }; /***************************************************************************** @@ -565,7 +591,13 @@ namespace tut " f.write(os.path.normcase(os.path.normpath(os.getcwd())))\n"); // Before running, call setWorkingDirectory() py.mParams.cwd = tempdir.getName(); - ensure_equals("os.getcwd()", py.run_read(), tempdir.getName()); + std::string expected{ tempdir.getName() }; +#if LL_WINDOWS + // SIGH, don't get tripped up by "C:" != "c:" -- + // but on the Mac, using tolower() fails because "/users" != "/Users"! + expected = utf8str_tolower(expected); +#endif + ensure_equals("os.getcwd()", py.run_read(), expected); } template<> template<> -- cgit v1.2.3 From 48759438b747c24b536cd099d65ab0c9ea720a38 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Aug 2023 09:53:01 -0400 Subject: DRTVWR-588: Remove Boost Phoenix, Bind and Assign from some tests. llsdserialize_test used Boost.Foreach, Boost.Function and Boost.Bind. llleap_test used Boost.Assign. Both used Boost.Phoenix. Replace Boost.Foreach with range 'for'. Replace Boost.Function with std::function. Replace Boost.Assign with initializer lists. Replace Boost.Bind and Boost.Phoenix with lambdas. --- indra/llcommon/tests/llleap_test.cpp | 251 ++++++++++++++-------------- indra/llcommon/tests/llsdserialize_test.cpp | 39 ++--- 2 files changed, 134 insertions(+), 156 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 7ee36a9ea6..671873e982 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -17,8 +17,6 @@ // std headers #include // external library headers -#include -#include // other Linden headers #include "../test/lltut.h" #include "../test/namedtempfile.h" @@ -30,10 +28,6 @@ #include "stringize.h" #include "StringVec.h" -using boost::assign::list_of; - -StringVec sv(const StringVec& listof) { return listof; } - #if defined(LL_WINDOWS) #define sleep(secs) _sleep((secs) * 1000) @@ -104,7 +98,7 @@ namespace tut llleap_data(): reader(".py", // This logic is adapted from vita.viewerclient.receiveEvent() - boost::phoenix::placeholders::arg1 << + [](std::ostream& out){ out << "import re\n" "import os\n" "import sys\n" @@ -188,7 +182,7 @@ namespace tut "def request(pump, data):\n" " # we expect 'data' is a dict\n" " data['reply'] = _reply\n" - " send(pump, data)\n"), + " send(pump, data)\n";}), // Get the actual pathname of the NamedExtTempFile and trim off // the ".py" extension. (We could cache reader.getName() in a // separate member variable, but I happen to know getName() just @@ -213,14 +207,14 @@ namespace tut void object::test<1>() { set_test_name("multiple LLLeap instances"); - NamedTempFile script("py", - "import time\n" - "time.sleep(1)\n"); + NamedExtTempFile script("py", + "import time\n" + "time.sleep(1)\n"); LLLeapVector instances; instances.push_back(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))->getWeak()); + StringVec{PYTHON, script.getName()})->getWeak()); instances.push_back(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))->getWeak()); + StringVec{PYTHON, script.getName()})->getWeak()); // In this case we're simply establishing that two LLLeap instances // can coexist without throwing exceptions or bombing in any other // way. Wait for them to terminate. @@ -231,10 +225,10 @@ namespace tut void object::test<2>() { set_test_name("stderr to log"); - NamedTempFile script("py", - "import sys\n" - "sys.stderr.write('''Hello from Python!\n" - "note partial line''')\n"); + NamedExtTempFile script("py", + "import sys\n" + "sys.stderr.write('''Hello from Python!\n" + "note partial line''')\n"); StringVec vcommand{ PYTHON, script.getName() }; CaptureLog log(LLError::LEVEL_INFO); waitfor(LLLeap::create(get_test_name(), vcommand)); @@ -246,11 +240,11 @@ namespace tut void object::test<3>() { set_test_name("bad stdout protocol"); - NamedTempFile script("py", - "print('Hello from Python!')\n"); + NamedExtTempFile script("py", + "print('Hello from Python!')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + StringVec{PYTHON, script.getName()})); ensure_contains("error log line", log.messageWith("invalid protocol"), "Hello from Python!"); } @@ -259,13 +253,13 @@ namespace tut void object::test<4>() { set_test_name("leftover stdout"); - NamedTempFile script("py", - "import sys\n" - // note lack of newline - "sys.stdout.write('Hello from Python!')\n"); + NamedExtTempFile script("py", + "import sys\n" + // note lack of newline + "sys.stdout.write('Hello from Python!')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + StringVec{PYTHON, script.getName()})); ensure_contains("error log line", log.messageWith("Discarding"), "Hello from Python!"); } @@ -274,12 +268,12 @@ namespace tut void object::test<5>() { set_test_name("bad stdout len prefix"); - NamedTempFile script("py", - "import sys\n" - "sys.stdout.write('5a2:something')\n"); + NamedExtTempFile script("py", + "import sys\n" + "sys.stdout.write('5a2:something')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + StringVec{PYTHON, script.getName()})); ensure_contains("error log line", log.messageWith("invalid protocol"), "5a2:"); } @@ -381,17 +375,17 @@ namespace tut set_test_name("round trip"); AckAPI api; Result result; - NamedTempFile script("py", - boost::phoenix::placeholders::arg1 << - "from " << reader_module << " import *\n" - // make a request on our little API - "request(pump='" << api.getName() << "', data={})\n" - // wait for its response - "resp = get()\n" - "result = '' if resp == dict(pump=replypump(), data='ack')\\\n" - " else 'bad: ' + str(resp)\n" - "send(pump='" << result.getName() << "', data=result)\n"); - waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); + NamedExtTempFile script("py", + [&](std::ostream& out){ out << + "from " << reader_module << " import *\n" + // make a request on our little API + "request(pump='" << api.getName() << "', data={})\n" + // wait for its response + "resp = get()\n" + "result = '' if resp == dict(pump=replypump(), data='ack')\\\n" + " else 'bad: ' + str(resp)\n" + "send(pump='" << result.getName() << "', data=result)\n";}); + waitfor(LLLeap::create(get_test_name(), StringVec{PYTHON, script.getName()})); result.ensure(); } @@ -419,38 +413,38 @@ namespace tut // iterations etc. in OS pipes and the LLLeap/LLProcess implementation. ReqIDAPI api; Result result; - NamedTempFile script("py", - boost::phoenix::placeholders::arg1 << - "import sys\n" - "from " << reader_module << " import *\n" - // Note that since reader imports llsd, this - // 'import *' gets us llsd too. - "sample = llsd.format_notation(dict(pump='" << - api.getName() << "', data=dict(reqid=999999, reply=replypump())))\n" - // The whole packet has length prefix too: "len:data" - "samplen = len(str(len(sample))) + 1 + len(sample)\n" - // guess how many messages it will take to - // accumulate BUFFERED_LENGTH - "count = int(" << BUFFERED_LENGTH << "/samplen)\n" - "print('Sending %s requests' % count, file=sys.stderr)\n" - "for i in range(count):\n" - " request('" << api.getName() << "', dict(reqid=i))\n" - // The assumption in this specific test that - // replies will arrive in the same order as - // requests is ONLY valid because the API we're - // invoking sends replies instantly. If the API - // had to wait for some external event before - // sending its reply, replies could arrive in - // arbitrary order, and we'd have to tick them - // off from a set. - "result = ''\n" - "for i in range(count):\n" - " resp = get()\n" - " if resp['data']['reqid'] != i:\n" - " result = 'expected reqid=%s in %s' % (i, resp)\n" - " break\n" - "send(pump='" << result.getName() << "', data=result)\n"); - waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName()))), + NamedExtTempFile script("py", + [&](std::ostream& out){ out << + "import sys\n" + "from " << reader_module << " import *\n" + // Note that since reader imports llsd, this + // 'import *' gets us llsd too. + "sample = llsd.format_notation(dict(pump='" << + api.getName() << "', data=dict(reqid=999999, reply=replypump())))\n" + // The whole packet has length prefix too: "len:data" + "samplen = len(str(len(sample))) + 1 + len(sample)\n" + // guess how many messages it will take to + // accumulate BUFFERED_LENGTH + "count = int(" << BUFFERED_LENGTH << "/samplen)\n" + "print('Sending %s requests' % count, file=sys.stderr)\n" + "for i in range(count):\n" + " request('" << api.getName() << "', dict(reqid=i))\n" + // The assumption in this specific test that + // replies will arrive in the same order as + // requests is ONLY valid because the API we're + // invoking sends replies instantly. If the API + // had to wait for some external event before + // sending its reply, replies could arrive in + // arbitrary order, and we'd have to tick them + // off from a set. + "result = ''\n" + "for i in range(count):\n" + " resp = get()\n" + " if resp['data']['reqid'] != i:\n" + " result = 'expected reqid=%s in %s' % (i, resp)\n" + " break\n" + "send(pump='" << result.getName() << "', data=result)\n";}); + waitfor(LLLeap::create(get_test_name(), StringVec{PYTHON, script.getName()}), 300); // needs more realtime than most tests result.ensure(); } @@ -462,65 +456,62 @@ namespace tut { ReqIDAPI api; Result result; - NamedTempFile script("py", - boost::phoenix::placeholders::arg1 << - "import sys\n" - "from " << reader_module << " import *\n" - // Generate a very large string value. - "desired = int(sys.argv[1])\n" - // 7 chars per item: 6 digits, 1 comma - "count = int((desired - 50)/7)\n" - "large = ''.join('%06d,' % i for i in range(count))\n" - // Pass 'large' as reqid because we know the API - // will echo reqid, and we want to receive it back. - "request('" << api.getName() << "', dict(reqid=large))\n" - "try:\n" - " resp = get()\n" - "except ParseError as e:\n" - " # try to find where e.data diverges from expectation\n" - // Normally we'd expect a 'pump' key in there, - // too, with value replypump(). But Python - // serializes keys in a different order than C++, - // so incoming data start with 'data'. - // Truthfully, though, if we get as far as 'pump' - // before we find a difference, something's very - // strange. - " expect = llsd.format_notation(dict(data=dict(reqid=large)))\n" - " chunk = 40\n" - " for offset in range(0, max(len(e.data), len(expect)), chunk):\n" - " if e.data[offset:offset+chunk] != \\\n" - " expect[offset:offset+chunk]:\n" - " print('Offset %06d: expect %r,\\n'\\\n" - " ' get %r' %\\\n" - " (offset,\n" - " expect[offset:offset+chunk],\n" - " e.data[offset:offset+chunk]),\n" - " file=sys.stderr)\n" - " break\n" - " else:\n" - " print('incoming data matches expect?!', file=sys.stderr)\n" - " send('" << result.getName() << "', '%s: %s' % (e.__class__.__name__, e))\n" - " sys.exit(1)\n" - "\n" - "echoed = resp['data']['reqid']\n" - "if echoed == large:\n" - " send('" << result.getName() << "', '')\n" - " sys.exit(0)\n" - // Here we know echoed did NOT match; try to find where - "for i in range(count):\n" - " start = 7*i\n" - " end = 7*(i+1)\n" - " if end > len(echoed)\\\n" - " or echoed[start:end] != large[start:end]:\n" - " send('" << result.getName() << "',\n" - " 'at offset %s, expected %r but got %r' %\n" - " (start, large[start:end], echoed[start:end]))\n" - "sys.exit(1)\n"); + NamedExtTempFile script("py", + [&](std::ostream& out){ out << + "import sys\n" + "from " << reader_module << " import *\n" + // Generate a very large string value. + "desired = int(sys.argv[1])\n" + // 7 chars per item: 6 digits, 1 comma + "count = int((desired - 50)/7)\n" + "large = ''.join('%06d,' % i for i in range(count))\n" + // Pass 'large' as reqid because we know the API + // will echo reqid, and we want to receive it back. + "request('" << api.getName() << "', dict(reqid=large))\n" + "try:\n" + " resp = get()\n" + "except ParseError as e:\n" + " # try to find where e.data diverges from expectation\n" + // Normally we'd expect a 'pump' key in there, + // too, with value replypump(). But Python + // serializes keys in a different order than C++, + // so incoming data start with 'data'. + // Truthfully, though, if we get as far as 'pump' + // before we find a difference, something's very + // strange. + " expect = llsd.format_notation(dict(data=dict(reqid=large)))\n" + " chunk = 40\n" + " for offset in range(0, max(len(e.data), len(expect)), chunk):\n" + " if e.data[offset:offset+chunk] != \\\n" + " expect[offset:offset+chunk]:\n" + " print('Offset %06d: expect %r,\\n'\\\n" + " ' get %r' %\\\n" + " (offset,\n" + " expect[offset:offset+chunk],\n" + " e.data[offset:offset+chunk]),\n" + " file=sys.stderr)\n" + " break\n" + " else:\n" + " print('incoming data matches expect?!', file=sys.stderr)\n" + " send('" << result.getName() << "', '%s: %s' % (e.__class__.__name__, e))\n" + " sys.exit(1)\n" + "\n" + "echoed = resp['data']['reqid']\n" + "if echoed == large:\n" + " send('" << result.getName() << "', '')\n" + " sys.exit(0)\n" + // Here we know echoed did NOT match; try to find where + "for i in range(count):\n" + " start = 7*i\n" + " end = 7*(i+1)\n" + " if end > len(echoed)\\\n" + " or echoed[start:end] != large[start:end]:\n" + " send('" << result.getName() << "',\n" + " 'at offset %s, expected %r but got %r' %\n" + " (start, large[start:end], echoed[start:end]))\n" + "sys.exit(1)\n";}); waitfor(LLLeap::create(test_name, - sv(list_of - (PYTHON) - (script.getName()) - (stringize(size)))), + StringVec{PYTHON, script.getName(), stringize(size)}), 180); // try a longer timeout result.ensure(); } diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 5dbcf4c9b8..f46682e2d7 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -44,18 +44,10 @@ typedef U32 uint32_t; #include "llstring.h" #endif -#include "boost/range.hpp" -#include "boost/foreach.hpp" -#include "boost/function.hpp" -#include "boost/bind.hpp" -#include "boost/phoenix/bind/bind_function.hpp" -#include "boost/phoenix/core/argument.hpp" -using namespace boost::phoenix; - -#include "../llsd.h" -#include "../llsdserialize.h" +#include "llsd.h" +#include "llsdserialize.h" #include "llsdutil.h" -#include "../llformat.h" +#include "llformat.h" #include "../test/lltut.h" #include "../test/namedtempfile.h" @@ -1697,13 +1689,6 @@ namespace tut struct TestPythonCompatible { TestPythonCompatible(): - // Note the peculiar insertion of __FILE__ into this string. Since - // this script is being written into a platform-dependent temp - // directory, we can't locate indra/lib/python relative to - // Python's __file__. Use __FILE__ instead, navigating relative - // to this C++ source file. Use Python raw-string syntax so - // Windows pathname backslashes won't mislead Python's string - // scanner. import_llsd("import os.path\n" "import sys\n" "from llbase import llsd\n") @@ -1801,7 +1786,7 @@ namespace tut // helper for test<3> static void writeLLSDArray(std::ostream& out, const LLSD& array) { - BOOST_FOREACH(LLSD item, llsd::inArray(array)) + for (LLSD item: llsd::inArray(array)) { LLSDSerialize::toNotation(item, out); // It's important to separate with newlines because Python's llsd @@ -1841,21 +1826,22 @@ namespace tut // Create an llsdXXXXXX file containing 'data' serialized to // notation. NamedTempFile file("llsd", - // NamedTempFile's boost::function constructor + // NamedTempFile's std::function constructor // takes a callable. To this callable it passes the // std::ostream with which it's writing the // NamedTempFile. - boost::bind(writeLLSDArray, _1, cdata)); + [cdata](std::ostream& out){ writeLLSDArray(out, cdata); }); python("read C++ notation", - placeholders::arg1 << + [this, pydata, &file](std::ostream& out) { + out << import_llsd << "def parse_each(iterable):\n" " for item in iterable:\n" " yield llsd.parse(item)\n" << pydata << // Don't forget raw-string syntax for Windows pathnames. - "verify(parse_each(open(r'" << file.getName() << "', 'rb')))\n"); + "verify(parse_each(open(r'" << file.getName() << "', 'rb')))\n"; }); } template<> template<> @@ -1869,7 +1855,8 @@ namespace tut NamedTempFile file("llsd", ""); python("write Python notation", - placeholders::arg1 << + [this, &file](std::ostream& out) { + out << import_llsd << "DATA = [\n" " 17,\n" @@ -1881,9 +1868,9 @@ namespace tut "]\n" // Don't forget raw-string syntax for Windows pathnames. // N.B. Using 'print' implicitly adds newlines. - "with open(r'" << file.getName() << "', 'w') as f:\n" + "with open(r'" << file.getName() << "', 'wb') as f:\n" " for item in DATA:\n" - " print(llsd.format_notation(item).decode(), file=f)\n"); + " print(llsd.format_notation(item), file=f)\n"; }); std::ifstream inf(file.getName().c_str()); LLSD item; -- cgit v1.2.3 From 3fbb1a496dd4aaadc11727832f3ab0cc8c64950f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Aug 2023 10:03:16 -0400 Subject: DRTVWR-588: Remove some unused redundant timer functionality. LLEventTimer supported static run_every(), run_at() and run_after() methods to schedule future work. This can still be done by deriving from LLEventTimer, but is better accomplished with a WorkSchedule instance. These convenience methods, which encourage use of LLEventTimer insted of WorkSchedule, weren't used except by LLEventTimeout. Remove them and the LLEventTimer::Generic subclass used to implement them. Similarly, LLEventTimeout supported static post_every(), post_at() and post_after() methods based on LLEventTimer::run_every(), run_at() and run_after(). These weren't used either. LLRunner is a very old mechanism to schedule future work that seems to be unused. Research suggests that it's indirectly engaged only by LLDeferredChain, which isn't used. LLIOSleeper is tested but isn't otherwise used. Add a deprecation warning to llrun.h prior to excision. Also replace Boost.Bind with lambdas. --- indra/llcommon/lleventfilter.cpp | 34 ++++---------------- indra/llcommon/lleventfilter.h | 13 -------- indra/llcommon/lleventtimer.h | 69 +--------------------------------------- indra/llcommon/llrun.h | 11 +++++++ 4 files changed, 18 insertions(+), 109 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/lleventfilter.cpp b/indra/llcommon/lleventfilter.cpp index 4cded7f88e..14c9c51830 100644 --- a/indra/llcommon/lleventfilter.cpp +++ b/indra/llcommon/lleventfilter.cpp @@ -33,20 +33,19 @@ // STL headers // std headers // external library headers -#include // other Linden headers +#include "lldate.h" #include "llerror.h" // LL_ERRS +#include "lleventtimer.h" #include "llsdutil.h" // llsd_matches() #include "stringize.h" -#include "lleventtimer.h" -#include "lldate.h" /***************************************************************************** * LLEventFilter *****************************************************************************/ LLEventFilter::LLEventFilter(LLEventPump& source, const std::string& name, bool tweak): LLEventStream(name, tweak), - mSource(source.listen(getName(), boost::bind(&LLEventFilter::post, this, _1))) + mSource(source.listen(getName(), [this](const LLSD& event){ return post(event); })) { } @@ -93,7 +92,7 @@ void LLEventTimeoutBase::actionAfter(F32 seconds, const Action& action) if (! mMainloop.connected()) { LLEventPump& mainloop(LLEventPumps::instance().obtain("mainloop")); - mMainloop = mainloop.listen(getName(), boost::bind(&LLEventTimeoutBase::tick, this, _1)); + mMainloop = mainloop.listen(getName(), [this](const LLSD& event){ return tick(event); }); } } @@ -185,27 +184,6 @@ bool LLEventTimeout::countdownElapsed() const return mTimer.hasExpired(); } -LLEventTimer* LLEventTimeout::post_every(F32 period, const std::string& pump, const LLSD& data) -{ - return LLEventTimer::run_every( - period, - [pump, data](){ LLEventPumps::instance().obtain(pump).post(data); }); -} - -LLEventTimer* LLEventTimeout::post_at(const LLDate& time, const std::string& pump, const LLSD& data) -{ - return LLEventTimer::run_at( - time, - [pump, data](){ LLEventPumps::instance().obtain(pump).post(data); }); -} - -LLEventTimer* LLEventTimeout::post_after(F32 interval, const std::string& pump, const LLSD& data) -{ - return LLEventTimer::run_after( - interval, - [pump, data](){ LLEventPumps::instance().obtain(pump).post(data); }); -} - /***************************************************************************** * LLEventBatch *****************************************************************************/ @@ -311,7 +289,7 @@ bool LLEventThrottleBase::post(const LLSD& event) // timeRemaining tells us how much longer it will be until // mInterval seconds since the last flush() call. At that time, // flush() deferred events. - alarmActionAfter(timeRemaining, boost::bind(&LLEventThrottleBase::flush, this)); + alarmActionAfter(timeRemaining, [this](){ flush(); }); } } return false; @@ -349,7 +327,7 @@ void LLEventThrottleBase::setInterval(F32 interval) // and if mAlarm is running, reset that too if (alarmRunning()) { - alarmActionAfter(timeRemaining, boost::bind(&LLEventThrottleBase::flush, this)); + alarmActionAfter(timeRemaining, [this](){ flush(); }); } } } diff --git a/indra/llcommon/lleventfilter.h b/indra/llcommon/lleventfilter.h index 7613850fb2..437a4826d4 100644 --- a/indra/llcommon/lleventfilter.h +++ b/indra/llcommon/lleventfilter.h @@ -214,19 +214,6 @@ public: LLEventTimeout(); LLEventTimeout(LLEventPump& source); - /// using LLEventTimeout as namespace for free functions - /// Post event to specified LLEventPump every period seconds. Delete - /// returned LLEventTimer* to cancel. - static LLEventTimer* post_every(F32 period, const std::string& pump, const LLSD& data); - /// Post event to specified LLEventPump at specified future time. Call - /// LLEventTimer::getInstance(returned pointer) to check whether it's still - /// pending; if so, delete the pointer to cancel. - static LLEventTimer* post_at(const LLDate& time, const std::string& pump, const LLSD& data); - /// Post event to specified LLEventPump after specified interval. Call - /// LLEventTimer::getInstance(returned pointer) to check whether it's still - /// pending; if so, delete the pointer to cancel. - static LLEventTimer* post_after(F32 interval, const std::string& pump, const LLSD& data); - protected: virtual void setCountdown(F32 seconds); virtual bool countdownElapsed() const; diff --git a/indra/llcommon/lleventtimer.h b/indra/llcommon/lleventtimer.h index dbbfe0c6e6..b5d40a0622 100644 --- a/indra/llcommon/lleventtimer.h +++ b/indra/llcommon/lleventtimer.h @@ -24,7 +24,7 @@ * $/LicenseInfo$ */ -#ifndef LL_EVENTTIMER_H +#ifndef LL_EVENTTIMER_H #define LL_EVENTTIMER_H #include "stdtypes.h" @@ -47,76 +47,9 @@ public: static void updateClass(); - /// Schedule recurring calls to generic callable every period seconds. - /// Returns a pointer; if you delete it, cancels the recurring calls. - template - static LLEventTimer* run_every(F32 period, const CALLABLE& callable); - - /// Schedule a future call to generic callable. Returns a pointer. - /// CAUTION: The object referenced by that pointer WILL BE DELETED once - /// the callback has been called! LLEventTimer::getInstance(pointer) (NOT - /// pointer->getInstance(pointer)!) can be used to test whether the - /// pointer is still valid. If it is, deleting it will cancel the - /// callback. - template - static LLEventTimer* run_at(const LLDate& time, const CALLABLE& callable); - - /// Like run_at(), but after a time delta rather than at a timestamp. - /// Same CAUTION. - template - static LLEventTimer* run_after(F32 interval, const CALLABLE& callable); - protected: LLTimer mEventTimer; F32 mPeriod; - -private: - template - class Generic; }; -template -class LLEventTimer::Generic: public LLEventTimer -{ -public: - // making TIME generic allows engaging either LLEventTimer constructor - template - Generic(const TIME& time, bool once, const CALLABLE& callable): - LLEventTimer(time), - mOnce(once), - mCallable(callable) - {} - BOOL tick() override - { - mCallable(); - // true tells updateClass() to delete this instance - return mOnce; - } - -private: - bool mOnce; - CALLABLE mCallable; -}; - -template -LLEventTimer* LLEventTimer::run_every(F32 period, const CALLABLE& callable) -{ - // return false to schedule recurring calls - return new Generic(period, false, callable); -} - -template -LLEventTimer* LLEventTimer::run_at(const LLDate& time, const CALLABLE& callable) -{ - // return true for one-shot callback - return new Generic(time, true, callable); -} - -template -LLEventTimer* LLEventTimer::run_after(F32 interval, const CALLABLE& callable) -{ - // one-shot callback after specified interval - return new Generic(interval, true, callable); -} - #endif //LL_EVENTTIMER_H diff --git a/indra/llcommon/llrun.h b/indra/llcommon/llrun.h index d610f86234..ebad5f3eaa 100644 --- a/indra/llcommon/llrun.h +++ b/indra/llcommon/llrun.h @@ -34,6 +34,17 @@ class LLRunnable; +////////////////////////////////////////////////////////////////////////////// +// DEPRECATION WARNING +// LLRunner is one of several mostly redundant ways to schedule future +// callbacks on the main thread. It seems to be unused in the current viewer. +// addRunner() is only called by LLPumpIO::sleepChain(). +// sleepChain() is only called by LLIOSleeper and LLIOSleep. +// LLIOSleeper is referenced only by tests. +// LLIOSleep is only called by LLDeferredChain. +// LLDeferredChain isn't referenced at all. +////////////////////////////////////////////////////////////////////////////// + /** * @class LLRunner * @brief This class manages a set of LLRunnable objects. -- cgit v1.2.3 From f24172d23d900bd6f9d10bb648107bbf4a755aaf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Aug 2023 10:34:40 -0400 Subject: DRTVWR-588: Correct typo in deprecation warning. --- indra/llcommon/llrun.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llrun.h b/indra/llcommon/llrun.h index ebad5f3eaa..27e4a43a8d 100644 --- a/indra/llcommon/llrun.h +++ b/indra/llcommon/llrun.h @@ -38,7 +38,7 @@ class LLRunnable; // DEPRECATION WARNING // LLRunner is one of several mostly redundant ways to schedule future // callbacks on the main thread. It seems to be unused in the current viewer. -// addRunner() is only called by LLPumpIO::sleepChain(). +// addRunnable() is only called by LLPumpIO::sleepChain(). // sleepChain() is only called by LLIOSleeper and LLIOSleep. // LLIOSleeper is referenced only by tests. // LLIOSleep is only called by LLDeferredChain. -- cgit v1.2.3 From 931a2fd63de2de417adbe3e02d55af7a459bca36 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Aug 2023 11:23:02 -0400 Subject: DRTVWR-588: print(file=) to binary file still requires str argument. Use f.writelines((bytes, b'\n')) instead. --- indra/llcommon/tests/llsdserialize_test.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index f46682e2d7..efd7dc9852 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -1867,10 +1867,9 @@ namespace tut "lines.''',\n" "]\n" // Don't forget raw-string syntax for Windows pathnames. - // N.B. Using 'print' implicitly adds newlines. "with open(r'" << file.getName() << "', 'wb') as f:\n" " for item in DATA:\n" - " print(llsd.format_notation(item), file=f)\n"; }); + " f.writelines((llsd.format_notation(item), b'\n'))\n"; }); std::ifstream inf(file.getName().c_str()); LLSD item; -- cgit v1.2.3 From 36ed0e98ea7bd591a798a4373e8aa78a8fca4d14 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Aug 2023 11:42:09 -0400 Subject: DRTVWR-588: Try harder to normalize Windows pathames to compare. --- indra/llcommon/tests/llprocess_test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index c48d913069..56dc2e4fa9 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -260,6 +260,7 @@ public: } std::string getName() const { return mPath.string(); } + std::string getNormalName() const { return mPath.lexically_normal().string(); } private: boost::filesystem::path mPath; @@ -591,7 +592,7 @@ namespace tut " f.write(os.path.normcase(os.path.normpath(os.getcwd())))\n"); // Before running, call setWorkingDirectory() py.mParams.cwd = tempdir.getName(); - std::string expected{ tempdir.getName() }; + std::string expected{ tempdir.getNormalName() }; #if LL_WINDOWS // SIGH, don't get tripped up by "C:" != "c:" -- // but on the Mac, using tolower() fails because "/users" != "/Users"! -- cgit v1.2.3 From b5fe9c476943807aa7526f67dd648b5ad250824b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Aug 2023 11:45:34 -0400 Subject: DRTVWR-588: To write b'\n' in Python source, use "b'\\n'" --- indra/llcommon/tests/llsdserialize_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index efd7dc9852..bb469f0686 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -1869,7 +1869,7 @@ namespace tut // Don't forget raw-string syntax for Windows pathnames. "with open(r'" << file.getName() << "', 'wb') as f:\n" " for item in DATA:\n" - " f.writelines((llsd.format_notation(item), b'\n'))\n"; }); + " f.writelines((llsd.format_notation(item), b'\\n'))\n"; }); std::ifstream inf(file.getName().c_str()); LLSD item; -- cgit v1.2.3 From dc8f2ae2ba1a1348f86f412df7f769e6cc2fe541 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Aug 2023 17:01:56 -0400 Subject: DRTVWR-588: Try even harder to normalize Windows pathnames (SIGHH) --- indra/llcommon/tests/llprocess_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 56dc2e4fa9..ece567f40e 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -260,7 +260,7 @@ public: } std::string getName() const { return mPath.string(); } - std::string getNormalName() const { return mPath.lexically_normal().string(); } + std::string getNormalName() const { return mPath.lexically_normal().make_preferred().string(); } private: boost::filesystem::path mPath; -- cgit v1.2.3 From 5ed44adec58ba2ecb8dd073122bd8882fed30638 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 12 Sep 2023 09:29:26 -0400 Subject: DRTVWR-588: Fix a couple merge glitches in llsdserialize_test.cpp. --- indra/llcommon/tests/llsdserialize_test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index c87deb8ffe..341b0d5609 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -1948,7 +1948,7 @@ namespace tut { writeLLSDArray(serialize, out, cdata); }); python("read C++ " + desc, - [this, pydata, &file](std::ostream& out) { + [pydata, &file](std::ostream& out) { out << import_llsd << "from functools import partial\n" @@ -2062,7 +2062,7 @@ namespace tut NamedTempFile file("llsd", ""); python("Python " + pyformatter, - [this, &file](std::ostream& out) { + [pyformatter, &file](std::ostream& out) { out << import_llsd << "import struct\n" @@ -2081,7 +2081,7 @@ namespace tut " for item in DATA:\n" " serialized = llsd." << pyformatter << "(item)\n" " f.write(lenformat.pack(len(serialized)))\n" - " f.write(serialized)\n"); + " f.write(serialized)\n";}); std::ifstream inf(file.getName().c_str()); LLSD item; -- cgit v1.2.3 From 796085fc537e6bac7999e4b6624f02196eeaf4ad Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 12 Sep 2023 09:32:32 -0400 Subject: DRTVWR-588: Reimplement LLMainThreadTask based on WorkQueue instead of on LLEventTimer. LLEventTimer takes cycles from the main loop to run through the collection of pending LLEventTimers, checking each to see if we've reached its timestamp. But LLMainThreadTask does not require delay timing; it wants the main loop to service it ASAP. That's what the "mainloop" WorkQueue is for. But WorkQueue::waitForResult() forbids calls from a thread's default coroutine. While that restriction may still make sense in general, we specifically want to be able to pause LLMainThreadTask's caller, no matter what coroutine it's running on. Introduce WorkQueue::waitForResult_() that bypasses the check. --- indra/llcommon/llmainthreadtask.h | 58 ++++++++++----------------------------- indra/llcommon/workqueue.h | 40 +++++++++++++++++++-------- 2 files changed, 43 insertions(+), 55 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llmainthreadtask.h b/indra/llcommon/llmainthreadtask.h index d509b687c0..dde6c20210 100644 --- a/indra/llcommon/llmainthreadtask.h +++ b/indra/llcommon/llmainthreadtask.h @@ -13,11 +13,8 @@ #if ! defined(LL_LLMAINTHREADTASK_H) #define LL_LLMAINTHREADTASK_H -#include "lleventtimer.h" #include "llthread.h" -#include "llmake.h" -#include -#include // std::result_of +#include "workqueue.h" /** * LLMainThreadTask provides a way to perform some task specifically on the @@ -28,18 +25,17 @@ * Instead of instantiating LLMainThreadTask, pass your invocable to its * static dispatch() method. dispatch() returns the result of calling your * task. (Or, if your task throws an exception, dispatch() throws that - * exception. See std::packaged_task.) + * exception.) * * When you call dispatch() on the main thread (as determined by * on_main_thread() in llthread.h), it simply calls your task and returns the * result. * - * When you call dispatch() on a secondary thread, it instantiates an - * LLEventTimer subclass scheduled immediately. Next time the main loop calls - * LLEventTimer::updateClass(), your task will be run, and LLMainThreadTask - * will fulfill a future with its result. Meanwhile the requesting thread - * blocks on that future. As soon as it is set, the requesting thread wakes up - * with the task result. + * When you call dispatch() on a secondary thread, it posts your task to + * gMainloopWork, the WorkQueue serviced by the main thread, using + * WorkQueue::waitForResult() to block the caller. Next time the main loop + * calls gMainloopWork.runFor(), your task will be run, and waitForResult() + * will return its result. */ class LLMainThreadTask { @@ -59,41 +55,15 @@ public: } else { - // It's essential to construct LLEventTimer subclass instances on - // the heap because, on completion, LLEventTimer deletes them. - // Once we enable C++17, we can use Class Template Argument - // Deduction. Until then, use llmake_heap(). - auto* task = llmake_heap(std::forward(callable)); - auto future = task->mTask.get_future(); - // Now simply block on the future. - return future.get(); + auto queue{ LL::WorkQueue::getInstance("mainloop") }; + // If this needs a null check and a message, please introduce a + // method in the .cpp file so consumers of this header don't drag + // in llerror.h. + // Use waitForResult_() so dispatch() can be used even from the + // calling thread's default coroutine. + return queue->waitForResult_(std::forward(callable)); } } - -private: - template - struct Task: public LLEventTimer - { - Task(CALLABLE&& callable): - // no wait time: call tick() next chance we get - LLEventTimer(0), - mTask(std::forward(callable)) - {} - BOOL tick() override - { - // run the task on the main thread, will populate the future - // obtained by get_future() - mTask(); - // tell LLEventTimer we're done (one shot) - return TRUE; - } - // Given arbitrary CALLABLE, which might be a lambda, how are we - // supposed to obtain its signature for std::packaged_task? It seems - // redundant to have to add an argument list to engage result_of, then - // add the argument list again to complete the signature. At least we - // only support a nullary CALLABLE. - std::packaged_task::type()> mTask; - }; }; #endif /* ! defined(LL_LLMAINTHREADTASK_H) */ diff --git a/indra/llcommon/workqueue.h b/indra/llcommon/workqueue.h index 70fd65bd0c..20fd15d0d5 100644 --- a/indra/llcommon/workqueue.h +++ b/indra/llcommon/workqueue.h @@ -12,7 +12,6 @@ #if ! defined(LL_WORKQUEUE_H) #define LL_WORKQUEUE_H -#include "llcoros.h" #include "llexception.h" #include "llinstancetracker.h" #include "threadsafeschedule.h" @@ -200,31 +199,51 @@ namespace LL } /** - * Post work to another WorkQueue to be run at a specified time, - * blocking the calling coroutine until then, returning the result to - * caller on completion. + * Post work, blocking the calling coroutine until then, returning the + * result to caller on completion. * * In general, we assume that each thread's default coroutine is busy * servicing its WorkQueue or whatever. To try to prevent mistakes, we * forbid calling waitForResult() from a thread's default coroutine. */ template - auto waitForResult(const TimePoint& time, CALLABLE&& callable); + auto waitForResult(CALLABLE&& callable) + { + return waitForResult(TimePoint::clock::now(), std::forward(callable)); + } /** - * Post work to another WorkQueue, blocking the calling coroutine - * until then, returning the result to caller on completion. + * Post work to be run at a specified time, blocking the calling + * coroutine until then, returning the result to caller on completion. * * In general, we assume that each thread's default coroutine is busy * servicing its WorkQueue or whatever. To try to prevent mistakes, we * forbid calling waitForResult() from a thread's default coroutine. */ template - auto waitForResult(CALLABLE&& callable) + auto waitForResult(const TimePoint& time, CALLABLE&& callable) { - return waitForResult(TimePoint::clock::now(), std::move(callable)); + checkCoroutine("waitForResult()"); + return waitForResult_(time, std::forward(callable)); } + /** + * Post work, blocking the calling coroutine until then, returning the + * result to caller on completion. + */ + template + auto waitForResult_(CALLABLE&& callable) + { + return waitForResult_(TimePoint::clock::now(), std::forward(callable)); + } + + /** + * Post work to be run at a specified time, blocking the calling + * coroutine until then, returning the result to caller on completion. + */ + template + auto waitForResult_(const TimePoint& time, CALLABLE&& callable); + /*--------------------------- worker API ---------------------------*/ /** @@ -561,9 +580,8 @@ namespace LL }; template - auto WorkQueue::waitForResult(const TimePoint& time, CALLABLE&& callable) + auto WorkQueue::waitForResult_(const TimePoint& time, CALLABLE&& callable) { - checkCoroutine("waitForResult()"); // derive callable's return type so we can specialize for void return WaitForResult(callable)())>() (this, time, std::forward(callable)); -- cgit v1.2.3 From 24d405048fa0b9b26d1cb1d9e8ea8b113b867a14 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 12 Sep 2023 10:06:03 -0400 Subject: DRTVWR-588: Move LLSingleton dependency on LLMainThreadTask to .cpp. Introduce LLSingletonBase::getInstanceForSecondaryThread(), used both by LLSingleton and LLParamSingleton. Because it's a method of the non-template base class, because it's not itself a template method, getInstanceForSecondaryThread()'s definition can live in llsingleton.cpp. This is what calls LLMainThreadTask::dispatch(). To support LLParamSingleton, though, getInstanceForSecondaryThread() must be capable of handling arguments. For that, it accepts a nullary std::function returning the LLSingletonBase* of interest. Packing initParamSingleton() arguments into a nullary std::function to pass to getInstanceForSecondaryThread() sounds like a job for std::bind(). Unfortunately std::bind() has trouble forwarding int and string literals to a function that infers its argument types. To work around that, use boost::call_traits::param_type and a lambda with an explicit tuple. --- indra/llcommon/llsingleton.cpp | 31 ++++++++++++++++-- indra/llcommon/llsingleton.h | 72 ++++++++++++++++++++++++------------------ 2 files changed, 70 insertions(+), 33 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 6b1986d0e9..5c99048123 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -27,11 +27,12 @@ #include "linden_common.h" #include "llsingleton.h" +#include "llcoros.h" +#include "lldependencies.h" #include "llerror.h" #include "llerrorcontrol.h" -#include "lldependencies.h" #include "llexception.h" -#include "llcoros.h" +#include "llmainthreadtask.h" #include #include #include // std::cerr in dire emergency @@ -486,3 +487,29 @@ std::string LLSingletonBase::demangle(const char* mangled) { return LLError::Log::demangle(mangled); } + +LLSingletonBase* LLSingletonBase::getInstanceForSecondaryThread( + const std::string& name, + const std::string& method, + const std::function& getInstance) +{ + // Normally it would be the height of folly to reference-bind args into a + // lambda to be executed on some other thread! By the time that thread + // executed the lambda, the references would all be dangling, and Bad + // Things would result. But LLMainThreadTask::dispatch() promises to block + // the calling thread until the passed task has completed. So in this case + // we know the references will remain valid until the lambda has run, so + // we dare to bind references. + return LLMainThreadTask::dispatch( + [&name, &method, &getInstance](){ + // VERY IMPORTANT to call getInstance() on the main thread, + // rather than going straight to constructSingleton()! + // During the time window before mInitState is INITIALIZED, + // multiple requests might be queued. It's essential that, as + // the main thread processes them, only the FIRST such request + // actually constructs the instance -- every subsequent one + // simply returns the existing instance. + loginfos({name, "::", method, " on main thread"}); + return getInstance(); + }); +} diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 51ef514cf7..caecc3594f 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -25,16 +25,18 @@ #ifndef LLSINGLETON_H #define LLSINGLETON_H +#include #include #include +#include #include #include #include #include #include "mutex.h" #include "lockstatic.h" +#include "apply.h" #include "llthread.h" // on_main_thread() -#include "llmainthreadtask.h" class LLSingletonBase: private boost::noncopyable { @@ -134,6 +136,17 @@ protected: // internal wrapper around calls to cleanupSingleton() void cleanup_(); + // This method is where we dispatch to LLMainThreadTask to acquire the + // subclass LLSingleton instance when the first getInstance() call is from + // a secondary thread. We delegate to the .cpp file to untangle header + // circularity. It accepts a std::function referencing the subclass + // getInstance() method -- which can't be virtual because it's static; we + // don't yet have an instance! For messaging, it also accepts the name of + // the subclass and the subclass method. + static LLSingletonBase* getInstanceForSecondaryThread( + const std::string& name, const std::string& method, + const std::function& getInstance); + // deleteSingleton() isn't -- and shouldn't be -- a virtual method. It's a // class static. However, given only Foo*, deleteAll() does need to be // able to reach Foo::deleteSingleton(). Make LLSingleton (which declares @@ -555,19 +568,11 @@ public: // Per the comment block above, dispatch to the main thread. loginfos({classname(), "::getInstance() dispatching to main thread"}); - auto instance = LLMainThreadTask::dispatch( - [](){ - // VERY IMPORTANT to call getInstance() on the main thread, - // rather than going straight to constructSingleton()! - // During the time window before mInitState is INITIALIZED, - // multiple requests might be queued. It's essential that, as - // the main thread processes them, only the FIRST such request - // actually constructs the instance -- every subsequent one - // simply returns the existing instance. - loginfos({classname(), - "::getInstance() on main thread"}); - return getInstance(); - }); + auto instance = static_cast( + getInstanceForSecondaryThread( + classname(), + "getInstance()", + getInstance)); // record the dependency chain tracked on THIS thread, not the main // thread (consider a getInstance() overload with a tag param that // suppresses dep tracking when dispatched to the main thread) @@ -632,8 +637,14 @@ private: // Passes arguments to DERIVED_TYPE's constructor and sets appropriate // states, returning a pointer to the new instance. + // If we just let initParamSingleton_() infer its argument types, the + // compiler has trouble passing int and string literals. Use + // boost::call_traits::param_type to smooth parameter passing. This + // construction requires, though, that each invocation of this method + // explicitly specify template arguments, instead of inferring them. template - static DERIVED_TYPE* initParamSingleton_(Args&&... args) + static LLSingletonBase* initParamSingleton_( + typename boost::call_traits::param_type... args) { // In case racing threads both call initParamSingleton() at the same // time, serialize them. One should initialize; the other should see @@ -652,7 +663,7 @@ private: // on the main thread, simply construct instance while holding lock super::logdebugs({super::template classname(), "::initParamSingleton()"}); - super::constructSingleton(lk, std::forward(args)...); + super::constructSingleton(lk, args...); return lk->mInstance; } else @@ -665,20 +676,19 @@ private: lk.unlock(); super::loginfos({super::template classname(), "::initParamSingleton() dispatching to main thread"}); - // Normally it would be the height of folly to reference-bind - // 'args' into a lambda to be executed on some other thread! By - // the time that thread executed the lambda, the references would - // all be dangling, and Bad Things would result. But - // LLMainThreadTask::dispatch() promises to block until the passed - // task has completed. So in this case we know the references will - // remain valid until the lambda has run, so we dare to bind - // references. - auto instance = LLMainThreadTask::dispatch( - [&](){ - super::loginfos({super::template classname(), - "::initParamSingleton() on main thread"}); - return initParamSingleton_(std::forward(args)...); - }); + auto instance = static_cast( + super::getInstanceForSecondaryThread( + super::template classname(), + "initParamSingleton()", + // This lambda does what std::bind() is supposed to do -- + // but when the actual parameter is (e.g.) a string + // literal, type inference makes it fail. Apply param_type + // to each incoming type to make it work. + [args=std::tuple::param_type...>(args...)] + () + { + return LL::apply(initParamSingleton_, args); + })); super::loginfos({super::template classname(), "::initParamSingleton() returning on requesting thread"}); return instance; @@ -695,7 +705,7 @@ public: template static DERIVED_TYPE& initParamSingleton(Args&&... args) { - return *initParamSingleton_(std::forward(args)...); + return *static_cast(initParamSingleton_(args...)); } static DERIVED_TYPE* getInstance() -- cgit v1.2.3 From 825c67612ce5ee6544055d82b337911050e86e75 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 25 Apr 2024 14:00:06 -0400 Subject: Add missing #include "stringize.h" Also change from boost::hof::is_invocable() to std::is_invocable(). --- indra/llcommon/lleventdispatcher.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index a82bc7a69b..3d7808ac31 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -35,7 +35,6 @@ #include #include #include -#include // until C++17, when we get std::is_invocable #include #include // std::function #include // std::unique_ptr @@ -48,6 +47,7 @@ #include "llevents.h" #include "llptrto.h" #include "llsdutil.h" +#include "stringize.h" class LLSD; @@ -99,7 +99,7 @@ public: template ::value + std::is_invocable::value >::type> void add(const std::string& name, const std::string& desc, @@ -296,7 +296,7 @@ public: */ template () + ! std::is_invocable() >::type> void add(const std::string& name, const std::string& desc, @@ -338,7 +338,7 @@ public: template::value && - ! boost::hof::is_invocable::value + ! std::is_invocable::value >::type> void add(const std::string& name, const std::string& desc, Function f, const LLSD& params, const LLSD& defaults=LLSD()); -- cgit v1.2.3 From 0bcbe6f850580306d3f665e459873adf736c37a2 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 25 Apr 2024 14:01:50 -0400 Subject: Resolve WorkQueue::waitForResult() merge glitch. --- indra/llcommon/workqueue.h | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/workqueue.h b/indra/llcommon/workqueue.h index 03c0494c1f..c4435cec40 100644 --- a/indra/llcommon/workqueue.h +++ b/indra/llcommon/workqueue.h @@ -115,33 +115,29 @@ namespace LL ARGS&&... args); /** - * Post work to be run at a specified time, blocking the calling - * coroutine until then, returning the result to caller on completion. - * Optional final argument is TimePoint for WorkSchedule. + * Post work, blocking the calling coroutine, returning the result to + * caller on completion. Optional final argument is TimePoint for + * WorkSchedule. * * In general, we assume that each thread's default coroutine is busy * servicing its WorkQueue or whatever. To try to prevent mistakes, we * forbid calling waitForResult() from a thread's default coroutine. */ template - auto waitForResult(CALLABLE&& callable, ARGS&&... args); - - /** - * Post work, blocking the calling coroutine until then, returning the - * result to caller on completion. - */ - template - auto waitForResult_(CALLABLE&& callable) + auto waitForResult(CALLABLE&& callable, ARGS&&... args) { - return waitForResult_(TimePoint::clock::now(), std::forward(callable)); + checkCoroutine("waitForResult()"); + return waitForResult_(std::forward(callable), + std::forward(args)...); } /** - * Post work to be run at a specified time, blocking the calling - * coroutine until then, returning the result to caller on completion. + * Post work, blocking the calling coroutine, returning the result to + * caller on completion. Optional final argument is TimePoint for + * WorkSchedule. */ - template - auto waitForResult_(const TimePoint& time, CALLABLE&& callable); + template + auto waitForResult_(CALLABLE&& callable, ARGS&&... args); /*--------------------------- worker API ---------------------------*/ @@ -634,7 +630,7 @@ namespace LL }; template - auto WorkQueueBase::waitForResult(CALLABLE&& callable, ARGS&&... args) + auto WorkQueueBase::waitForResult_(CALLABLE&& callable, ARGS&&... args) { // derive callable's return type so we can specialize for void return WaitForResult(callable)())>() -- cgit v1.2.3 From c231c97eeefc484b74198ba86251054b7dc0e6bb Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 2 May 2024 21:13:28 -0400 Subject: WIP: In llcallbacklist.h, add singleton LLLater for time delays. The big idea is to reduce the number of per-tick callbacks asking, "Is it time yet? Is it time yet?" We do that for LLEventTimer and LLEventTimeout. LLLater presents doAtTime(LLDate), with doAfterInterval() and doPeriodically() methods implemented using doAtTime(). All return handles. The free functions doAfterInterval() and doPeriodically() now forward to the corresponding LLLater methods. LLLater also presents isRunning(handle) and cancel(handle). LLLater borrows the tactic of LLEventTimer: while there's at least one running timer, it registers an LLCallbackList tick() callback to service ready timers. But instead of looping over all of them asking, "Are you ready?" it keeps them in a priority queue ordered by desired timestamp, and only touches those whose timestamp has been reached. Also, it honors a maximum time slice: once the ready timers have run for longer than the limit, it defers processing other ready timers to the next tick() call. The intent is to consume fewer cycles per tick() call, both by the management machinery and the timers themselves. Revamp LLCallbackList to accept C++ callables in addition to (classic C function pointer, void*) pairs. Make addFunction() return a handle (different than LLLater handles) that can be passed to a new deleteFunction() overload, since std::function instances can't be compared for equality. In fact, implement LLCallbackList using boost::signals2::signal, which provides almost exactly what we want. LLCallbackList continues to accept (function pointer, void*) pairs, but now we store a lambda that calls the function pointer with that void*. It takes less horsing around to create a C++ callable from a (function pointer, void*) pair than the other way around. For containsFunction() and deleteFunction(), such pairs are the keys for a lookup table whose values are handles. Instead of having a static global LLCallbackList gIdleCallbacks, make LLCallbackList an LLSingleton to guarantee initialization. For backwards compatibility, gIdleCallbacks is now a macro for LLCallbackList::instance(). Move doOnIdleOneTime() and doOnIdleRepeating() functions to LLCallbackList methods, but for backwards compatibility continue providing free functions. Reimplement LLEventTimer using LLLater::doPeriodically(). One implication is that LLEventTimer need no longer be derived from LLInstanceTracker, which we used to iterate over all instances every tick. Give it start() and stop() methods, since some subclasses (e.g. LLFlashTimer) used to call its member LLTimer's start() and stop(). Remove updateClass(): LLCallbackList::callFunctions() now takes care of that. Remove LLToastLifeTimer::start() and stop(), since LLEventTimer now provides those. Remove getRemainingTimeF32(), since LLLater does not (yet) provide that feature. While at it, make LLEventTimer::tick() return bool instead of BOOL, and change existing overrides. Make LLApp::stepFrame() call LLCallbackList::callFunctions() instead of LLEventTimer::updateClass(). We could have refactored LLEventTimer to use the mechanism now built into LLLater, but frankly the LLEventTimer API is rather clumsy. You MUST derive a subclass and override tick(), and you must instantiate your subclass on the heap because, when your tick() override returns false, LLEventTimer deletes its subclass instance. The LLLater API is simpler to use, and LLEventTimer is much simplified by using it. Merge lleventfilter.h's LLEventTimeoutBase into LLEventTimeout, and likewise merge LLEventThrottleBase into LLEventThrottle. The separation was for testability, but now that they're no longer based on LLTimer, it becomes harder to use dummy time for testing. Temporarily skip tests based on LLEventTimeoutBase and LLEventThrottleBase. Instead of listening for LLEventPump("mainloop") ticks and using LLTimer, LLEventTimeout now uses LLLater::doAfterInterval(). Instead of LLTimer and LLEventTimeout, LLEventThrottle likewise now uses LLLater::doAfterInterval(). Recast a couple local LLEventTimeout pre-lambda callable classes with lambdas. Dignify F64 with a new typedef LLDate::timestamp. LLDate heavily depends on that as its base time representation, but there are those who question use of floating-point for time. This is a step towards insulating us from any future change. --- indra/llcommon/llapp.cpp | 2 +- indra/llcommon/llcallbacklist.cpp | 324 +++++++++++++++---------- indra/llcommon/llcallbacklist.h | 228 +++++++++++++++-- indra/llcommon/lldate.cpp | 12 +- indra/llcommon/lldate.h | 8 +- indra/llcommon/llerrorlegacy.h | 32 --- indra/llcommon/lleventfilter.cpp | 138 +++-------- indra/llcommon/lleventfilter.h | 129 +++------- indra/llcommon/lleventtimer.cpp | 39 +-- indra/llcommon/lleventtimer.h | 18 +- indra/llcommon/lllivefile.cpp | 2 +- indra/llcommon/tests/lleventfilter_test.cpp | 14 ++ indra/llcommon/tests/llmainthreadtask_test.cpp | 4 +- 13 files changed, 521 insertions(+), 429 deletions(-) delete mode 100644 indra/llcommon/llerrorlegacy.h (limited to 'indra/llcommon') diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index 90d0c28eb1..5722f10f62 100644 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -322,7 +322,7 @@ void LLApp::stepFrame() { LLFrameTimer::updateFrameTime(); LLFrameTimer::updateFrameCount(); - LLEventTimer::updateClass(); + LLCallbackList::instance().callFunctions(); mRunner.run(); } diff --git a/indra/llcommon/llcallbacklist.cpp b/indra/llcommon/llcallbacklist.cpp index 9f23ce5317..7f7fdc7370 100644 --- a/indra/llcommon/llcallbacklist.cpp +++ b/indra/llcommon/llcallbacklist.cpp @@ -25,17 +25,14 @@ */ #include "llcallbacklist.h" -#include "lleventtimer.h" -#include "llerrorlegacy.h" - -// Globals -// -LLCallbackList gIdleCallbacks; // // Member functions // +/***************************************************************************** +* LLCallbackList +*****************************************************************************/ LLCallbackList::LLCallbackList() { // nothing @@ -45,186 +42,251 @@ LLCallbackList::~LLCallbackList() { } - -void LLCallbackList::addFunction( callback_t func, void *data) +LLCallbackList::handle_t LLCallbackList::addFunction( callback_t func, void *data) { if (!func) { - return; + return {}; } // only add one callback per func/data pair // if (containsFunction(func, data)) { - return; + return {}; } - - callback_pair_t t(func, data); - mCallbackList.push_back(t); + + auto handle = addFunction([func, data]{ func(data); }); + mLookup.emplace(callback_pair_t(func, data), handle); + return handle; } -bool LLCallbackList::containsFunction( callback_t func, void *data) +LLCallbackList::handle_t LLCallbackList::addFunction( const callable_t& func ) { - callback_pair_t t(func, data); - callback_list_t::iterator iter = find(func,data); - if (iter != mCallbackList.end()) - { - return TRUE; - } - else - { - return FALSE; - } + return mCallbackList.connect(func); } +bool LLCallbackList::containsFunction( callback_t func, void *data) +{ + return mLookup.find(callback_pair_t(func, data)) != mLookup.end(); +} bool LLCallbackList::deleteFunction( callback_t func, void *data) { - callback_list_t::iterator iter = find(func,data); - if (iter != mCallbackList.end()) + auto found = mLookup.find(callback_pair_t(func, data)); + if (found != mLookup.end()) { - mCallbackList.erase(iter); - return TRUE; + mLookup.erase(found); + deleteFunction(found->second); + return true; } else { - return FALSE; + return false; } } -inline -LLCallbackList::callback_list_t::iterator -LLCallbackList::find(callback_t func, void *data) +void LLCallbackList::deleteFunction( const handle_t& handle ) { - callback_pair_t t(func, data); - return std::find(mCallbackList.begin(), mCallbackList.end(), t); + handle.disconnect(); } void LLCallbackList::deleteAllFunctions() { - mCallbackList.clear(); + mCallbackList = {}; + mLookup.clear(); } - void LLCallbackList::callFunctions() { - for (callback_list_t::iterator iter = mCallbackList.begin(); iter != mCallbackList.end(); ) - { - callback_list_t::iterator curiter = iter++; - curiter->first(curiter->second); - } + mCallbackList(); } -// Shim class to allow arbitrary boost::bind -// expressions to be run as one-time idle callbacks. -class OnIdleCallbackOneTime +LLCallbackList::handle_t LLCallbackList::doOnIdleOneTime( const callable_t& func ) { -public: - OnIdleCallbackOneTime(nullary_func_t callable): - mCallable(callable) - { - } - static void onIdle(void *data) - { - gIdleCallbacks.deleteFunction(onIdle, data); - OnIdleCallbackOneTime* self = reinterpret_cast(data); - self->call(); - delete self; - } - void call() - { - mCallable(); - } -private: - nullary_func_t mCallable; -}; + // connect_extended() passes the connection to the callback + return mCallbackList.connect_extended( + [func](const handle_t& handle) + { + handle.disconnect(); + func(); + }); +} -void doOnIdleOneTime(nullary_func_t callable) +LLCallbackList::handle_t LLCallbackList::doOnIdleRepeating( const bool_func_t& func ) { - OnIdleCallbackOneTime* cb_functor = new OnIdleCallbackOneTime(callable); - gIdleCallbacks.addFunction(&OnIdleCallbackOneTime::onIdle,cb_functor); + return mCallbackList.connect_extended( + [func](const handle_t& handle) + { + if (func()) + { + handle.disconnect(); + } + }); } -// Shim class to allow generic boost functions to be run as -// recurring idle callbacks. Callable should return true when done, -// false to continue getting called. -class OnIdleCallbackRepeating -{ -public: - OnIdleCallbackRepeating(bool_func_t callable): - mCallable(callable) - { - } - // Will keep getting called until the callable returns true. - static void onIdle(void *data) - { - OnIdleCallbackRepeating* self = reinterpret_cast(data); - bool done = self->call(); - if (done) - { - gIdleCallbacks.deleteFunction(onIdle, data); - delete self; - } - } - bool call() - { - return mCallable(); - } -private: - bool_func_t mCallable; -}; +/***************************************************************************** +* LLLater +*****************************************************************************/ +LLLater::LLLater() {} -void doOnIdleRepeating(bool_func_t callable) +// Call a given callable once at specified timestamp. +LLLater::handle_t LLLater::doAtTime(nullary_func_t callable, LLDate::timestamp time) { - OnIdleCallbackRepeating* cb_functor = new OnIdleCallbackRepeating(callable); - gIdleCallbacks.addFunction(&OnIdleCallbackRepeating::onIdle,cb_functor); + bool first{ mQueue.empty() }; + // Pick token FIRST to store a self-reference in mQueue's managed node as + // well as in mHandles. Pre-increment to distinguish 0 from any live + // handle_t. + token_t token{ ++mToken }; + auto handle{ mQueue.emplace(callable, time, token) }; + mHandles.emplace(token, handle); + if (first) + { + // If this is our first entry, register for regular callbacks. + mLive = LLCallbackList::instance().doOnIdleRepeating([this]{ return tick(); }); + } + return handle_t{ token }; } -class NullaryFuncEventTimer: public LLEventTimer +// Call a given callable once after specified interval. +LLLater::handle_t LLLater::doAfterInterval(nullary_func_t callable, F32 seconds) { -public: - NullaryFuncEventTimer(nullary_func_t callable, F32 seconds): - LLEventTimer(seconds), - mCallable(callable) - { - } + // Passing 0 is a slightly more expensive way of calling + // LLCallbackList::doOnIdleOneTime(). Are we sure the caller is correct? + // (If there's a valid use case, remove the llassert() and carry on.) + llassert(seconds > 0); + return doAtTime(callable, LLDate::now().secondsSinceEpoch() + seconds); +} -private: - BOOL tick() - { - mCallable(); - return TRUE; - } +// For doPeriodically(), we need a struct rather than a lambda because a +// struct, unlike a lambda, has access to 'this'. +struct Periodic +{ + LLLater* mLater; + bool_func_t mCallable; + LLDate::timestamp mNext; + F32 mSeconds; - nullary_func_t mCallable; + void operator()() + { + if (! mCallable()) + { + // Returning false means please schedule another call. + // Don't call doAfterInterval(), which rereads LLDate::now(), + // since that would defer by however long it took us to wake + // up and notice plus however long callable() took to run. + mNext += mSeconds; + mLater->doAtTime(*this, mNext); + } + } }; -// Call a given callable once after specified interval. -void doAfterInterval(nullary_func_t callable, F32 seconds) +// Call a given callable every specified number of seconds, until it returns true. +LLLater::handle_t LLLater::doPeriodically(bool_func_t callable, F32 seconds) { - new NullaryFuncEventTimer(callable, seconds); + // Passing seconds <= 0 will produce an infinite loop. + llassert(seconds > 0); + auto next{ LLDate::now().secondsSinceEpoch() + seconds }; + return doAtTime(Periodic{ this, callable, next, seconds }, next); } -class BoolFuncEventTimer: public LLEventTimer +bool LLLater::isRunning(handle_t timer) { -public: - BoolFuncEventTimer(bool_func_t callable, F32 seconds): - LLEventTimer(seconds), - mCallable(callable) - { - } -private: - BOOL tick() - { - return mCallable(); - } + // A default-constructed timer isn't running. + // A timer we don't find in mHandles has fired or been canceled. + return timer && mHandles.find(timer.token) != mHandles.end(); +} - bool_func_t mCallable; -}; +// Cancel a future timer set by doAtTime(), doAfterInterval(), doPeriodically() +bool LLLater::cancel(handle_t& timer) +{ + // For exception safety, capture and clear timer before canceling. + // Once we've canceled this handle, don't retain the live handle. + const handle_t ctimer{ timer }; + timer = handle_t(); + return cancel(ctimer); +} -// Call a given callable every specified number of seconds, until it returns true. -void doPeriodically(bool_func_t callable, F32 seconds) +bool LLLater::cancel(const handle_t& timer) +{ + if (! timer) + { + return false; + } + + // fibonacci_heap documentation does not address the question of what + // happens if you call erase() twice with the same handle. Is it a no-op? + // Does it invalidate the heap? Is it UB? + + // Nor do we find any documented way to ask whether a given handle still + // tracks a valid heap node. That's why we capture all returned handles in + // mHandles and validate against that collection. What about the pop() + // call in tick()? How to map from the top() value back to the + // corresponding handle_t? That's why we store func_at::mToken. + + // fibonacci_heap provides a pair of begin()/end() methods to iterate over + // all nodes (NOT in heap order), plus a function to convert from such + // iterators to handles. Without mHandles, that would be our only chance + // to validate. + auto found{ mHandles.find(timer.token) }; + if (found == mHandles.end()) + { + // we don't recognize this handle -- maybe the timer has already + // fired, maybe it was previously canceled. + return false; + } + + // erase from mQueue the handle_t referenced by timer.token + mQueue.erase(found->second); + // before erasing timer.token from mHandles + mHandles.erase(found); + if (mQueue.empty()) + { + // If that was the last active timer, unregister for callbacks. + //LLCallbackList::instance().deleteFunction(mLive); + // Since we're in the source file that knows the true identity of an + // LLCallbackList::handle_t, we don't even need to call instance(). + mLive.disconnect(); + } + return true; +} + +bool LLLater::tick() { - new BoolFuncEventTimer(callable, seconds); + // Fetch current time only on entry, even though running some mQueue task + // may take long enough that the next one after would become ready. We're + // sharing this thread with everything else, and there's a risk we might + // starve it if we have a sequence of tasks that take nontrivial time. + auto now{ LLDate::now().secondsSinceEpoch() }; + auto cutoff{ now + TIMESLICE }; + while (! mQueue.empty()) + { + auto& top{ mQueue.top() }; + if (top.mTime > now) + { + // we've hit an entry that's still in the future: + // done with this tick(), but schedule another call + return false; + } + if (LLDate::now().secondsSinceEpoch() > cutoff) + { + // we still have ready tasks, but we've already eaten too much + // time this tick() -- defer until next tick() -- call again + return false; + } + + // Found a ready task. Hate to copy stuff, but -- what if the task + // indirectly ends up trying to cancel a handle referencing its own + // node in mQueue? If the task has any state, that would be Bad. Copy + // the node before running it. + auto current{ top }; + // remove the mHandles entry referencing this task + mHandles.erase(current.mToken); + // before removing the mQueue task entry itself + mQueue.pop(); + // okay, NOW run + current.mFunc(); + } + // queue is empty: stop callbacks + return true; } diff --git a/indra/llcommon/llcallbacklist.h b/indra/llcommon/llcallbacklist.h index 89716cd74c..522a9b838b 100644 --- a/indra/llcommon/llcallbacklist.h +++ b/indra/llcommon/llcallbacklist.h @@ -27,53 +27,237 @@ #ifndef LL_LLCALLBACKLIST_H #define LL_LLCALLBACKLIST_H +#include "lldate.h" +#include "llsingleton.h" #include "llstl.h" -#include -#include +#include +#include +#include +#include +#include -class LLCallbackList +/***************************************************************************** +* LLCallbackList: callbacks every idle tick (every callFunctions() call) +*****************************************************************************/ +class LLCallbackList: public LLSingleton { + LLSINGLETON(LLCallbackList); public: typedef void (*callback_t)(void*); - typedef std::pair< callback_t,void* > callback_pair_t; - // NOTE: It is confirmed that we DEPEND on the order provided by using a list :( - // - typedef std::list< callback_pair_t > callback_list_t; - - LLCallbackList(); + typedef boost::signals2::signal callback_list_t; + typedef callback_list_t::slot_type callable_t; + typedef boost::signals2::connection handle_t; + typedef boost::signals2::scoped_connection temp_handle_t; + typedef std::function bool_func_t; + ~LLCallbackList(); - void addFunction( callback_t func, void *data = NULL ); // register a callback, which will be called as func(data) + handle_t addFunction( callback_t func, void *data = NULL ); // register a callback, which will be called as func(data) + handle_t addFunction( const callable_t& func ); bool containsFunction( callback_t func, void *data = NULL ); // true if list already contains the function/data pair bool deleteFunction( callback_t func, void *data = NULL ); // removes the first instance of this function/data pair from the list, false if not found - void callFunctions(); // calls all functions + void deleteFunction( const handle_t& handle ); + void callFunctions(); // calls all functions void deleteAllFunctions(); + handle_t doOnIdleOneTime( const callable_t& func ); + handle_t doOnIdleRepeating( const bool_func_t& func ); + bool isRunning(const handle_t& handle) const { return handle.connected(); }; + static void test(); protected: - - inline callback_list_t::iterator find(callback_t func, void *data); - callback_list_t mCallbackList; + + // "Additional specializations for std::pair and the standard container + // types, as well as utility functions to compose hashes are available in + // boost::hash." + // https://en.cppreference.com/w/cpp/utility/hash + typedef std::pair< callback_t,void* > callback_pair_t; + typedef std::unordered_map> lookup_table; + lookup_table mLookup; }; -typedef boost::function nullary_func_t; -typedef boost::function bool_func_t; +/*-------------------- legacy names in global namespace --------------------*/ +#define gIdleCallbacks (LLCallbackList::instance()) + +using nullary_func_t = LLCallbackList::callable_t; +using bool_func_t = LLCallbackList::bool_func_t; // Call a given callable once in idle loop. -void doOnIdleOneTime(nullary_func_t callable); +inline +LLCallbackList::handle_t doOnIdleOneTime(nullary_func_t callable) +{ + return gIdleCallbacks.doOnIdleOneTime(callable); +} // Repeatedly call a callable in idle loop until it returns true. -void doOnIdleRepeating(bool_func_t callable); +inline +LLCallbackList::handle_t doOnIdleRepeating(bool_func_t callable) +{ + return gIdleCallbacks.doOnIdleRepeating(callable); +} + +/***************************************************************************** +* LLLater: callbacks at some future time +*****************************************************************************/ +class LLLater: public LLSingleton +{ + LLSINGLETON(LLLater); + + using token_t = U32; + + // Define a struct for our priority queue entries, instead of using + // a tuple, because we need to define the comparison operator. + struct func_at + { + nullary_func_t mFunc; + LLDate::timestamp mTime; + token_t mToken; + + func_at(const nullary_func_t& func, LLDate::timestamp tm, token_t token): + mFunc(func), + mTime(tm), + mToken(token) + {} + + friend bool operator<(const func_at& lhs, const func_at& rhs) + { + // use greater-than because we want fibonacci_heap to select the + // EARLIEST time as the top() + return lhs.mTime > rhs.mTime; + } + }; + + // Accept default stable: when two funcs have the same timestamp, + // we don't care in what order they're called. + // Specify constant_time_size: we don't need to optimize the size() + // method, iow we don't need to store and maintain a count of entries. + typedef boost::heap::fibonacci_heap> + queue_t; + +public: + // If tasks that come ready during a given tick() take longer than this, + // defer any subsequent ready tasks to a future tick() call. + static constexpr F32 TIMESLICE{ 0.005f }; + class handle_t + { + private: + friend class LLLater; + token_t token; + public: + handle_t(token_t token=0): token(token) {} + bool operator==(const handle_t& rhs) const { return this->token == rhs.token; } + explicit operator bool() const { return bool(token); } + bool operator!() const { return ! bool(*this); } + }; + + // Call a given callable once at specified timestamp. + handle_t doAtTime(nullary_func_t callable, LLDate::timestamp time); + + // Call a given callable once after specified interval. + handle_t doAfterInterval(nullary_func_t callable, F32 seconds); + + // Call a given callable every specified number of seconds, until it returns true. + handle_t doPeriodically(bool_func_t callable, F32 seconds); + + // test whether specified handle is still live + bool isRunning(handle_t timer); + + // Cancel a future timer set by doAtTime(), doAfterInterval(), doPeriodically(). + // Return true iff the handle corresponds to a live timer. + bool cancel(const handle_t& timer); + // If we're canceling a non-const handle_t, also clear it so we need not + // cancel again. + bool cancel(handle_t& timer); + + // Store a handle_t returned by doAtTime(), doAfterInterval() or + // doPeriodically() in a temp_handle_t to cancel() automatically on + // destruction of the temp_handle_t. + class temp_handle_t + { + public: + temp_handle_t() {} + temp_handle_t(const handle_t& hdl): mHandle(hdl) {} + temp_handle_t(const temp_handle_t&) = delete; + temp_handle_t(temp_handle_t&&) = default; + temp_handle_t& operator=(const handle_t& hdl) + { + // initializing a new temp_handle_t, then swapping it into *this, + // takes care of destroying any previous mHandle + temp_handle_t replacement(hdl); + swap(replacement); + return *this; + } + temp_handle_t& operator=(const temp_handle_t&) = delete; + temp_handle_t& operator=(temp_handle_t&&) = default; + ~temp_handle_t() + { + cancel(); + } + + // temp_handle_t should be usable wherever handle_t is + operator handle_t() const { return mHandle; } + // If we're dealing with a non-const temp_handle_t, pass a reference + // to our handle_t member (e.g. to LLLater::cancel()). + operator handle_t&() { return mHandle; } + + // For those in the know, provide a cancel() method of our own that + // avoids LLLater::instance() lookup when mHandle isn't live. + bool cancel() + { + if (! mHandle) + { + return false; + } + else + { + return LLLater::instance().cancel(mHandle); + } + } + + void swap(temp_handle_t& other) noexcept + { + std::swap(this->mHandle, other.mHandle); + } + + private: + handle_t mHandle; + }; + +private: + bool tick(); + + // NOTE: We don't lock our data members because it doesn't make sense to + // register cross-thread callbacks. If we start wanting to use them on + // threads other than the main thread, it would make more sense to make + // our data members thread_local than to lock them. + + // the heap aka priority queue + queue_t mQueue; + // handles we've returned that haven't yet canceled + std::unordered_map mHandles; + token_t mToken{ 0 }; + // While mQueue is non-empty, register for regular callbacks. + LLCallbackList::temp_handle_t mLive; +}; + +/*-------------------- legacy names in global namespace --------------------*/ // Call a given callable once after specified interval. -void doAfterInterval(nullary_func_t callable, F32 seconds); +inline +LLLater::handle_t doAfterInterval(nullary_func_t callable, F32 seconds) +{ + return LLLater::instance().doAfterInterval(callable, seconds); +} // Call a given callable every specified number of seconds, until it returns true. -void doPeriodically(bool_func_t callable, F32 seconds); - -extern LLCallbackList gIdleCallbacks; +inline +LLLater::handle_t doPeriodically(bool_func_t callable, F32 seconds) +{ + return LLLater::instance().doPeriodically(callable, seconds); +} #endif diff --git a/indra/llcommon/lldate.cpp b/indra/llcommon/lldate.cpp index 2ddcf40895..6c23444820 100644 --- a/indra/llcommon/lldate.cpp +++ b/indra/llcommon/lldate.cpp @@ -41,9 +41,9 @@ #include "llstring.h" #include "llfasttimer.h" -static const F64 DATE_EPOCH = 0.0; +static const LLDate::timestamp DATE_EPOCH = 0.0; -static const F64 LL_APR_USEC_PER_SEC = 1000000.0; +static const LLDate::timestamp LL_APR_USEC_PER_SEC = 1000000.0; // should be APR_USEC_PER_SEC, but that relies on INT64_C which // isn't defined in glib under our build set up for some reason @@ -233,13 +233,13 @@ bool LLDate::fromStream(std::istream& s) return false; } - F64 seconds_since_epoch = time / LL_APR_USEC_PER_SEC; + timestamp seconds_since_epoch = time / LL_APR_USEC_PER_SEC; // check for fractional c = s.peek(); if(c == '.') { - F64 fractional = 0.0; + timestamp fractional = 0.0; s >> fractional; seconds_since_epoch += fractional; } @@ -299,12 +299,12 @@ bool LLDate::fromYMDHMS(S32 year, S32 month, S32 day, S32 hour, S32 min, S32 sec return true; } -F64 LLDate::secondsSinceEpoch() const +LLDate::timestamp LLDate::secondsSinceEpoch() const { return mSecondsSinceEpoch; } -void LLDate::secondsSinceEpoch(F64 seconds) +void LLDate::secondsSinceEpoch(timestamp seconds) { mSecondsSinceEpoch = seconds; } diff --git a/indra/llcommon/lldate.h b/indra/llcommon/lldate.h index be2cd2d051..c3d0cb97f3 100644 --- a/indra/llcommon/lldate.h +++ b/indra/llcommon/lldate.h @@ -44,6 +44,8 @@ class LL_COMMON_API LLDate { public: + using timestamp = F64; + /** * @brief Construct a date equal to epoch. */ @@ -103,14 +105,14 @@ public: * * @return The number of seconds since epoch UTC. */ - F64 secondsSinceEpoch() const; + timestamp secondsSinceEpoch() const; /** * @brief Set the date in seconds since epoch. * * @param seconds The number of seconds since epoch UTC. */ - void secondsSinceEpoch(F64 seconds); + void secondsSinceEpoch(timestamp seconds); /** * @brief Create an LLDate object set to the current time. @@ -147,7 +149,7 @@ public: private: - F64 mSecondsSinceEpoch; + timestamp mSecondsSinceEpoch; }; // Helper function to stream out a date diff --git a/indra/llcommon/llerrorlegacy.h b/indra/llcommon/llerrorlegacy.h deleted file mode 100644 index 31dd207008..0000000000 --- a/indra/llcommon/llerrorlegacy.h +++ /dev/null @@ -1,32 +0,0 @@ -/** - * @file llerrorlegacy.h - * @date January 2007 - * @brief old things from the older error system - * - * $LicenseInfo:firstyear=2007&license=viewerlgpl$ - * Second Life Viewer Source Code - * Copyright (C) 2010, Linden Research, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; - * version 2.1 of the License only. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - * - * Linden Research, Inc., 945 Battery Street, San Francisco, CA 94111 USA - * $/LicenseInfo$ - */ - -#ifndef LL_LLERRORLEGACY_H -#define LL_LLERRORLEGACY_H - - -#endif // LL_LLERRORLEGACY_H diff --git a/indra/llcommon/lleventfilter.cpp b/indra/llcommon/lleventfilter.cpp index 14c9c51830..e72ae7ad33 100644 --- a/indra/llcommon/lleventfilter.cpp +++ b/indra/llcommon/lleventfilter.cpp @@ -73,115 +73,52 @@ bool LLEventMatching::post(const LLSD& event) } /***************************************************************************** -* LLEventTimeoutBase +* LLEventTimeout *****************************************************************************/ -LLEventTimeoutBase::LLEventTimeoutBase(): +LLEventTimeout::LLEventTimeout(): LLEventFilter("timeout") { } -LLEventTimeoutBase::LLEventTimeoutBase(LLEventPump& source): +LLEventTimeout::LLEventTimeout(LLEventPump& source): LLEventFilter(source, "timeout") { } -void LLEventTimeoutBase::actionAfter(F32 seconds, const Action& action) +void LLEventTimeout::actionAfter(F32 seconds, const Action& action) { - setCountdown(seconds); - mAction = action; - if (! mMainloop.connected()) - { - LLEventPump& mainloop(LLEventPumps::instance().obtain("mainloop")); - mMainloop = mainloop.listen(getName(), [this](const LLSD& event){ return tick(event); }); - } + mTimer = LLLater::instance().doAfterInterval(action, seconds); } -class ErrorAfter -{ -public: - ErrorAfter(const std::string& message): mMessage(message) {} - - void operator()() - { - LL_ERRS("LLEventTimeout") << mMessage << LL_ENDL; - } - -private: - std::string mMessage; -}; - -void LLEventTimeoutBase::errorAfter(F32 seconds, const std::string& message) +void LLEventTimeout::errorAfter(F32 seconds, const std::string& message) { - actionAfter(seconds, ErrorAfter(message)); + actionAfter( + seconds, + [message=message] + { + LL_ERRS("LLEventTimeout") << message << LL_ENDL; + }); } -class EventAfter +void LLEventTimeout::eventAfter(F32 seconds, const LLSD& event) { -public: - EventAfter(LLEventPump& pump, const LLSD& event): - mPump(pump), - mEvent(event) - {} - - void operator()() - { - mPump.post(mEvent); - } - -private: - LLEventPump& mPump; - LLSD mEvent; -}; - -void LLEventTimeoutBase::eventAfter(F32 seconds, const LLSD& event) -{ - actionAfter(seconds, EventAfter(*this, event)); + actionAfter(seconds, [this, event]{ post(event); }); } -bool LLEventTimeoutBase::post(const LLSD& event) +bool LLEventTimeout::post(const LLSD& event) { cancel(); return LLEventStream::post(event); } -void LLEventTimeoutBase::cancel() -{ - mMainloop.disconnect(); -} - -bool LLEventTimeoutBase::tick(const LLSD&) -{ - if (countdownElapsed()) - { - cancel(); - mAction(); - } - return false; // show event to other listeners -} - -bool LLEventTimeoutBase::running() const -{ - return mMainloop.connected(); -} - -/***************************************************************************** -* LLEventTimeout -*****************************************************************************/ -LLEventTimeout::LLEventTimeout() {} - -LLEventTimeout::LLEventTimeout(LLEventPump& source): - LLEventTimeoutBase(source) +void LLEventTimeout::cancel() { + mTimer.cancel(); } -void LLEventTimeout::setCountdown(F32 seconds) +bool LLEventTimeout::running() const { - mTimer.setTimerExpirySec(seconds); -} - -bool LLEventTimeout::countdownElapsed() const -{ - return mTimer.hasExpired(); + return LLLater::instance().isRunning(mTimer); } /***************************************************************************** @@ -224,21 +161,21 @@ void LLEventBatch::setSize(std::size_t size) } /***************************************************************************** -* LLEventThrottleBase +* LLEventThrottle *****************************************************************************/ -LLEventThrottleBase::LLEventThrottleBase(F32 interval): +LLEventThrottle::LLEventThrottle(F32 interval): LLEventFilter("throttle"), mInterval(interval), mPosts(0) {} -LLEventThrottleBase::LLEventThrottleBase(LLEventPump& source, F32 interval): +LLEventThrottle::LLEventThrottle(LLEventPump& source, F32 interval): LLEventFilter(source, "throttle"), mInterval(interval), mPosts(0) {} -void LLEventThrottleBase::flush() +void LLEventThrottle::flush() { // flush() is a no-op unless there's something pending. // Don't test mPending because there's no requirement that the consumer @@ -259,12 +196,12 @@ void LLEventThrottleBase::flush() } } -LLSD LLEventThrottleBase::pending() const +LLSD LLEventThrottle::pending() const { return mPending; } -bool LLEventThrottleBase::post(const LLSD& event) +bool LLEventThrottle::post(const LLSD& event) { // Always capture most recent post() event data. If caller wants to // aggregate multiple events, let them retrieve pending() and modify @@ -289,13 +226,13 @@ bool LLEventThrottleBase::post(const LLSD& event) // timeRemaining tells us how much longer it will be until // mInterval seconds since the last flush() call. At that time, // flush() deferred events. - alarmActionAfter(timeRemaining, [this](){ flush(); }); + alarmActionAfter(timeRemaining, [this]{ flush(); }); } } return false; } -void LLEventThrottleBase::setInterval(F32 interval) +void LLEventThrottle::setInterval(F32 interval) { F32 oldInterval = mInterval; mInterval = interval; @@ -333,35 +270,24 @@ void LLEventThrottleBase::setInterval(F32 interval) } } -F32 LLEventThrottleBase::getDelay() const +F32 LLEventThrottle::getDelay() const { return timerGetRemaining(); } -/***************************************************************************** -* LLEventThrottle implementation -*****************************************************************************/ -LLEventThrottle::LLEventThrottle(F32 interval): - LLEventThrottleBase(interval) -{} - -LLEventThrottle::LLEventThrottle(LLEventPump& source, F32 interval): - LLEventThrottleBase(source, interval) -{} - -void LLEventThrottle::alarmActionAfter(F32 interval, const LLEventTimeoutBase::Action& action) +void LLEventThrottle::alarmActionAfter(F32 interval, const LLEventTimeout::Action& action) { - mAlarm.actionAfter(interval, action); + mAlarm = LLLater::instance().doAfterInterval(action, interval); } bool LLEventThrottle::alarmRunning() const { - return mAlarm.running(); + return LLLater::instance().isRunning(mAlarm); } void LLEventThrottle::alarmCancel() { - return mAlarm.cancel(); + LLLater::instance().cancel(mAlarm); } void LLEventThrottle::timerSet(F32 interval) diff --git a/indra/llcommon/lleventfilter.h b/indra/llcommon/lleventfilter.h index 88dc5a3015..1deb6f0f4c 100644 --- a/indra/llcommon/lleventfilter.h +++ b/indra/llcommon/lleventfilter.h @@ -29,13 +29,13 @@ #if ! defined(LL_LLEVENTFILTER_H) #define LL_LLEVENTFILTER_H +#include "llcallbacklist.h" #include "llevents.h" -#include "stdtypes.h" -#include "lltimer.h" #include "llsdutil.h" -#include +#include "lltimer.h" +#include "stdtypes.h" +#include -class LLEventTimer; class LLDate; /** @@ -78,22 +78,27 @@ private: /** * Wait for an event to be posted. If no such event arrives within a specified - * time, take a specified action. See LLEventTimeout for production - * implementation. - * - * @NOTE This is an abstract base class so that, for testing, we can use an - * alternate "timer" that doesn't actually consume real time. + * time, take a specified action. + * + * @NOTE: Caution should be taken when using the LLEventTimeout(LLEventPump &) + * constructor to ensure that the upstream event pump is not an LLEventMaildrop + * or any other kind of store and forward pump which may have events outstanding. + * Using this constructor will cause the upstream event pump to fire any pending + * events and could result in the invocation of a virtual method before the timeout + * has been fully constructed. The timeout should instead be constructed separately + * from the event pump and attached using the listen method. + * See llcoro::suspendUntilEventOnWithTimeout() for an example. */ -class LL_COMMON_API LLEventTimeoutBase: public LLEventFilter +class LL_COMMON_API LLEventTimeout: public LLEventFilter { public: /// construct standalone - LLEventTimeoutBase(); + LLEventTimeout(); /// construct and connect - LLEventTimeoutBase(LLEventPump& source); + LLEventTimeout(LLEventPump& source); /// Callable, can be constructed with boost::bind() - typedef boost::function Action; + typedef std::function Action; /** * Start countdown timer for the specified number of @a seconds. Forward @@ -120,8 +125,8 @@ public: * @endcode * * @NOTE - * The implementation relies on frequent events on the LLEventPump named - * "mainloop". + * The implementation relies on frequent calls to + * gIdleCallbacks.callFunctions(). */ void actionAfter(F32 seconds, const Action& action); @@ -134,7 +139,7 @@ public: * Instantiate an LLEventTimeout listening to that API and call * errorAfter() on each async request with a timeout comfortably longer * than the API's time guarantee (much longer than the anticipated - * "mainloop" granularity). + * gIdleCallbacks.callFunctions() granularity). * * Then if the async API breaks its promise, the program terminates with * the specified LL_ERRS @a message. The client of the async API can @@ -184,42 +189,9 @@ public: /// Is this timer currently running? bool running() const; -protected: - virtual void setCountdown(F32 seconds) = 0; - virtual bool countdownElapsed() const = 0; - private: - bool tick(const LLSD&); - - LLTempBoundListener mMainloop; - Action mAction; -}; - -/** - * Production implementation of LLEventTimoutBase. - * - * @NOTE: Caution should be taken when using the LLEventTimeout(LLEventPump &) - * constructor to ensure that the upstream event pump is not an LLEventMaildrop - * or any other kind of store and forward pump which may have events outstanding. - * Using this constructor will cause the upstream event pump to fire any pending - * events and could result in the invocation of a virtual method before the timeout - * has been fully constructed. The timeout should instead be connected upstream - * from the event pump and attached using the listen method. - * See llcoro::suspendUntilEventOnWithTimeout() for an example. - */ - -class LL_COMMON_API LLEventTimeout: public LLEventTimeoutBase -{ -public: - LLEventTimeout(); - LLEventTimeout(LLEventPump& source); - -protected: - virtual void setCountdown(F32 seconds); - virtual bool countdownElapsed() const; - -private: - LLTimer mTimer; + // Use a temp_handle_t so it's canceled on destruction. + LLLater::temp_handle_t mTimer; }; /** @@ -251,7 +223,7 @@ private: }; /** - * LLEventThrottleBase: construct with a time interval. Regardless of how + * LLEventThrottle: construct with a time interval. Regardless of how * frequently you call post(), LLEventThrottle will pass on an event to * its listeners no more often than once per specified interval. * @@ -284,13 +256,13 @@ private: * alternate "timer" that doesn't actually consume real time. See * LLEventThrottle. */ -class LL_COMMON_API LLEventThrottleBase: public LLEventFilter +class LL_COMMON_API LLEventThrottle: public LLEventFilter { public: // pass time interval - LLEventThrottleBase(F32 interval); + LLEventThrottle(F32 interval); // construct and connect - LLEventThrottleBase(LLEventPump& source, F32 interval); + LLEventThrottle(LLEventPump& source, F32 interval); // force out any deferred events void flush(); @@ -311,45 +283,24 @@ public: // time until next event would be passed through, 0.0 if now F32 getDelay() const; -protected: - // Implement these time-related methods for a valid LLEventThrottleBase - // subclass (see LLEventThrottle). For testing, we use a subclass that - // doesn't involve actual elapsed time. - virtual void alarmActionAfter(F32 interval, const LLEventTimeoutBase::Action& action) = 0; - virtual bool alarmRunning() const = 0; - virtual void alarmCancel() = 0; - virtual void timerSet(F32 interval) = 0; - virtual F32 timerGetRemaining() const = 0; - private: - // remember throttle interval - F32 mInterval; - // count post() calls since last flush() - std::size_t mPosts; + void alarmActionAfter(F32 interval, const LLEventTimeout::Action& action); + bool alarmRunning() const; + void alarmCancel(); + void timerSet(F32 interval); + F32 timerGetRemaining() const; + // pending event data from most recent deferred event LLSD mPending; -}; - -/** - * Production implementation of LLEventThrottle. - */ -class LLEventThrottle: public LLEventThrottleBase -{ -public: - LLEventThrottle(F32 interval); - LLEventThrottle(LLEventPump& source, F32 interval); - -private: - virtual void alarmActionAfter(F32 interval, const LLEventTimeoutBase::Action& action) /*override*/; - virtual bool alarmRunning() const /*override*/; - virtual void alarmCancel() /*override*/; - virtual void timerSet(F32 interval) /*override*/; - virtual F32 timerGetRemaining() const /*override*/; - - // use this to arrange a deferred flush() call - LLEventTimeout mAlarm; // use this to track whether we're within mInterval of last flush() LLTimer mTimer; + // count post() calls since last flush() + std::size_t mPosts; + // remember throttle interval + F32 mInterval; + + // use this to arrange a deferred flush() call + LLLater::handle_t mAlarm; }; /** diff --git a/indra/llcommon/lleventtimer.cpp b/indra/llcommon/lleventtimer.cpp index f575a7b6bf..b163ad375c 100644 --- a/indra/llcommon/lleventtimer.cpp +++ b/indra/llcommon/lleventtimer.cpp @@ -25,49 +25,34 @@ */ #include "linden_common.h" - #include "lleventtimer.h" -#include "u64.h" - - ////////////////////////////////////////////////////////////////////////////// // // LLEventTimer Implementation // ////////////////////////////////////////////////////////////////////////////// -LLEventTimer::LLEventTimer(F32 period) -: mEventTimer() +LLEventTimer::LLEventTimer(F32 period): + mPeriod(period) { - mPeriod = period; -} - -LLEventTimer::LLEventTimer(const LLDate& time) -: mEventTimer() -{ - mPeriod = (F32)(time.secondsSinceEpoch() - LLDate::now().secondsSinceEpoch()); + start(); } +LLEventTimer::LLEventTimer(const LLDate& time): + LLEventTimer(F32(time.secondsSinceEpoch() - LLDate::now().secondsSinceEpoch())) +{} LLEventTimer::~LLEventTimer() { } -//static -void LLEventTimer::updateClass() +void LLEventTimer::start() { - for (auto& timer : instance_snapshot()) - { - F32 et = timer.mEventTimer.getElapsedTimeF32(); - if (timer.mEventTimer.getStarted() && et > timer.mPeriod) { - timer.mEventTimer.reset(); - if ( timer.tick() ) - { - delete &timer; - } - } - } + mTimer = LLLater::instance().doPeriodically([this]{ return tick(); }, mPeriod); } - +void LLEventTimer::stop() +{ + LLLater::instance().cancel(mTimer); +} diff --git a/indra/llcommon/lleventtimer.h b/indra/llcommon/lleventtimer.h index b5d40a0622..34ff157e22 100644 --- a/indra/llcommon/lleventtimer.h +++ b/indra/llcommon/lleventtimer.h @@ -27,13 +27,12 @@ #ifndef LL_EVENTTIMER_H #define LL_EVENTTIMER_H -#include "stdtypes.h" +#include "llcallbacklist.h" #include "lldate.h" -#include "llinstancetracker.h" -#include "lltimer.h" +#include "stdtypes.h" // class for scheduling a function to be called at a given frequency (approximate, inprecise) -class LL_COMMON_API LLEventTimer : public LLInstanceTracker +class LL_COMMON_API LLEventTimer { public: @@ -41,14 +40,15 @@ public: LLEventTimer(const LLDate& time); virtual ~LLEventTimer(); - //function to be called at the supplied frequency - // Normally return FALSE; TRUE will delete the timer after the function returns. - virtual BOOL tick() = 0; + void start(); + void stop(); - static void updateClass(); + //function to be called at the supplied frequency + // Normally return false; true will delete the timer after the function returns. + virtual bool tick() = 0; protected: - LLTimer mEventTimer; + LLLater::temp_handle_t mTimer; F32 mPeriod; }; diff --git a/indra/llcommon/lllivefile.cpp b/indra/llcommon/lllivefile.cpp index ea485c2d86..692a21c1f1 100644 --- a/indra/llcommon/lllivefile.cpp +++ b/indra/llcommon/lllivefile.cpp @@ -170,7 +170,7 @@ namespace : LLEventTimer(refresh), mLiveFile(f) { } - BOOL tick() + bool tick() override { mLiveFile.checkAndReload(); return FALSE; diff --git a/indra/llcommon/tests/lleventfilter_test.cpp b/indra/llcommon/tests/lleventfilter_test.cpp index fa2cb03e95..38d6d0076e 100644 --- a/indra/llcommon/tests/lleventfilter_test.cpp +++ b/indra/llcommon/tests/lleventfilter_test.cpp @@ -51,6 +51,7 @@ // as we've carefully put all functionality except actual LLTimer calls into // LLEventTimeoutBase, that should suffice. We're not not not trying to test // LLTimer here. +#if 0 // time testing needs reworking class TestEventTimeout: public LLEventTimeoutBase { public: @@ -151,6 +152,7 @@ public: F32 mAlarmRemaining, mTimerRemaining; LLEventTimeoutBase::Action mAlarmAction; }; +#endif // time testing needs reworking /***************************************************************************** * TUT @@ -220,6 +222,8 @@ namespace tut void filter_object::test<2>() { set_test_name("LLEventTimeout::actionAfter()"); + skip("time testing needs reworking"); +#if 0 // time testing needs reworking LLEventPump& driver(pumps.obtain("driver")); TestEventTimeout filter(driver); listener0.reset(0); @@ -285,12 +289,15 @@ namespace tut filter.forceTimeout(); mainloop.post(17); check_listener("no timeout 6", listener1, LLSD(0)); +#endif // time testing needs reworking } template<> template<> void filter_object::test<3>() { set_test_name("LLEventTimeout::eventAfter()"); + skip("time testing needs reworking"); +#if 0 // time testing needs reworking LLEventPump& driver(pumps.obtain("driver")); TestEventTimeout filter(driver); listener0.reset(0); @@ -322,12 +329,15 @@ namespace tut filter.forceTimeout(); mainloop.post(17); check_listener("no timeout 3", listener0, LLSD(0)); +#endif // time testing needs reworking } template<> template<> void filter_object::test<4>() { set_test_name("LLEventTimeout::errorAfter()"); + skip("time testing needs reworking"); +#if 0 // time testing needs reworking WrapLLErrs capture; LLEventPump& driver(pumps.obtain("driver")); TestEventTimeout filter(driver); @@ -362,12 +372,15 @@ namespace tut filter.forceTimeout(); mainloop.post(17); check_listener("no timeout 3", listener0, LLSD(0)); +#endif // time testing needs reworking } template<> template<> void filter_object::test<5>() { set_test_name("LLEventThrottle"); + skip("time testing needs reworking"); +#if 0 // time testing needs reworking TestEventThrottle throttle(3); Concat cat; throttle.listen("concat", boost::ref(cat)); @@ -403,6 +416,7 @@ namespace tut throttle.advance(5); throttle.post(";17"); ensure_equals("17", cat.result, "136;12;17"); // "17" delivered +#endif // time testing needs reworking } template diff --git a/indra/llcommon/tests/llmainthreadtask_test.cpp b/indra/llcommon/tests/llmainthreadtask_test.cpp index 69b11ccafb..4a15e30a30 100644 --- a/indra/llcommon/tests/llmainthreadtask_test.cpp +++ b/indra/llcommon/tests/llmainthreadtask_test.cpp @@ -20,8 +20,8 @@ // other Linden headers #include "../test/lltut.h" #include "../test/sync.h" +#include "llcallbacklist.h" #include "llthread.h" // on_main_thread() -#include "lleventtimer.h" #include "lockstatic.h" /***************************************************************************** @@ -108,7 +108,7 @@ namespace tut lk.unlock(); // run the task -- should unblock thread, which will immediately block // on mSync - LLEventTimer::updateClass(); + LLCallbackList::instance().callFunctions(); // 'lk', having unlocked, can no longer be used to access; relock with // a new LockStatic instance ensure("should now have run", LockStatic()->ran); -- cgit v1.2.3 From 48e1979abaecc03af96e7e752e65c645083a4268 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 2 May 2024 23:57:29 -0400 Subject: Introduce LLLater::getRemaining(handle). Some timer use cases need to know not only whether the timer is active, but how much time remains before it (next) fires. Introduce LLLater::mDoneTimes to track, for each handle, the timestamp at which it's expected to fire. We can't just look up the target timestamp in mQueue's func_at entry because there's no documented way to navigate from a handle_type to a node iterator or pointer. Nor can we store it in mHandles because of order dependency: we need the mDoneTimes iterator so we can bind it into the Periodic functor for doPeriodically(), but we need the mQueue handle to store in mHandles. If we could find the mQueue node from the new handle, we could update the func_at entry after emplace() -- but if we could find the mQueue node from a handle, we wouldn't need to store the target timestamp separately anyway. Split LLLater::doAtTime() into internal doAtTime1() and doAtTime2(): the first creates an mDoneTimes entry and returns an iterator, the second finishes creating new mQueue and mHandles entries based on that mDoneTimes entry. This lets doPeriodically()'s Periodic bind the mDoneTimes iterator. Then instead of continually incrementing an internal data member, it increments the mDoneTimes entry to set the next upcoming timestamp. That lets getRemaining() report the next upcoming timestamp rather than only the original one. Add LLEventTimer::isRunning() and getRemaining(), forwarding to its LLLater handle. Fix various LLEventTimer subclass references to mEventTimer.stop(), etc. Fix non-inline LLEventTimer subclass tick() overrides for bool, not BOOL. Remove LLAppViewer::idle() call to LLEventTimer::updateClass(). Since LLApp::stepFrame() already calls LLCallbackList::callFunctions(), assume we've already handled that every tick. --- indra/llcommon/llcallbacklist.cpp | 64 +++++++++++++++++++++++++++++++-------- indra/llcommon/llcallbacklist.h | 23 ++++++++++---- indra/llcommon/lleventtimer.cpp | 10 ++++++ indra/llcommon/lleventtimer.h | 2 ++ 4 files changed, 80 insertions(+), 19 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llcallbacklist.cpp b/indra/llcommon/llcallbacklist.cpp index 7f7fdc7370..85c73fec8f 100644 --- a/indra/llcommon/llcallbacklist.cpp +++ b/indra/llcommon/llcallbacklist.cpp @@ -130,22 +130,40 @@ LLCallbackList::handle_t LLCallbackList::doOnIdleRepeating( const bool_func_t& f *****************************************************************************/ LLLater::LLLater() {} -// Call a given callable once at specified timestamp. -LLLater::handle_t LLLater::doAtTime(nullary_func_t callable, LLDate::timestamp time) +LLLater::DoneMap::iterator LLLater::doAtTime1(LLDate::timestamp time) { - bool first{ mQueue.empty() }; // Pick token FIRST to store a self-reference in mQueue's managed node as // well as in mHandles. Pre-increment to distinguish 0 from any live // handle_t. token_t token{ ++mToken }; - auto handle{ mQueue.emplace(callable, time, token) }; - mHandles.emplace(token, handle); + auto [iter, inserted]{ mDoneTimes.emplace(token, time) }; + llassert(inserted); + return iter; +} + +LLLater::handle_t LLLater::doAtTime2(nullary_func_t callable, DoneMap::iterator iter) +{ + bool first{ mQueue.empty() }; + // DoneMap::iterator references (token, time) pair + auto handle{ mQueue.emplace(callable, iter->first, iter->second) }; + auto hiter{ mHandles.emplace(iter->first, handle).first }; + // When called by Periodic, we're passed an existing DoneMap entry, + // meaning the token already exists, meaning emplace() will tell us it + // found an existing entry rather than creating a new one. In that case, + // it's essential to update the handle. + hiter->second = handle; if (first) { // If this is our first entry, register for regular callbacks. mLive = LLCallbackList::instance().doOnIdleRepeating([this]{ return tick(); }); } - return handle_t{ token }; + return { iter->first }; +} + +// Call a given callable once at specified timestamp. +LLLater::handle_t LLLater::doAtTime(nullary_func_t callable, LLDate::timestamp time) +{ + return doAtTime2(callable, doAtTime1(time)); } // Call a given callable once after specified interval. @@ -160,11 +178,11 @@ LLLater::handle_t LLLater::doAfterInterval(nullary_func_t callable, F32 seconds) // For doPeriodically(), we need a struct rather than a lambda because a // struct, unlike a lambda, has access to 'this'. -struct Periodic +struct LLLater::Periodic { LLLater* mLater; + DoneMap::iterator mDone; bool_func_t mCallable; - LLDate::timestamp mNext; F32 mSeconds; void operator()() @@ -175,8 +193,9 @@ struct Periodic // Don't call doAfterInterval(), which rereads LLDate::now(), // since that would defer by however long it took us to wake // up and notice plus however long callable() took to run. - mNext += mSeconds; - mLater->doAtTime(*this, mNext); + // Bump our mDoneTimes entry so getRemaining() can track. + mDone->second += mSeconds; + mLater->doAtTime2(*this, mDone); } } }; @@ -186,17 +205,32 @@ LLLater::handle_t LLLater::doPeriodically(bool_func_t callable, F32 seconds) { // Passing seconds <= 0 will produce an infinite loop. llassert(seconds > 0); - auto next{ LLDate::now().secondsSinceEpoch() + seconds }; - return doAtTime(Periodic{ this, callable, next, seconds }, next); + auto iter{ doAtTime1(LLDate::now().secondsSinceEpoch() + seconds) }; + // The whole reason we split doAtTime() into doAtTime1() and doAtTime2() + // is to be able to bind the mDoneTimes entry into Periodic. + return doAtTime2(Periodic{ this, iter, callable, seconds }, iter); } -bool LLLater::isRunning(handle_t timer) +bool LLLater::isRunning(handle_t timer) const { // A default-constructed timer isn't running. // A timer we don't find in mHandles has fired or been canceled. return timer && mHandles.find(timer.token) != mHandles.end(); } +F32 LLLater::getRemaining(handle_t timer) const +{ + auto found{ mDoneTimes.find(timer.token) }; + if (found == mDoneTimes.end()) + { + return 0.f; + } + else + { + return found->second - LLDate::now().secondsSinceEpoch(); + } +} + // Cancel a future timer set by doAtTime(), doAfterInterval(), doPeriodically() bool LLLater::cancel(handle_t& timer) { @@ -240,6 +274,8 @@ bool LLLater::cancel(const handle_t& timer) mQueue.erase(found->second); // before erasing timer.token from mHandles mHandles.erase(found); + // don't forget to erase mDoneTimes entry + mDoneTimes.erase(timer.token); if (mQueue.empty()) { // If that was the last active timer, unregister for callbacks. @@ -282,6 +318,8 @@ bool LLLater::tick() auto current{ top }; // remove the mHandles entry referencing this task mHandles.erase(current.mToken); + // and the mDoneTimes entry + mDoneTimes.erase(current.mToken); // before removing the mQueue task entry itself mQueue.pop(); // okay, NOW run diff --git a/indra/llcommon/llcallbacklist.h b/indra/llcommon/llcallbacklist.h index 522a9b838b..17adb7f431 100644 --- a/indra/llcommon/llcallbacklist.h +++ b/indra/llcommon/llcallbacklist.h @@ -114,13 +114,13 @@ class LLLater: public LLSingleton struct func_at { nullary_func_t mFunc; - LLDate::timestamp mTime; token_t mToken; + LLDate::timestamp mTime; - func_at(const nullary_func_t& func, LLDate::timestamp tm, token_t token): + func_at(const nullary_func_t& func, token_t token, LLDate::timestamp tm): mFunc(func), - mTime(tm), - mToken(token) + mToken(token), + mTime(tm) {} friend bool operator<(const func_at& lhs, const func_at& rhs) @@ -165,7 +165,9 @@ public: handle_t doPeriodically(bool_func_t callable, F32 seconds); // test whether specified handle is still live - bool isRunning(handle_t timer); + bool isRunning(handle_t timer) const; + // check remaining time + F32 getRemaining(handle_t timer) const; // Cancel a future timer set by doAtTime(), doAfterInterval(), doPeriodically(). // Return true iff the handle corresponds to a live timer. @@ -239,10 +241,19 @@ private: // the heap aka priority queue queue_t mQueue; // handles we've returned that haven't yet canceled - std::unordered_map mHandles; + using HandleMap = std::unordered_map; + HandleMap mHandles; + using DoneMap = std::unordered_map; + DoneMap mDoneTimes; token_t mToken{ 0 }; // While mQueue is non-empty, register for regular callbacks. LLCallbackList::temp_handle_t mLive; + + struct Periodic; + + // internal implementation for doAtTime() + DoneMap::iterator doAtTime1(LLDate::timestamp time); + handle_t doAtTime2(nullary_func_t callable, DoneMap::iterator iter); }; /*-------------------- legacy names in global namespace --------------------*/ diff --git a/indra/llcommon/lleventtimer.cpp b/indra/llcommon/lleventtimer.cpp index b163ad375c..0f8d1e636f 100644 --- a/indra/llcommon/lleventtimer.cpp +++ b/indra/llcommon/lleventtimer.cpp @@ -56,3 +56,13 @@ void LLEventTimer::stop() { LLLater::instance().cancel(mTimer); } + +bool LLEventTimer::isRunning() +{ + return LLLater::instance().isRunning(mTimer); +} + +F32 LLEventTimer::getRemaining() +{ + return LLLater::instance().getRemaining(mTimer); +} diff --git a/indra/llcommon/lleventtimer.h b/indra/llcommon/lleventtimer.h index 34ff157e22..05d8bc038d 100644 --- a/indra/llcommon/lleventtimer.h +++ b/indra/llcommon/lleventtimer.h @@ -42,6 +42,8 @@ public: void start(); void stop(); + bool isRunning(); + F32 getRemaining(); //function to be called at the supplied frequency // Normally return false; true will delete the timer after the function returns. -- cgit v1.2.3 From 9f620efa9dd60c5de6b7ea807d53bba922294726 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 3 May 2024 08:52:32 -0400 Subject: Make LLLater store target time in mHandles; ditch 2nd unordered_map. Instead of maintaining a whole separate unordered_map to look up target times, make room in the HandleMap entry for the target time. There's still circularity, but the split into doAtTime1() and doAtTime2() resolves it: since doAtTime2() accepts the mHandles iterator created by doAtTime1(), doAtTime2() can simply store the new mQueue handle_type into the appropriate slot. Also sprinkle in a few more override keywords for consistency. --- indra/llcommon/llcallbacklist.cpp | 50 +++++++++++++++++++-------------------- indra/llcommon/llcallbacklist.h | 10 ++++---- 2 files changed, 30 insertions(+), 30 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llcallbacklist.cpp b/indra/llcommon/llcallbacklist.cpp index 85c73fec8f..19b948a5e1 100644 --- a/indra/llcommon/llcallbacklist.cpp +++ b/indra/llcommon/llcallbacklist.cpp @@ -130,33 +130,34 @@ LLCallbackList::handle_t LLCallbackList::doOnIdleRepeating( const bool_func_t& f *****************************************************************************/ LLLater::LLLater() {} -LLLater::DoneMap::iterator LLLater::doAtTime1(LLDate::timestamp time) +LLLater::HandleMap::iterator LLLater::doAtTime1(LLDate::timestamp time) { // Pick token FIRST to store a self-reference in mQueue's managed node as // well as in mHandles. Pre-increment to distinguish 0 from any live // handle_t. token_t token{ ++mToken }; - auto [iter, inserted]{ mDoneTimes.emplace(token, time) }; + // For the moment, store a default-constructed mQueue handle -- + // doAtTime2() will fill in. + auto [iter, inserted]{ mHandles.emplace( + token, + HandleMap::mapped_type{ queue_t::handle_type(), time }) }; llassert(inserted); return iter; } -LLLater::handle_t LLLater::doAtTime2(nullary_func_t callable, DoneMap::iterator iter) +LLLater::handle_t LLLater::doAtTime2(nullary_func_t callable, HandleMap::iterator iter) { bool first{ mQueue.empty() }; - // DoneMap::iterator references (token, time) pair - auto handle{ mQueue.emplace(callable, iter->first, iter->second) }; - auto hiter{ mHandles.emplace(iter->first, handle).first }; - // When called by Periodic, we're passed an existing DoneMap entry, - // meaning the token already exists, meaning emplace() will tell us it - // found an existing entry rather than creating a new one. In that case, - // it's essential to update the handle. - hiter->second = handle; + // HandleMap::iterator references (token, (handle, time)) pair + auto handle{ mQueue.emplace(callable, iter->first, iter->second.second) }; + // Now that we have an mQueue handle_type, store it in mHandles entry. + iter->second.first = handle; if (first) { // If this is our first entry, register for regular callbacks. mLive = LLCallbackList::instance().doOnIdleRepeating([this]{ return tick(); }); } + // Make an LLLater::handle_t from token. return { iter->first }; } @@ -181,7 +182,7 @@ LLLater::handle_t LLLater::doAfterInterval(nullary_func_t callable, F32 seconds) struct LLLater::Periodic { LLLater* mLater; - DoneMap::iterator mDone; + HandleMap::iterator mHandleEntry; bool_func_t mCallable; F32 mSeconds; @@ -193,9 +194,10 @@ struct LLLater::Periodic // Don't call doAfterInterval(), which rereads LLDate::now(), // since that would defer by however long it took us to wake // up and notice plus however long callable() took to run. - // Bump our mDoneTimes entry so getRemaining() can track. - mDone->second += mSeconds; - mLater->doAtTime2(*this, mDone); + // Bump the time in our mHandles entry so getRemaining() can see. + // HandleMap::iterator references (token, (handle, time)) pair. + mHandleEntry->second.second += mSeconds; + mLater->doAtTime2(*this, mHandleEntry); } } }; @@ -207,7 +209,7 @@ LLLater::handle_t LLLater::doPeriodically(bool_func_t callable, F32 seconds) llassert(seconds > 0); auto iter{ doAtTime1(LLDate::now().secondsSinceEpoch() + seconds) }; // The whole reason we split doAtTime() into doAtTime1() and doAtTime2() - // is to be able to bind the mDoneTimes entry into Periodic. + // is to be able to bind the mHandles entry into Periodic. return doAtTime2(Periodic{ this, iter, callable, seconds }, iter); } @@ -220,14 +222,15 @@ bool LLLater::isRunning(handle_t timer) const F32 LLLater::getRemaining(handle_t timer) const { - auto found{ mDoneTimes.find(timer.token) }; - if (found == mDoneTimes.end()) + auto found{ mHandles.find(timer.token) }; + if (found == mHandles.end()) { return 0.f; } else { - return found->second - LLDate::now().secondsSinceEpoch(); + // HandleMap::iterator references (token, (handle, time)) pair + return found->second.second - LLDate::now().secondsSinceEpoch(); } } @@ -270,12 +273,11 @@ bool LLLater::cancel(const handle_t& timer) return false; } - // erase from mQueue the handle_t referenced by timer.token - mQueue.erase(found->second); + // HandleMap::iterator references (token, (handle, time)) pair. + // Erase from mQueue the handle_type referenced by timer.token. + mQueue.erase(found->second.first); // before erasing timer.token from mHandles mHandles.erase(found); - // don't forget to erase mDoneTimes entry - mDoneTimes.erase(timer.token); if (mQueue.empty()) { // If that was the last active timer, unregister for callbacks. @@ -318,8 +320,6 @@ bool LLLater::tick() auto current{ top }; // remove the mHandles entry referencing this task mHandles.erase(current.mToken); - // and the mDoneTimes entry - mDoneTimes.erase(current.mToken); // before removing the mQueue task entry itself mQueue.pop(); // okay, NOW run diff --git a/indra/llcommon/llcallbacklist.h b/indra/llcommon/llcallbacklist.h index 17adb7f431..3ff1aad04e 100644 --- a/indra/llcommon/llcallbacklist.h +++ b/indra/llcommon/llcallbacklist.h @@ -241,10 +241,10 @@ private: // the heap aka priority queue queue_t mQueue; // handles we've returned that haven't yet canceled - using HandleMap = std::unordered_map; + using HandleMap = std::unordered_map< + token_t, + std::pair>; HandleMap mHandles; - using DoneMap = std::unordered_map; - DoneMap mDoneTimes; token_t mToken{ 0 }; // While mQueue is non-empty, register for regular callbacks. LLCallbackList::temp_handle_t mLive; @@ -252,8 +252,8 @@ private: struct Periodic; // internal implementation for doAtTime() - DoneMap::iterator doAtTime1(LLDate::timestamp time); - handle_t doAtTime2(nullary_func_t callable, DoneMap::iterator iter); + HandleMap::iterator doAtTime1(LLDate::timestamp time); + handle_t doAtTime2(nullary_func_t callable, HandleMap::iterator iter); }; /*-------------------- legacy names in global namespace --------------------*/ -- cgit v1.2.3 From 922277764d41e96a1c41291272bb70e3a1b8c677 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 3 May 2024 09:27:16 -0400 Subject: Prevent LLLater from thrashing on LLCallbackList. If there is exactly one doPeriodically() entry on LLLater, the mQueue entry is deleted before Periodic::operator()() is called. If Periodic's callable returns false, it reinstates itself on mQueue for a future time. That's okay, because the test for mQueue.empty(), and the consequent disconnect from LLCallbackList, happens only after that Periodic call returns. But at the moment Periodic reinstates itself on mQueue, mQueue happens to be empty. That alerts doAtTime2() to register itself on LLCallbackList -- even though at that moment it's already registered. Semantically that's okay because assigning to the LLLater's LLCallbackList::temp_handle_t should implicitly disconnect the previous connection. But it's pointless to create a new connection and disconnect the old one every time we call that lone Periodic. Add a test for mLive.connected(); that way if we already have an LLCallbackList connection, we retain it instead of replacing it with a new one. --- indra/llcommon/llcallbacklist.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llcallbacklist.cpp b/indra/llcommon/llcallbacklist.cpp index 19b948a5e1..992c83b4d2 100644 --- a/indra/llcommon/llcallbacklist.cpp +++ b/indra/llcommon/llcallbacklist.cpp @@ -152,7 +152,7 @@ LLLater::handle_t LLLater::doAtTime2(nullary_func_t callable, HandleMap::iterato auto handle{ mQueue.emplace(callable, iter->first, iter->second.second) }; // Now that we have an mQueue handle_type, store it in mHandles entry. iter->second.first = handle; - if (first) + if (first && ! mLive.connected()) { // If this is our first entry, register for regular callbacks. mLive = LLCallbackList::instance().doOnIdleRepeating([this]{ return tick(); }); -- cgit v1.2.3 From c89d51c60a6f42a5e279e2c9e06adcf1f13822c0 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 3 May 2024 11:46:00 -0400 Subject: Log a stack trace on LL_ERRS(). --- indra/llcommon/llerror.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 0f48ce16b2..ccdf3b60a8 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -44,6 +44,8 @@ # include #endif // !LL_WINDOWS #include +#define BOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED +#include #include "string.h" #include "llapp.h" @@ -1429,16 +1431,17 @@ namespace LLError message_stream << message; message = message_stream.str(); } - + writeToRecorders(site, message); if (site.mLevel == LEVEL_ERROR) { + writeToRecorders(site, stringize(boost::stacktrace::stacktrace())); g->mFatalMessage = message; - if (s->mCrashFunction) - { - s->mCrashFunction(message); - } + if (s->mCrashFunction) + { + s->mCrashFunction(message); + } } } } -- cgit v1.2.3 From d4f384b4ec55758a7f2ca4338894ea6cacc98eec Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 7 May 2024 16:13:03 -0400 Subject: Refactor LLLater -> LL::Timers to accommodate nonpositive repeats. In the previous design, the tick() method ran each task exactly once. doPeriodically() was implemented by posting a functor that would, after calling the specified callable, repost itself at (timestamp + interval). The trouble with that design is that it required (interval > 0). A nonpositive interval would result in looping over any timers with nonpositive repetition intervals without ever returning from the tick() method. To avoid that, doPeriodically() contained an llassert(interval > 0). Unfortunately the viewer failed that constraint immediately at login, leading to the suspicion that eliminating every such usage might require a protracted search. Lifting that restriction required a redesign. Now the priority queue stores a callable returning bool, and the tick() method itself contains the logic to repost a recurring task -- but defers doing so until after it stops looping over ready tasks, ensuring that a task with a nonpositive interval will at least wait until the following tick() call. This simplifies not only doPeriodically(), but also doAtTime(). The previous split of doAtTime() into doAtTime1() and doAtTime2() was only to accommodate the needs of the Periodic functor class. Ditch Periodic. Per feedback from NickyD, rename doAtTime() to scheduleAt(), which wraps its passed nullary callable into a callable that unconditionally returns true (so tick() will run it only once). Rename the doAfterInterval() method to scheduleAfter(), which similarly wraps its nullary callable. However, the legacy doAfterInterval() free function remains. scheduleAfter() also loses its llassert(seconds > 0). Rename the doPeriodically() method to scheduleRepeating(). However, the legacy doPeriodically() free function remains. Add internal scheduleAtRepeating(), whose role is to accept both a specific timestamp and a repetition interval (which might be ignored, depending on the callable). scheduleAtRepeating() now contains the real logic to add a task. Rename getRemaining() to timeUntilCall(), hopefully resolving the question of "remaining what?" Expand the std::pair metadata stored in Timers's auxiliary unordered_map to a Metadata struct containing the repetition interval plus two bools to mediate deferred cancel() processing. Rename HandleMap to MetaMap, mHandles to mMeta. Defend against the case when cancel(handle) is reached during the call to that handle's callable. Meta::mRunning is set for the duration of that call. When cancel() sees mRunning, instead of immediately deleting map entries, it sets mCancel. Upon return from a task's callable, tick() notices mCancel and behaves as if the callable returned true to stop the series of calls. To guarantee that mRunning doesn't inadvertently remain set even in the case of an exception, introduce local RAII class TempSet whose constructor accepts a non-const variable reference and a desired value. The constructor captures the current value and sets the desired value; the destructor restores the previous value. Defend against exception in a task's callable, and stop calling that task. Use LOG_UNHANDLED_EXCEPTION() to report it. --- indra/llcommon/llcallbacklist.cpp | 266 +++++++++++++++++++++++--------------- indra/llcommon/llcallbacklist.h | 94 +++++++++----- indra/llcommon/lleventfilter.cpp | 10 +- indra/llcommon/lleventfilter.h | 4 +- indra/llcommon/lleventtimer.cpp | 8 +- indra/llcommon/lleventtimer.h | 2 +- 6 files changed, 233 insertions(+), 151 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llcallbacklist.cpp b/indra/llcommon/llcallbacklist.cpp index 992c83b4d2..52e9860e02 100644 --- a/indra/llcommon/llcallbacklist.cpp +++ b/indra/llcommon/llcallbacklist.cpp @@ -25,6 +25,8 @@ */ #include "llcallbacklist.h" +#include "llexception.h" +#include // // Member functions @@ -126,116 +128,83 @@ LLCallbackList::handle_t LLCallbackList::doOnIdleRepeating( const bool_func_t& f } /***************************************************************************** -* LLLater +* LL::Timers *****************************************************************************/ -LLLater::LLLater() {} - -LLLater::HandleMap::iterator LLLater::doAtTime1(LLDate::timestamp time) +namespace LL { - // Pick token FIRST to store a self-reference in mQueue's managed node as - // well as in mHandles. Pre-increment to distinguish 0 from any live - // handle_t. - token_t token{ ++mToken }; - // For the moment, store a default-constructed mQueue handle -- - // doAtTime2() will fill in. - auto [iter, inserted]{ mHandles.emplace( - token, - HandleMap::mapped_type{ queue_t::handle_type(), time }) }; - llassert(inserted); - return iter; -} -LLLater::handle_t LLLater::doAtTime2(nullary_func_t callable, HandleMap::iterator iter) -{ - bool first{ mQueue.empty() }; - // HandleMap::iterator references (token, (handle, time)) pair - auto handle{ mQueue.emplace(callable, iter->first, iter->second.second) }; - // Now that we have an mQueue handle_type, store it in mHandles entry. - iter->second.first = handle; - if (first && ! mLive.connected()) - { - // If this is our first entry, register for regular callbacks. - mLive = LLCallbackList::instance().doOnIdleRepeating([this]{ return tick(); }); - } - // Make an LLLater::handle_t from token. - return { iter->first }; -} +Timers::Timers() {} // Call a given callable once at specified timestamp. -LLLater::handle_t LLLater::doAtTime(nullary_func_t callable, LLDate::timestamp time) +Timers::handle_t Timers::scheduleAt(nullary_func_t callable, LLDate::timestamp time) { - return doAtTime2(callable, doAtTime1(time)); + // tick() assumes you want to run periodically until you return true. + // Schedule a task that returns true after a single call. + return scheduleAtRepeating(once(callable), time, 0); } // Call a given callable once after specified interval. -LLLater::handle_t LLLater::doAfterInterval(nullary_func_t callable, F32 seconds) +Timers::handle_t Timers::scheduleAfter(nullary_func_t callable, F32 seconds) { - // Passing 0 is a slightly more expensive way of calling - // LLCallbackList::doOnIdleOneTime(). Are we sure the caller is correct? - // (If there's a valid use case, remove the llassert() and carry on.) - llassert(seconds > 0); - return doAtTime(callable, LLDate::now().secondsSinceEpoch() + seconds); + return scheduleRepeating(once(callable), seconds); } -// For doPeriodically(), we need a struct rather than a lambda because a -// struct, unlike a lambda, has access to 'this'. -struct LLLater::Periodic +// Call a given callable every specified number of seconds, until it returns true. +Timers::handle_t Timers::scheduleRepeating(bool_func_t callable, F32 seconds) { - LLLater* mLater; - HandleMap::iterator mHandleEntry; - bool_func_t mCallable; - F32 mSeconds; + return scheduleAtRepeating(callable, now() + seconds, seconds); +} - void operator()() +Timers::handle_t Timers::scheduleAtRepeating(bool_func_t callable, + LLDate::timestamp time, F32 interval) +{ + // Pick token FIRST to store a self-reference in mQueue's managed node as + // well as in mMeta. Pre-increment to distinguish 0 from any live + // handle_t. + token_t token{ ++mToken }; + // For the moment, store a default-constructed mQueue handle -- + // we'll fill in later. + auto [iter, inserted] = mMeta.emplace(token, + Metadata{ queue_t::handle_type(), time, interval }); + // It's important that our token is unique. + llassert(inserted); + + // Remember whether this is the first entry in mQueue + bool first{ mQueue.empty() }; + auto handle{ mQueue.emplace(callable, token, time) }; + // Now that we have an mQueue handle_type, store it in mMeta entry. + iter->second.mHandle = handle; + if (first && ! mLive.connected()) { - if (! mCallable()) - { - // Returning false means please schedule another call. - // Don't call doAfterInterval(), which rereads LLDate::now(), - // since that would defer by however long it took us to wake - // up and notice plus however long callable() took to run. - // Bump the time in our mHandles entry so getRemaining() can see. - // HandleMap::iterator references (token, (handle, time)) pair. - mHandleEntry->second.second += mSeconds; - mLater->doAtTime2(*this, mHandleEntry); - } + // If this is our first entry, register for regular callbacks. + mLive = LLCallbackList::instance().doOnIdleRepeating([this]{ return tick(); }); } -}; - -// Call a given callable every specified number of seconds, until it returns true. -LLLater::handle_t LLLater::doPeriodically(bool_func_t callable, F32 seconds) -{ - // Passing seconds <= 0 will produce an infinite loop. - llassert(seconds > 0); - auto iter{ doAtTime1(LLDate::now().secondsSinceEpoch() + seconds) }; - // The whole reason we split doAtTime() into doAtTime1() and doAtTime2() - // is to be able to bind the mHandles entry into Periodic. - return doAtTime2(Periodic{ this, iter, callable, seconds }, iter); + // Make an Timers::handle_t from token. + return { token }; } -bool LLLater::isRunning(handle_t timer) const +bool Timers::isRunning(handle_t timer) const { // A default-constructed timer isn't running. - // A timer we don't find in mHandles has fired or been canceled. - return timer && mHandles.find(timer.token) != mHandles.end(); + // A timer we don't find in mMeta has fired or been canceled. + return timer && mMeta.find(timer.token) != mMeta.end(); } -F32 LLLater::getRemaining(handle_t timer) const +F32 Timers::timeUntilCall(handle_t timer) const { - auto found{ mHandles.find(timer.token) }; - if (found == mHandles.end()) + MetaMap::const_iterator found; + if ((! timer) || (found = mMeta.find(timer.token)) == mMeta.end()) { return 0.f; } else { - // HandleMap::iterator references (token, (handle, time)) pair - return found->second.second - LLDate::now().secondsSinceEpoch(); + return found->second.mTime - now(); } } -// Cancel a future timer set by doAtTime(), doAfterInterval(), doPeriodically() -bool LLLater::cancel(handle_t& timer) +// Cancel a future timer set by scheduleAt(), scheduleAfter(), scheduleRepeating() +bool Timers::cancel(handle_t& timer) { // For exception safety, capture and clear timer before canceling. // Once we've canceled this handle, don't retain the live handle. @@ -244,7 +213,7 @@ bool LLLater::cancel(handle_t& timer) return cancel(ctimer); } -bool LLLater::cancel(const handle_t& timer) +bool Timers::cancel(const handle_t& timer) { if (! timer) { @@ -257,27 +226,38 @@ bool LLLater::cancel(const handle_t& timer) // Nor do we find any documented way to ask whether a given handle still // tracks a valid heap node. That's why we capture all returned handles in - // mHandles and validate against that collection. What about the pop() + // mMeta and validate against that collection. What about the pop() // call in tick()? How to map from the top() value back to the // corresponding handle_t? That's why we store func_at::mToken. // fibonacci_heap provides a pair of begin()/end() methods to iterate over // all nodes (NOT in heap order), plus a function to convert from such - // iterators to handles. Without mHandles, that would be our only chance + // iterators to handles. Without mMeta, that would be our only chance // to validate. - auto found{ mHandles.find(timer.token) }; - if (found == mHandles.end()) + auto found{ mMeta.find(timer.token) }; + if (found == mMeta.end()) { // we don't recognize this handle -- maybe the timer has already // fired, maybe it was previously canceled. return false; } - // HandleMap::iterator references (token, (handle, time)) pair. + // Funny case: what if the callback directly or indirectly reaches a + // cancel() call for its own handle? + if (found->second.mRunning) + { + // tick() has special logic to defer the actual deletion until the + // callback has returned + found->second.mCancel = true; + // this handle does in fact reference a live timer, + // which we're going to cancel when we get a chance + return true; + } + // Erase from mQueue the handle_type referenced by timer.token. - mQueue.erase(found->second.first); - // before erasing timer.token from mHandles - mHandles.erase(found); + mQueue.erase(found->second.mHandle); + // before erasing the mMeta entry + mMeta.erase(found); if (mQueue.empty()) { // If that was the last active timer, unregister for callbacks. @@ -289,7 +269,33 @@ bool LLLater::cancel(const handle_t& timer) return true; } -bool LLLater::tick() +// RAII class to set specified variable to specified value +// only for the duration of containing scope +template +class TempSet +{ +public: + TempSet(VAR& var, const VALUE& value): + mVar(var), + mOldValue(mVar) + { + mVar = value; + } + + TempSet(const TempSet&) = delete; + TempSet& operator=(const TempSet&) = delete; + + ~TempSet() + { + mVar = mOldValue; + } + +private: + VAR& mVar; + VALUE mOldValue; +}; + +bool Timers::tick() { // Fetch current time only on entry, even though running some mQueue task // may take long enough that the next one after would become ready. We're @@ -297,34 +303,84 @@ bool LLLater::tick() // starve it if we have a sequence of tasks that take nontrivial time. auto now{ LLDate::now().secondsSinceEpoch() }; auto cutoff{ now + TIMESLICE }; + + // Capture tasks we've processed but that want to be rescheduled. + // Defer rescheduling them immediately to avoid getting stuck looping over + // a recurring task with a nonpositive interval. + std::vector> deferred; + while (! mQueue.empty()) { auto& top{ mQueue.top() }; if (top.mTime > now) { // we've hit an entry that's still in the future: - // done with this tick(), but schedule another call - return false; + // done with this tick() + break; } if (LLDate::now().secondsSinceEpoch() > cutoff) { // we still have ready tasks, but we've already eaten too much - // time this tick() -- defer until next tick() -- call again - return false; + // time this tick() -- defer until next tick() + break; } - // Found a ready task. Hate to copy stuff, but -- what if the task - // indirectly ends up trying to cancel a handle referencing its own - // node in mQueue? If the task has any state, that would be Bad. Copy - // the node before running it. - auto current{ top }; - // remove the mHandles entry referencing this task - mHandles.erase(current.mToken); - // before removing the mQueue task entry itself + // Found a ready task. Look up its corresponding mMeta entry. + auto meta{ mMeta.find(top.mToken) }; + llassert(meta != mMeta.end()); + bool done; + { + // Mark our mMeta entry so we don't cancel this timer while its + // callback is running, but unmark it even in case of exception. + TempSet running(meta->second.mRunning, true); + // run the callback and capture its desire to end repetition + try + { + done = top.mFunc(); + } + catch (...) + { + // Don't crash if a timer callable throws. + // But don't continue calling that callable, either. + done = true; + LOG_UNHANDLED_EXCEPTION("LL::Timers"); + } + } // clear mRunning + + // If mFunc() returned true (all done, stop calling me) or + // meta->mCancel (somebody tried to cancel this timer during the + // callback call), then we're done: clean up both entries. + if (done || meta->second.mCancel) + { + // remove the mMeta entry referencing this task + mMeta.erase(meta); + } + else + { + // mFunc returned false, and nobody asked to cancel: + // continue calling this task at a future time. + meta->second.mTime += meta->second.mInterval; + // capture this task to reschedule once we break loop + deferred.push_back({meta, top}); + // update func_at's mTime to match meta's + deferred.back().second.mTime = meta->second.mTime; + } + // Remove the mQueue entry regardless, or we risk stalling the + // queue right here if we have a nonpositive interval. mQueue.pop(); - // okay, NOW run - current.mFunc(); } - // queue is empty: stop callbacks - return true; + + // Now reschedule any tasks that need to be rescheduled. + for (const auto& [meta, task] : deferred) + { + auto handle{ mQueue.push(task) }; + // track this new mQueue handle_type + meta->second.mHandle = handle; + } + + // If, after all the twiddling above, our queue ended up empty, + // stop calling every tick. + return mQueue.empty(); } + +} // namespace LL diff --git a/indra/llcommon/llcallbacklist.h b/indra/llcommon/llcallbacklist.h index 3ff1aad04e..f9b15867ef 100644 --- a/indra/llcommon/llcallbacklist.h +++ b/indra/llcommon/llcallbacklist.h @@ -101,11 +101,14 @@ LLCallbackList::handle_t doOnIdleRepeating(bool_func_t callable) } /***************************************************************************** -* LLLater: callbacks at some future time +* LL::Timers: callbacks at some future time *****************************************************************************/ -class LLLater: public LLSingleton +namespace LL { - LLSINGLETON(LLLater); + +class Timers: public LLSingleton +{ + LLSINGLETON(Timers); using token_t = U32; @@ -113,11 +116,14 @@ class LLLater: public LLSingleton // a tuple, because we need to define the comparison operator. struct func_at { - nullary_func_t mFunc; + // callback to run when this timer fires + bool_func_t mFunc; + // key to look up metadata in mHandles token_t mToken; + // time at which this timer is supposed to fire LLDate::timestamp mTime; - func_at(const nullary_func_t& func, token_t token, LLDate::timestamp tm): + func_at(const bool_func_t& func, token_t token, LLDate::timestamp tm): mFunc(func), mToken(token), mTime(tm) @@ -146,7 +152,7 @@ public: class handle_t { private: - friend class LLLater; + friend class Timers; token_t token; public: handle_t(token_t token=0): token(token) {} @@ -156,33 +162,33 @@ public: }; // Call a given callable once at specified timestamp. - handle_t doAtTime(nullary_func_t callable, LLDate::timestamp time); + handle_t scheduleAt(nullary_func_t callable, LLDate::timestamp time); // Call a given callable once after specified interval. - handle_t doAfterInterval(nullary_func_t callable, F32 seconds); + handle_t scheduleAfter(nullary_func_t callable, F32 seconds); // Call a given callable every specified number of seconds, until it returns true. - handle_t doPeriodically(bool_func_t callable, F32 seconds); + handle_t scheduleRepeating(bool_func_t callable, F32 seconds); // test whether specified handle is still live bool isRunning(handle_t timer) const; // check remaining time - F32 getRemaining(handle_t timer) const; + F32 timeUntilCall(handle_t timer) const; - // Cancel a future timer set by doAtTime(), doAfterInterval(), doPeriodically(). - // Return true iff the handle corresponds to a live timer. + // Cancel a future timer set by scheduleAt(), scheduleAfter(), scheduleRepeating(). + // Return true if and only if the handle corresponds to a live timer. bool cancel(const handle_t& timer); // If we're canceling a non-const handle_t, also clear it so we need not // cancel again. bool cancel(handle_t& timer); - // Store a handle_t returned by doAtTime(), doAfterInterval() or - // doPeriodically() in a temp_handle_t to cancel() automatically on + // Store a handle_t returned by scheduleAt(), scheduleAfter() or + // scheduleRepeating() in a temp_handle_t to cancel() automatically on // destruction of the temp_handle_t. class temp_handle_t { public: - temp_handle_t() {} + temp_handle_t() = default; temp_handle_t(const handle_t& hdl): mHandle(hdl) {} temp_handle_t(const temp_handle_t&) = delete; temp_handle_t(temp_handle_t&&) = default; @@ -204,11 +210,11 @@ public: // temp_handle_t should be usable wherever handle_t is operator handle_t() const { return mHandle; } // If we're dealing with a non-const temp_handle_t, pass a reference - // to our handle_t member (e.g. to LLLater::cancel()). + // to our handle_t member (e.g. to Timers::cancel()). operator handle_t&() { return mHandle; } // For those in the know, provide a cancel() method of our own that - // avoids LLLater::instance() lookup when mHandle isn't live. + // avoids Timers::instance() lookup when mHandle isn't live. bool cancel() { if (! mHandle) @@ -217,7 +223,7 @@ public: } else { - return LLLater::instance().cancel(mHandle); + return Timers::instance().cancel(mHandle); } } @@ -231,44 +237,64 @@ public: }; private: + handle_t scheduleAtRepeating(bool_func_t callable, LLDate::timestamp time, F32 interval); + LLDate::timestamp now() const { return LLDate::now().secondsSinceEpoch(); } + // wrap a nullary_func_t with a bool_func_t that will only execute once + bool_func_t once(nullary_func_t callable) + { + return [callable] + { + callable(); + return true; + }; + } bool tick(); // NOTE: We don't lock our data members because it doesn't make sense to - // register cross-thread callbacks. If we start wanting to use them on + // register cross-thread callbacks. If we start wanting to use Timers on // threads other than the main thread, it would make more sense to make // our data members thread_local than to lock them. // the heap aka priority queue queue_t mQueue; - // handles we've returned that haven't yet canceled - using HandleMap = std::unordered_map< - token_t, - std::pair>; - HandleMap mHandles; + + // metadata about a given task + struct Metadata + { + // handle to mQueue entry + queue_t::handle_type mHandle; + // time at which this timer is supposed to fire + LLDate::timestamp mTime; + // interval at which this timer is supposed to fire repeatedly + F32 mInterval{ 0 }; + // mFunc is currently running: don't delete this entry + bool mRunning{ false }; + // cancel() was called while mFunc was running: deferred cancel + bool mCancel{ false }; + }; + + using MetaMap = std::unordered_map; + MetaMap mMeta; token_t mToken{ 0 }; // While mQueue is non-empty, register for regular callbacks. LLCallbackList::temp_handle_t mLive; - - struct Periodic; - - // internal implementation for doAtTime() - HandleMap::iterator doAtTime1(LLDate::timestamp time); - handle_t doAtTime2(nullary_func_t callable, HandleMap::iterator iter); }; +} // namespace LL + /*-------------------- legacy names in global namespace --------------------*/ // Call a given callable once after specified interval. inline -LLLater::handle_t doAfterInterval(nullary_func_t callable, F32 seconds) +LL::Timers::handle_t doAfterInterval(nullary_func_t callable, F32 seconds) { - return LLLater::instance().doAfterInterval(callable, seconds); + return LL::Timers::instance().scheduleAfter(callable, seconds); } // Call a given callable every specified number of seconds, until it returns true. inline -LLLater::handle_t doPeriodically(bool_func_t callable, F32 seconds) +LL::Timers::handle_t doPeriodically(bool_func_t callable, F32 seconds) { - return LLLater::instance().doPeriodically(callable, seconds); + return LL::Timers::instance().scheduleRepeating(callable, seconds); } #endif diff --git a/indra/llcommon/lleventfilter.cpp b/indra/llcommon/lleventfilter.cpp index e72ae7ad33..ad61e9298a 100644 --- a/indra/llcommon/lleventfilter.cpp +++ b/indra/llcommon/lleventfilter.cpp @@ -87,7 +87,7 @@ LLEventTimeout::LLEventTimeout(LLEventPump& source): void LLEventTimeout::actionAfter(F32 seconds, const Action& action) { - mTimer = LLLater::instance().doAfterInterval(action, seconds); + mTimer = LL::Timers::instance().scheduleAfter(action, seconds); } void LLEventTimeout::errorAfter(F32 seconds, const std::string& message) @@ -118,7 +118,7 @@ void LLEventTimeout::cancel() bool LLEventTimeout::running() const { - return LLLater::instance().isRunning(mTimer); + return LL::Timers::instance().isRunning(mTimer); } /***************************************************************************** @@ -277,17 +277,17 @@ F32 LLEventThrottle::getDelay() const void LLEventThrottle::alarmActionAfter(F32 interval, const LLEventTimeout::Action& action) { - mAlarm = LLLater::instance().doAfterInterval(action, interval); + mAlarm = LL::Timers::instance().scheduleAfter(action, interval); } bool LLEventThrottle::alarmRunning() const { - return LLLater::instance().isRunning(mAlarm); + return LL::Timers::instance().isRunning(mAlarm); } void LLEventThrottle::alarmCancel() { - LLLater::instance().cancel(mAlarm); + LL::Timers::instance().cancel(mAlarm); } void LLEventThrottle::timerSet(F32 interval) diff --git a/indra/llcommon/lleventfilter.h b/indra/llcommon/lleventfilter.h index 1deb6f0f4c..b39791c560 100644 --- a/indra/llcommon/lleventfilter.h +++ b/indra/llcommon/lleventfilter.h @@ -191,7 +191,7 @@ public: private: // Use a temp_handle_t so it's canceled on destruction. - LLLater::temp_handle_t mTimer; + LL::Timers::temp_handle_t mTimer; }; /** @@ -300,7 +300,7 @@ private: F32 mInterval; // use this to arrange a deferred flush() call - LLLater::handle_t mAlarm; + LL::Timers::handle_t mAlarm; }; /** diff --git a/indra/llcommon/lleventtimer.cpp b/indra/llcommon/lleventtimer.cpp index 0f8d1e636f..1d2da93683 100644 --- a/indra/llcommon/lleventtimer.cpp +++ b/indra/llcommon/lleventtimer.cpp @@ -49,20 +49,20 @@ LLEventTimer::~LLEventTimer() void LLEventTimer::start() { - mTimer = LLLater::instance().doPeriodically([this]{ return tick(); }, mPeriod); + mTimer = LL::Timers::instance().scheduleRepeating([this]{ return tick(); }, mPeriod); } void LLEventTimer::stop() { - LLLater::instance().cancel(mTimer); + LL::Timers::instance().cancel(mTimer); } bool LLEventTimer::isRunning() { - return LLLater::instance().isRunning(mTimer); + return LL::Timers::instance().isRunning(mTimer); } F32 LLEventTimer::getRemaining() { - return LLLater::instance().getRemaining(mTimer); + return LL::Timers::instance().timeUntilCall(mTimer); } diff --git a/indra/llcommon/lleventtimer.h b/indra/llcommon/lleventtimer.h index 05d8bc038d..a325c704e0 100644 --- a/indra/llcommon/lleventtimer.h +++ b/indra/llcommon/lleventtimer.h @@ -50,7 +50,7 @@ public: virtual bool tick() = 0; protected: - LLLater::temp_handle_t mTimer; + LL::Timers::temp_handle_t mTimer; F32 mPeriod; }; -- cgit v1.2.3 From b400f83deb068060f9dde5c0a2d6d1a259191ddd Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 8 May 2024 13:59:21 -0400 Subject: Fix llerror_test.cpp now that LL_ERRS() includes a stacktrace. --- indra/llcommon/llerror.cpp | 2 - indra/llcommon/tests/llerror_test.cpp | 247 ++++++++++++++++++++-------------- 2 files changed, 145 insertions(+), 104 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index ccdf3b60a8..b6285db073 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -44,8 +44,6 @@ # include #endif // !LL_WINDOWS #include -#define BOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED -#include #include "string.h" #include "llapp.h" diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp index b4cdbdc6bf..63d5bdbb70 100644 --- a/indra/llcommon/tests/llerror_test.cpp +++ b/indra/llcommon/tests/llerror_test.cpp @@ -112,10 +112,10 @@ namespace tut mMessages.push_back(message); } - int countMessages() { return (int) mMessages.size(); } + int countMessages() const { return (int) mMessages.size(); } void clearMessages() { mMessages.clear(); } - std::string message(int n) + std::string message(int n) const { std::ostringstream test_name; test_name << "testing message " << n << ", not enough messages"; @@ -124,6 +124,16 @@ namespace tut return mMessages[n]; } + void reportMessages() const + { + std::cerr << '\n'; + int n = 0; + for (const auto& msg : mMessages) + { + std::cerr << std::setw(2) << n++ << ": " << msg.substr(0, 100) << '\n'; + } + } + private: typedef std::vector MessageVector; MessageVector mMessages; @@ -134,6 +144,8 @@ namespace tut LLError::RecorderPtr mRecorder; LLError::SettingsStoragePtr mPriorErrorSettings; + auto recorder() { return std::dynamic_pointer_cast(mRecorder); } + ErrorTestData(): mRecorder(new TestRecorder()) { @@ -153,27 +165,32 @@ namespace tut int countMessages() { - return std::dynamic_pointer_cast(mRecorder)->countMessages(); + return recorder()->countMessages(); } void clearMessages() { - std::dynamic_pointer_cast(mRecorder)->clearMessages(); + recorder()->clearMessages(); } void setWantsTime(bool t) - { - std::dynamic_pointer_cast(mRecorder)->showTime(t); - } + { + recorder()->showTime(t); + } void setWantsMultiline(bool t) - { - std::dynamic_pointer_cast(mRecorder)->showMultiline(t); - } + { + recorder()->showMultiline(t); + } std::string message(int n) { - return std::dynamic_pointer_cast(mRecorder)->message(n); + return recorder()->message(n); + } + + void reportMessages() + { + recorder()->reportMessages(); } void ensure_message_count(int expectedCount) @@ -181,71 +198,87 @@ namespace tut ensure_equals("message count", countMessages(), expectedCount); } - std::string message_field(int msgnum, LogFieldIndex fieldnum) - { - std::ostringstream test_name; - test_name << "testing message " << msgnum << ", not enough messages"; - tut::ensure(test_name.str(), msgnum < countMessages()); - - std::string msg(message(msgnum)); - - std::string field_value; - - // find the start of the field; fields are separated by a single space - size_t scan = 0; - int on_field = 0; - while ( scan < msg.length() && on_field < fieldnum ) - { - // fields are delimited by one space - if ( ' ' == msg[scan] ) - { - if ( on_field < FUNCTION_FIELD ) - { - on_field++; - } - // except function, which may have embedded spaces so ends with " : " - else if ( ( on_field == FUNCTION_FIELD ) - && ( ':' == msg[scan+1] && ' ' == msg[scan+2] ) - ) - { - on_field++; - scan +=2; - } - } - scan++; - } - size_t start_field = scan; - size_t fieldlen = 0; - if ( fieldnum < FUNCTION_FIELD ) - { - fieldlen = msg.find(' ', start_field) - start_field; - } - else if ( fieldnum == FUNCTION_FIELD ) - { - fieldlen = msg.find(" : ", start_field) - start_field; - } - else if ( MSG_FIELD == fieldnum ) // no delimiter, just everything to the end - { - fieldlen = msg.length() - start_field; - } - - return msg.substr(start_field, fieldlen); - } - + std::string message_field(int msgnum, LogFieldIndex fieldnum) + { + std::ostringstream test_name; + test_name << "testing message " << msgnum << ", not enough messages"; + tut::ensure(test_name.str(), msgnum < countMessages()); + + std::string msg(message(msgnum)); + + std::string field_value; + + // find the start of the field; fields are separated by a single space + size_t scan = 0; + int on_field = 0; + while ( scan < msg.length() && on_field < fieldnum ) + { + // fields are delimited by one space + if ( ' ' == msg[scan] ) + { + if ( on_field < FUNCTION_FIELD ) + { + on_field++; + } + // except function, which may have embedded spaces so ends with " : " + else if (( on_field == FUNCTION_FIELD ) + && ( ':' == msg[scan+1] && ' ' == msg[scan+2] ) + ) + { + on_field++; + scan +=2; + } + } + scan++; + } + size_t start_field = scan; + size_t fieldlen = 0; + if ( fieldnum < FUNCTION_FIELD ) + { + fieldlen = msg.find(' ', start_field) - start_field; + } + else if ( fieldnum == FUNCTION_FIELD ) + { + fieldlen = msg.find(" : ", start_field) - start_field; + } + else if ( MSG_FIELD == fieldnum ) // no delimiter, just everything to the end + { + fieldlen = msg.length() - start_field; + } + + return msg.substr(start_field, fieldlen); + } + void ensure_message_field_equals(int msgnum, LogFieldIndex fieldnum, const std::string& expectedText) - { - std::ostringstream test_name; - test_name << "testing message " << msgnum << " field " << FieldName[fieldnum] << "\n message: \"" << message(msgnum) << "\"\n "; + { + std::ostringstream test_name; + test_name << "testing message " << msgnum << " field " << FieldName[fieldnum] << "\n message: \"" << message(msgnum).substr(0, 100) << "\"\n "; - ensure_equals(test_name.str(), message_field(msgnum, fieldnum), expectedText); - } + try + { + ensure_equals(test_name.str(), message_field(msgnum, fieldnum), expectedText); + } + catch (const failure&) + { + reportMessages(); + throw; + } + } void ensure_message_does_not_contain(int n, const std::string& expectedText) { std::ostringstream test_name; test_name << "testing message " << n; - ensure_does_not_contain(test_name.str(), message(n), expectedText); + try + { + ensure_does_not_contain(test_name.str(), message(n), expectedText); + } + catch (const failure&) + { + reportMessages(); + throw; + } } }; @@ -297,29 +330,33 @@ namespace tut ensure_message_field_equals(3, MSG_FIELD, "four"); ensure_message_field_equals(3, LEVEL_FIELD, "ERROR"); ensure_message_field_equals(3, TAGS_FIELD, "#WriteTag#"); - ensure_message_count(4); + // LL_ERRS() produces 2 recordMessage() calls + ensure_message_count(5); LLError::setDefaultLevel(LLError::LEVEL_INFO); writeSome(); - ensure_message_field_equals(4, MSG_FIELD, "two"); - ensure_message_field_equals(5, MSG_FIELD, "three"); - ensure_message_field_equals(6, MSG_FIELD, "four"); - ensure_message_count(7); + ensure_message_field_equals(5, MSG_FIELD, "two"); + ensure_message_field_equals(6, MSG_FIELD, "three"); + ensure_message_field_equals(7, MSG_FIELD, "four"); + // LL_ERRS() produces 2 recordMessage() calls + ensure_message_count(9); LLError::setDefaultLevel(LLError::LEVEL_WARN); writeSome(); - ensure_message_field_equals(7, MSG_FIELD, "three"); - ensure_message_field_equals(8, MSG_FIELD, "four"); - ensure_message_count(9); + ensure_message_field_equals(9, MSG_FIELD, "three"); + ensure_message_field_equals(10, MSG_FIELD, "four"); + // LL_ERRS() produces 2 recordMessage() calls + ensure_message_count(12); LLError::setDefaultLevel(LLError::LEVEL_ERROR); writeSome(); - ensure_message_field_equals(9, MSG_FIELD, "four"); - ensure_message_count(10); + ensure_message_field_equals(12, MSG_FIELD, "four"); + // LL_ERRS() produces 2 recordMessage() calls + ensure_message_count(14); LLError::setDefaultLevel(LLError::LEVEL_NONE); writeSome(); - ensure_message_count(10); + ensure_message_count(14); } template<> template<> @@ -331,7 +368,8 @@ namespace tut ensure_message_field_equals(1, LEVEL_FIELD, "INFO"); ensure_message_field_equals(2, LEVEL_FIELD, "WARNING"); ensure_message_field_equals(3, LEVEL_FIELD, "ERROR"); - ensure_message_count(4); + // LL_ERRS() produces 2 recordMessage() calls + ensure_message_count(5); } template<> template<> @@ -339,20 +377,20 @@ namespace tut // file abbreviation { std::string prev, abbreviateFile = __FILE__; - do - { - prev = abbreviateFile; - abbreviateFile = LLError::abbreviateFile(abbreviateFile); - // __FILE__ is assumed to end with - // indra/llcommon/tests/llerror_test.cpp. This test used to call - // abbreviateFile() exactly once, then check below whether it - // still contained the string 'indra'. That fails if the FIRST - // part of the pathname also contains indra! Certain developer - // machine images put local directory trees under - // /ngi-persist/indra, which is where we observe the problem. So - // now, keep calling abbreviateFile() until it returns its - // argument unchanged, THEN check. - } while (abbreviateFile != prev); + do + { + prev = abbreviateFile; + abbreviateFile = LLError::abbreviateFile(abbreviateFile); + // __FILE__ is assumed to end with + // indra/llcommon/tests/llerror_test.cpp. This test used to call + // abbreviateFile() exactly once, then check below whether it + // still contained the string 'indra'. That fails if the FIRST + // part of the pathname also contains indra! Certain developer + // machine images put local directory trees under + // /ngi-persist/indra, which is where we observe the problem. So + // now, keep calling abbreviateFile() until it returns its + // argument unchanged, THEN check. + } while (abbreviateFile != prev); ensure_ends_with("file name abbreviation", abbreviateFile, @@ -627,7 +665,8 @@ namespace tut ensure_message_field_equals(0, LOCATION_FIELD, location); ensure_message_field_equals(0, MSG_FIELD, "die"); - ensure_message_count(1); + // LL_ERRS() produces 2 recordMessage() calls + ensure_message_count(2); ensure("fatal callback called", fatalWasCalled); } @@ -751,10 +790,12 @@ namespace tut ensure_message_field_equals(0, MSG_FIELD, "aim west"); ensure_message_field_equals(1, MSG_FIELD, "ate eels"); - ensure_message_field_equals(2, MSG_FIELD, "buy iron"); - ensure_message_field_equals(3, MSG_FIELD, "bad word"); - ensure_message_field_equals(4, MSG_FIELD, "big easy"); - ensure_message_count(5); + // LL_ERRS() produces 2 recordMessage() calls + ensure_message_field_equals(3, MSG_FIELD, "buy iron"); + ensure_message_field_equals(4, MSG_FIELD, "bad word"); + ensure_message_field_equals(5, MSG_FIELD, "big easy"); + // LL_ERRS() produces 2 recordMessage() calls + ensure_message_count(7); } template<> template<> @@ -868,9 +909,11 @@ namespace tut TestBeta::doAll(); ensure_message_field_equals(3, MSG_FIELD, "aim west"); ensure_message_field_equals(4, MSG_FIELD, "ate eels"); - ensure_message_field_equals(5, MSG_FIELD, "bad word"); - ensure_message_field_equals(6, MSG_FIELD, "big easy"); - ensure_message_count(7); + // LL_ERRS() produces 2 recordMessage() calls + ensure_message_field_equals(6, MSG_FIELD, "bad word"); + ensure_message_field_equals(7, MSG_FIELD, "big easy"); + // LL_ERRS() produces 2 recordMessage() calls + ensure_message_count(9); } } -- cgit v1.2.3 From dc0b3aed4782e4e4835fd6b9d59d1d70b78be4a7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 8 May 2024 15:26:00 -0400 Subject: Bump up coroutine stack size: saw C00000FD test termination. --- indra/llcommon/llcoros.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index c13900f74a..8612f9353f 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -123,7 +123,7 @@ LLCoros::LLCoros(): // Previously we used // boost::context::guarded_stack_allocator::default_stacksize(); // empirically this is insufficient. - mStackSize(900*1024), + mStackSize(1024*1024), // mCurrent does NOT own the current CoroData instance -- it simply // points to it. So initialize it with a no-op deleter. mCurrent{ [](CoroData*){} } -- cgit v1.2.3