diff options
author | Nat Goodspeed <nat@lindenlab.com> | 2022-06-09 12:06:23 -0400 |
---|---|---|
committer | Nat Goodspeed <nat@lindenlab.com> | 2022-06-09 12:06:23 -0400 |
commit | 50dca86f64a167fe0db901310d04784b2f0dfa1f (patch) | |
tree | 6d39646a8506d219fe67b9ba6e41aaa0af5e1b70 /indra/llimage | |
parent | 0bf91fc141c988d61bc5c593766981e71454d6fd (diff) |
SL-17483: Recast LLImageDecodeThread as a facade for ThreadPool.
Remove all references to LLQueuedThread (but emulate a couple bits of its API
such as handle_t and getPending()).
Migrate ImageRequest into llimageworker.cpp. It has never been part of
LLImageDecodeThread's public API. Remove ImageRequest tests.
Remove all references to LLImageDecodeThread::pause(). The idea of pausing
another thread is bizarre to me, and LLThreadPool has no such operation. Nor
does it have an abortRequest().
Diffstat (limited to 'indra/llimage')
-rw-r--r-- | indra/llimage/llimageworker.cpp | 99 | ||||
-rw-r--r-- | indra/llimage/llimageworker.h | 61 | ||||
-rw-r--r-- | indra/llimage/tests/llimageworker_test.cpp | 66 |
3 files changed, 81 insertions, 145 deletions
diff --git a/indra/llimage/llimageworker.cpp b/indra/llimage/llimageworker.cpp index d8503396d7..3c21499673 100644 --- a/indra/llimage/llimageworker.cpp +++ b/indra/llimage/llimageworker.cpp @@ -28,44 +28,83 @@ #include "llimageworker.h" #include "llimagedxt.h" +#include "threadpool.h" + +/*--------------------------------------------------------------------------*/ +class ImageRequest +{ +public: + ImageRequest(LLPointer<LLImageFormatted> image, + S32 discard, BOOL needs_aux, + LLPointer<LLImageDecodeThread::Responder> responder); + virtual ~ImageRequest(); + + /*virtual*/ bool processRequest(); + /*virtual*/ void finishRequest(bool completed); + +private: + // input + LLPointer<LLImageFormatted> mFormattedImage; + S32 mDiscardLevel; + BOOL mNeedsAux; + // output + LLPointer<LLImageRaw> mDecodedImageRaw; + LLPointer<LLImageRaw> mDecodedImageAux; + BOOL mDecodedRaw; + BOOL mDecodedAux; + LLPointer<LLImageDecodeThread::Responder> mResponder; +}; + //---------------------------------------------------------------------------- // MAIN THREAD -LLImageDecodeThread::LLImageDecodeThread(bool threaded) - : LLQueuedThread("imagedecode", threaded) +LLImageDecodeThread::LLImageDecodeThread(bool /*threaded*/) { - mCreationMutex = new LLMutex(); + mThreadPool.reset(new LL::ThreadPool("ImageDecode", 8)); + mThreadPool->start(); } //virtual LLImageDecodeThread::~LLImageDecodeThread() -{ - delete mCreationMutex ; -} +{} // MAIN THREAD // virtual S32 LLImageDecodeThread::update(F32 max_time_ms) { LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; - S32 res = LLQueuedThread::update(max_time_ms); - return res; + return getPending(); } -LLImageDecodeThread::handle_t LLImageDecodeThread::decodeImage(LLImageFormatted* image, - S32 discard, BOOL needs_aux, Responder* responder) +S32 LLImageDecodeThread::getPending() { - LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; - handle_t handle = generateHandle(); + return mThreadPool->getQueue().size(); +} - ImageRequest* req = new ImageRequest(handle, image, - discard, needs_aux, - responder); +LLImageDecodeThread::handle_t LLImageDecodeThread::decodeImage(LLPointer<LLImageFormatted> image, + S32 discard, BOOL needs_aux, LLPointer<LLImageDecodeThread::Responder> responder) +{ + LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; - addRequest(req); + // Instantiate the ImageRequest right in the lambda, why not? + mThreadPool->getQueue().post( + [req = ImageRequest(image, discard, needs_aux, responder)] + () mutable + { + auto done = req.processRequest(); + req.finishRequest(done); + }); + + // It's important to our consumer (LLTextureFetchWorker) that we return a + // nonzero handle. It is NOT important that the nonzero handle be unique: + // nothing is ever done with it except to compare it to zero, or zero it. + return 17; +} - return handle; +void LLImageDecodeThread::shutdown() +{ + mThreadPool->close(); } LLImageDecodeThread::Responder::~Responder() @@ -74,11 +113,10 @@ LLImageDecodeThread::Responder::~Responder() //---------------------------------------------------------------------------- -LLImageDecodeThread::ImageRequest::ImageRequest(handle_t handle, LLImageFormatted* image, - S32 discard, BOOL needs_aux, - LLImageDecodeThread::Responder* responder) - : LLQueuedThread::QueuedRequest(handle, FLAG_AUTO_COMPLETE), - mFormattedImage(image), +ImageRequest::ImageRequest(LLPointer<LLImageFormatted> image, + S32 discard, BOOL needs_aux, + LLPointer<LLImageDecodeThread::Responder> responder) + : mFormattedImage(image), mDiscardLevel(discard), mNeedsAux(needs_aux), mDecodedRaw(FALSE), @@ -87,7 +125,7 @@ LLImageDecodeThread::ImageRequest::ImageRequest(handle_t handle, LLImageFormatte { } -LLImageDecodeThread::ImageRequest::~ImageRequest() +ImageRequest::~ImageRequest() { mDecodedImageRaw = NULL; mDecodedImageAux = NULL; @@ -98,7 +136,7 @@ LLImageDecodeThread::ImageRequest::~ImageRequest() // Returns true when done, whether or not decode was successful. -bool LLImageDecodeThread::ImageRequest::processRequest() +bool ImageRequest::processRequest() { LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; const F32 decode_time_slice = 0.f; //disable time slicing @@ -125,7 +163,7 @@ bool LLImageDecodeThread::ImageRequest::processRequest() mFormattedImage->getHeight(), mFormattedImage->getComponents()); } - done = mFormattedImage->decode(mDecodedImageRaw, decode_time_slice); // 1ms + done = mFormattedImage->decode(mDecodedImageRaw, decode_time_slice); // some decoders are removing data when task is complete and there were errors mDecodedRaw = done && mDecodedImageRaw->getData(); } @@ -138,14 +176,14 @@ bool LLImageDecodeThread::ImageRequest::processRequest() mFormattedImage->getHeight(), 1); } - done = mFormattedImage->decodeChannels(mDecodedImageAux, decode_time_slice, 4, 4); // 1ms + done = mFormattedImage->decodeChannels(mDecodedImageAux, decode_time_slice, 4, 4); mDecodedAux = done && mDecodedImageAux->getData(); } return done; } -void LLImageDecodeThread::ImageRequest::finishRequest(bool completed) +void ImageRequest::finishRequest(bool completed) { LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; if (mResponder.notNull()) @@ -155,10 +193,3 @@ void LLImageDecodeThread::ImageRequest::finishRequest(bool completed) } // Will automatically be deleted } - -// Used by unit test only -// Checks that a responder exists for this instance so that something can happen when completion is reached -bool LLImageDecodeThread::ImageRequest::tut_isOK() -{ - return mResponder.notNull(); -} diff --git a/indra/llimage/llimageworker.h b/indra/llimage/llimageworker.h index e0a94d2841..6a0b2b4681 100644 --- a/indra/llimage/llimageworker.h +++ b/indra/llimage/llimageworker.h @@ -29,9 +29,13 @@ #include "llimage.h" #include "llpointer.h" -#include "llworkerthread.h" -class LLImageDecodeThread : public LLQueuedThread +namespace LL +{ + class ThreadPool; +} // namespace LL + +class LLImageDecodeThread { public: class Responder : public LLThreadSafeRefCount @@ -42,57 +46,24 @@ public: virtual void completed(bool success, LLImageRaw* raw, LLImageRaw* aux) = 0; }; - class ImageRequest : public LLQueuedThread::QueuedRequest - { - protected: - virtual ~ImageRequest(); // use deleteRequest() - - public: - ImageRequest(handle_t handle, LLImageFormatted* image, - S32 discard, BOOL needs_aux, - LLImageDecodeThread::Responder* responder); - - /*virtual*/ bool processRequest(); - /*virtual*/ void finishRequest(bool completed); - - // Used by unit tests to check the consitency of the request instance - bool tut_isOK(); - - private: - // input - LLPointer<LLImageFormatted> mFormattedImage; - S32 mDiscardLevel; - BOOL mNeedsAux; - // output - LLPointer<LLImageRaw> mDecodedImageRaw; - LLPointer<LLImageRaw> mDecodedImageAux; - BOOL mDecodedRaw; - BOOL mDecodedAux; - LLPointer<LLImageDecodeThread::Responder> mResponder; - }; - public: LLImageDecodeThread(bool threaded = true); virtual ~LLImageDecodeThread(); - handle_t decodeImage(LLImageFormatted* image, + // meant to resemble LLQueuedThread::handle_t + typedef U32 handle_t; + handle_t decodeImage(LLPointer<LLImageFormatted> image, S32 discard, BOOL needs_aux, - Responder* responder); + LLPointer<Responder> responder); + S32 getPending(); S32 update(F32 max_time_ms); + void shutdown(); private: - struct creation_info - { - handle_t handle; - LLPointer<LLImageFormatted> image; - S32 discard; - BOOL needs_aux; - LLPointer<Responder> responder; - creation_info(handle_t h, LLImageFormatted* i, U32 p, S32 d, BOOL aux, Responder* r) - : handle(h), image(i), discard(d), needs_aux(aux), responder(r) - {} - }; - LLMutex* mCreationMutex; + // As of SL-17483, LLImageDecodeThread is no longer itself an + // LLQueuedThread - instead this is the API by which we submit work to the + // "ImageDecode" ThreadPool. + std::unique_ptr<LL::ThreadPool> mThreadPool; }; #endif diff --git a/indra/llimage/tests/llimageworker_test.cpp b/indra/llimage/tests/llimageworker_test.cpp index d36d35aba4..0a97b739b0 100644 --- a/indra/llimage/tests/llimageworker_test.cpp +++ b/indra/llimage/tests/llimageworker_test.cpp @@ -125,42 +125,11 @@ namespace tut } }; - // Test wrapper declaration : image worker - // Note: this class is not meant to be instantiated outside an LLImageDecodeThread instance - // but it's not a bad idea to get its public API a good shake as part of a thorough unit test set. - // Some gotcha with the destructor though (see below). - struct imagerequest_test - { - // Instance to be tested - LLImageDecodeThread::ImageRequest* mRequest; - bool done; - - // Constructor and destructor of the test wrapper - imagerequest_test() - { - done = false; - - mRequest = new LLImageDecodeThread::ImageRequest(0, 0, - 0, FALSE, - new responder_test(&done)); - } - ~imagerequest_test() - { - // We should delete the object *but*, because its destructor is protected, that cannot be - // done from outside an LLImageDecodeThread instance... So we leak memory here... It's fine... - //delete mRequest; - } - }; - // Tut templating thingamagic: test group, object and test instance typedef test_group<imagedecodethread_test> imagedecodethread_t; typedef imagedecodethread_t::object imagedecodethread_object_t; tut::imagedecodethread_t tut_imagedecodethread("LLImageDecodeThread"); - typedef test_group<imagerequest_test> imagerequest_t; - typedef imagerequest_t::object imagerequest_object_t; - tut::imagerequest_t tut_imagerequest("LLImageRequest"); - // --------------------------------------------------------------------------------------- // Test functions // Notes: @@ -172,21 +141,6 @@ namespace tut // --------------------------------------------------------------------------------------- // Test the LLImageDecodeThread interface // --------------------------------------------------------------------------------------- - // - // Note on Unit Testing Queued Thread Classes - // - // Since methods on such a class are called on a separate loop and that we can't insert tut - // ensure() calls in there, we exercise the class with 2 sets of tests: - // - 1: Test as a single threaded instance: We declare the class but ask for no thread - // to be spawned (easy with LLThreads since there's a boolean argument on the constructor - // just for that). We can then unit test each public method like we do on a normal class. - // - 2: Test as a threaded instance: We let the thread launch and check that its external - // behavior is as expected (i.e. it runs, can accept a work order and processes - // it). Typically though there's no guarantee that this exercises all the methods of the - // class which is why we also need the previous "non threaded" set of unit tests for - // complete coverage. - // - // --------------------------------------------------------------------------------------- template<> template<> void imagedecodethread_object_t::test<1>() @@ -211,24 +165,4 @@ namespace tut // Verifies that the responder has now been called ensure("LLImageDecodeThread: threaded work unit not processed", done == true); } - - // --------------------------------------------------------------------------------------- - // Test the LLImageDecodeThread::ImageRequest interface - // --------------------------------------------------------------------------------------- - - template<> template<> - void imagerequest_object_t::test<1>() - { - // Test that we start with a correct request at creation - ensure("LLImageDecodeThread::ImageRequest::ImageRequest() constructor test failed", mRequest->tut_isOK()); - bool res = mRequest->processRequest(); - // Verifies that we processed the request successfully - ensure("LLImageDecodeThread::ImageRequest::processRequest() processing request test failed", res == true); - // Check that we can call the finishing call safely - try { - mRequest->finishRequest(false); - } catch (...) { - fail("LLImageDecodeThread::ImageRequest::finishRequest() test failed"); - } - } } |