From 34e79c8f4e251200496651f9ae2b5126a6f7faa3 Mon Sep 17 00:00:00 2001 From: Dave Parks Date: Fri, 11 Mar 2022 10:21:08 -0600 Subject: SL-17005 WIP Simplify what feeds texture loading bias to only pay attention to available memory according to OS and GL driver, not (broken) internal accounting (breaks intel GPUs, compatibility pass incoming). --- indra/llimage/llimage.cpp | 5 ----- indra/llimage/llimage.h | 1 - 2 files changed, 6 deletions(-) (limited to 'indra/llimage') diff --git a/indra/llimage/llimage.cpp b/indra/llimage/llimage.cpp index 0fa027c9c3..3a3011d99b 100644 --- a/indra/llimage/llimage.cpp +++ b/indra/llimage/llimage.cpp @@ -798,7 +798,6 @@ U8* LLImageBase::allocateDataSize(S32 width, S32 height, S32 ncomponents, S32 si // LLImageRaw //--------------------------------------------------------------------------- -S32 LLImageRaw::sGlobalRawMemory = 0; S32 LLImageRaw::sRawImageCount = 0; LLImageRaw::LLImageRaw() @@ -847,16 +846,13 @@ LLImageRaw::~LLImageRaw() U8* LLImageRaw::allocateData(S32 size) { U8* res = LLImageBase::allocateData(size); - sGlobalRawMemory += getDataSize(); return res; } // virtual U8* LLImageRaw::reallocateData(S32 size) { - sGlobalRawMemory -= getDataSize(); U8* res = LLImageBase::reallocateData(size); - sGlobalRawMemory += getDataSize(); return res; } @@ -869,7 +865,6 @@ void LLImageRaw::releaseData() // virtual void LLImageRaw::deleteData() { - sGlobalRawMemory -= getDataSize(); LLImageBase::deleteData(); } diff --git a/indra/llimage/llimage.h b/indra/llimage/llimage.h index 7a588cfb03..460d5e0f5b 100644 --- a/indra/llimage/llimage.h +++ b/indra/llimage/llimage.h @@ -275,7 +275,6 @@ protected: void setDataAndSize(U8 *data, S32 width, S32 height, S8 components) ; public: - static S32 sGlobalRawMemory; static S32 sRawImageCount; private: -- cgit v1.2.3 From b6841d75c2f259c84d5ab6b012bd2ae37d985451 Mon Sep 17 00:00:00 2001 From: Dave Parks Date: Fri, 15 Apr 2022 19:02:07 -0500 Subject: SL-17219 WIP - Texture pipeline overhaul --- indra/llimage/llimagej2c.cpp | 2 ++ indra/llimage/llimageworker.cpp | 36 ++++++++++++------------------------ indra/llimage/llimageworker.h | 9 +++------ 3 files changed, 17 insertions(+), 30 deletions(-) (limited to 'indra/llimage') diff --git a/indra/llimage/llimagej2c.cpp b/indra/llimage/llimagej2c.cpp index e1809dbe59..8dba1641a6 100644 --- a/indra/llimage/llimagej2c.cpp +++ b/indra/llimage/llimagej2c.cpp @@ -146,6 +146,7 @@ bool LLImageJ2C::initEncode(LLImageRaw &raw_image, int blocks_size, int precinct bool LLImageJ2C::decode(LLImageRaw *raw_imagep, F32 decode_time) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; return decodeChannels(raw_imagep, decode_time, 0, 4); } @@ -153,6 +154,7 @@ bool LLImageJ2C::decode(LLImageRaw *raw_imagep, F32 decode_time) // Returns true to mean done, whether successful or not. bool LLImageJ2C::decodeChannels(LLImageRaw *raw_imagep, F32 decode_time, S32 first_channel, S32 max_channel_count ) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; LLTimer elapsed; bool res = true; diff --git a/indra/llimage/llimageworker.cpp b/indra/llimage/llimageworker.cpp index 0dbb744bcf..1aace5f3e8 100644 --- a/indra/llimage/llimageworker.cpp +++ b/indra/llimage/llimageworker.cpp @@ -49,33 +49,22 @@ LLImageDecodeThread::~LLImageDecodeThread() S32 LLImageDecodeThread::update(F32 max_time_ms) { LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; - LLMutexLock lock(mCreationMutex); - for (creation_list_t::iterator iter = mCreationList.begin(); - iter != mCreationList.end(); ++iter) - { - creation_info& info = *iter; - ImageRequest* req = new ImageRequest(info.handle, info.image, - info.priority, info.discard, info.needs_aux, - info.responder); - - bool res = addRequest(req); - if (!res) - { - LL_ERRS() << "request added after LLLFSThread::cleanupClass()" << LL_ENDL; - } - } - mCreationList.clear(); S32 res = LLQueuedThread::update(max_time_ms); return res; } LLImageDecodeThread::handle_t LLImageDecodeThread::decodeImage(LLImageFormatted* image, - U32 priority, S32 discard, BOOL needs_aux, Responder* responder) + S32 discard, BOOL needs_aux, Responder* responder) { LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; - LLMutexLock lock(mCreationMutex); handle_t handle = generateHandle(); - mCreationList.push_back(creation_info(handle, image, priority, discard, needs_aux, responder)); + + ImageRequest* req = new ImageRequest(handle, image, + discard, needs_aux, + responder); + + addRequest(req); + return handle; } @@ -84,8 +73,7 @@ LLImageDecodeThread::handle_t LLImageDecodeThread::decodeImage(LLImageFormatted* S32 LLImageDecodeThread::tut_size() { LLMutexLock lock(mCreationMutex); - S32 res = mCreationList.size(); - return res; + return 0; } LLImageDecodeThread::Responder::~Responder() @@ -95,9 +83,9 @@ LLImageDecodeThread::Responder::~Responder() //---------------------------------------------------------------------------- LLImageDecodeThread::ImageRequest::ImageRequest(handle_t handle, LLImageFormatted* image, - U32 priority, S32 discard, BOOL needs_aux, + S32 discard, BOOL needs_aux, LLImageDecodeThread::Responder* responder) - : LLQueuedThread::QueuedRequest(handle, priority, FLAG_AUTO_COMPLETE), + : LLQueuedThread::QueuedRequest(handle, FLAG_AUTO_COMPLETE), mFormattedImage(image), mDiscardLevel(discard), mNeedsAux(needs_aux), @@ -121,7 +109,7 @@ LLImageDecodeThread::ImageRequest::~ImageRequest() bool LLImageDecodeThread::ImageRequest::processRequest() { LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; - const F32 decode_time_slice = .1f; + const F32 decode_time_slice = 0.f; //disable time slicing bool done = true; if (!mDecodedRaw && mFormattedImage.notNull()) { diff --git a/indra/llimage/llimageworker.h b/indra/llimage/llimageworker.h index 1bfb0ddfd3..4619ddd6a2 100644 --- a/indra/llimage/llimageworker.h +++ b/indra/llimage/llimageworker.h @@ -49,7 +49,7 @@ public: public: ImageRequest(handle_t handle, LLImageFormatted* image, - U32 priority, S32 discard, BOOL needs_aux, + S32 discard, BOOL needs_aux, LLImageDecodeThread::Responder* responder); /*virtual*/ bool processRequest(); @@ -76,7 +76,7 @@ public: virtual ~LLImageDecodeThread(); handle_t decodeImage(LLImageFormatted* image, - U32 priority, S32 discard, BOOL needs_aux, + S32 discard, BOOL needs_aux, Responder* responder); S32 update(F32 max_time_ms); @@ -88,16 +88,13 @@ private: { handle_t handle; LLPointer image; - U32 priority; S32 discard; BOOL needs_aux; LLPointer responder; creation_info(handle_t h, LLImageFormatted* i, U32 p, S32 d, BOOL aux, Responder* r) - : handle(h), image(i), priority(p), discard(d), needs_aux(aux), responder(r) + : handle(h), image(i), discard(d), needs_aux(aux), responder(r) {} }; - typedef std::list creation_list_t; - creation_list_t mCreationList; LLMutex* mCreationMutex; }; -- cgit v1.2.3 From 41e32126d4b4b5d7bd810484a0f6c732f4367169 Mon Sep 17 00:00:00 2001 From: Dave Parks Date: Tue, 24 May 2022 17:48:41 -0500 Subject: SL-17484 Unit test build fix (remove priority parameters). --- indra/llimage/tests/llimageworker_test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/llimage') diff --git a/indra/llimage/tests/llimageworker_test.cpp b/indra/llimage/tests/llimageworker_test.cpp index 9011ac615c..462dde9fb8 100644 --- a/indra/llimage/tests/llimageworker_test.cpp +++ b/indra/llimage/tests/llimageworker_test.cpp @@ -141,7 +141,7 @@ namespace tut done = false; mRequest = new LLImageDecodeThread::ImageRequest(0, 0, - LLQueuedThread::PRIORITY_NORMAL, 0, FALSE, + 0, FALSE, new responder_test(&done)); } ~imagerequest_test() @@ -198,7 +198,7 @@ namespace tut ensure("LLImageDecodeThread: non threaded init state incorrect", mThread->tut_size() == 0); // Insert something in the queue bool done = false; - LLImageDecodeThread::handle_t decodeHandle = mThread->decodeImage(NULL, LLQueuedThread::PRIORITY_NORMAL, 0, FALSE, new responder_test(&done)); + LLImageDecodeThread::handle_t decodeHandle = mThread->decodeImage(NULL, 0, FALSE, new responder_test(&done)); // Verifies we got a valid handle ensure("LLImageDecodeThread: non threaded decodeImage(), returned handle is null", decodeHandle != 0); // Verifies that we do now have something in the queued list @@ -221,7 +221,7 @@ namespace tut ensure("LLImageDecodeThread: threaded init state incorrect", mThread->tut_size() == 0); // Insert something in the queue bool done = false; - LLImageDecodeThread::handle_t decodeHandle = mThread->decodeImage(NULL, LLQueuedThread::PRIORITY_NORMAL, 0, FALSE, new responder_test(&done)); + LLImageDecodeThread::handle_t decodeHandle = mThread->decodeImage(NULL, 0, FALSE, new responder_test(&done)); // Verifies we get back a valid handle ensure("LLImageDecodeThread: threaded decodeImage(), returned handle is null", decodeHandle != 0); // Wait a little so to simulate the main thread doing something on its main loop... -- cgit v1.2.3 From fc7b5549cb6092194d11b8d87600f21992305c1c Mon Sep 17 00:00:00 2001 From: Dave Parks Date: Tue, 31 May 2022 16:54:05 -0500 Subject: SL-17484 Fix for unit tests. Deprecate non-threaded LLQueuedThread and make lllfsthread threaded. --- indra/llimage/llimageworker.cpp | 8 -------- indra/llimage/llimageworker.h | 3 --- indra/llimage/tests/llimageworker_test.cpp | 25 ------------------------- 3 files changed, 36 deletions(-) (limited to 'indra/llimage') diff --git a/indra/llimage/llimageworker.cpp b/indra/llimage/llimageworker.cpp index 1aace5f3e8..d8503396d7 100644 --- a/indra/llimage/llimageworker.cpp +++ b/indra/llimage/llimageworker.cpp @@ -68,14 +68,6 @@ LLImageDecodeThread::handle_t LLImageDecodeThread::decodeImage(LLImageFormatted* return handle; } -// Used by unit test only -// Returns the size of the mutex guarded list as an indication of sanity -S32 LLImageDecodeThread::tut_size() -{ - LLMutexLock lock(mCreationMutex); - return 0; -} - LLImageDecodeThread::Responder::~Responder() { } diff --git a/indra/llimage/llimageworker.h b/indra/llimage/llimageworker.h index 4619ddd6a2..e0a94d2841 100644 --- a/indra/llimage/llimageworker.h +++ b/indra/llimage/llimageworker.h @@ -80,9 +80,6 @@ public: Responder* responder); S32 update(F32 max_time_ms); - // Used by unit tests to check the consistency of the thread instance - S32 tut_size(); - private: struct creation_info { diff --git a/indra/llimage/tests/llimageworker_test.cpp b/indra/llimage/tests/llimageworker_test.cpp index 462dde9fb8..57d922b1a1 100644 --- a/indra/llimage/tests/llimageworker_test.cpp +++ b/indra/llimage/tests/llimageworker_test.cpp @@ -190,35 +190,10 @@ namespace tut template<> template<> void imagedecodethread_object_t::test<1>() - { - // Test a *non threaded* instance of the class - mThread = new LLImageDecodeThread(false); - ensure("LLImageDecodeThread: non threaded constructor failed", mThread != NULL); - // Test that we start with an empty list right at creation - ensure("LLImageDecodeThread: non threaded init state incorrect", mThread->tut_size() == 0); - // Insert something in the queue - bool done = false; - LLImageDecodeThread::handle_t decodeHandle = mThread->decodeImage(NULL, 0, FALSE, new responder_test(&done)); - // Verifies we got a valid handle - ensure("LLImageDecodeThread: non threaded decodeImage(), returned handle is null", decodeHandle != 0); - // Verifies that we do now have something in the queued list - ensure("LLImageDecodeThread: non threaded decodeImage() insertion in threaded list failed", mThread->tut_size() == 1); - // Trigger queue handling "manually" (on a threaded instance, this is done on the thread loop) - S32 res = mThread->update(0); - // Verifies that we successfully handled the list - ensure("LLImageDecodeThread: non threaded update() list handling test failed", res == 0); - // Verifies that the list is now empty - ensure("LLImageDecodeThread: non threaded update() list emptying test failed", mThread->tut_size() == 0); - } - - template<> template<> - void imagedecodethread_object_t::test<2>() { // Test a *threaded* instance of the class mThread = new LLImageDecodeThread(true); ensure("LLImageDecodeThread: threaded constructor failed", mThread != NULL); - // Test that we start with an empty list right at creation - ensure("LLImageDecodeThread: threaded init state incorrect", mThread->tut_size() == 0); // Insert something in the queue bool done = false; LLImageDecodeThread::handle_t decodeHandle = mThread->decodeImage(NULL, 0, FALSE, new responder_test(&done)); -- cgit v1.2.3 From 609476e607b18d303afa5f5b5eeabeca84f95d16 Mon Sep 17 00:00:00 2001 From: Dave Parks Date: Wed, 1 Jun 2022 09:25:16 -0500 Subject: SL-17484 More unit test pruning. Fix for crash when deleting textures. --- indra/llimage/tests/llimageworker_test.cpp | 6 ------ 1 file changed, 6 deletions(-) (limited to 'indra/llimage') diff --git a/indra/llimage/tests/llimageworker_test.cpp b/indra/llimage/tests/llimageworker_test.cpp index 57d922b1a1..d36d35aba4 100644 --- a/indra/llimage/tests/llimageworker_test.cpp +++ b/indra/llimage/tests/llimageworker_test.cpp @@ -199,12 +199,6 @@ namespace tut LLImageDecodeThread::handle_t decodeHandle = mThread->decodeImage(NULL, 0, FALSE, new responder_test(&done)); // Verifies we get back a valid handle ensure("LLImageDecodeThread: threaded decodeImage(), returned handle is null", decodeHandle != 0); - // Wait a little so to simulate the main thread doing something on its main loop... - ms_sleep(500); // 500 milliseconds - // Verifies that the responder has *not* been called yet in the meantime - ensure("LLImageDecodeThread: responder creation failed", done == false); - // Ask the thread to update: that means tells the queue to check itself and creates work requests - mThread->update(1); // Wait till the thread has time to handle the work order (though it doesn't do much per work order...) const U32 INCREMENT_TIME = 500; // 500 milliseconds const U32 MAX_TIME = 20 * INCREMENT_TIME; // Do the loop 20 times max, i.e. wait 10 seconds but no more -- cgit v1.2.3 From 50dca86f64a167fe0db901310d04784b2f0dfa1f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 9 Jun 2022 12:06:23 -0400 Subject: 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(). --- indra/llimage/llimageworker.cpp | 99 ++++++++++++++++++++---------- indra/llimage/llimageworker.h | 61 +++++------------- indra/llimage/tests/llimageworker_test.cpp | 66 -------------------- 3 files changed, 81 insertions(+), 145 deletions(-) (limited to 'indra/llimage') 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 image, + S32 discard, BOOL needs_aux, + LLPointer responder); + virtual ~ImageRequest(); + + /*virtual*/ bool processRequest(); + /*virtual*/ void finishRequest(bool completed); + +private: + // input + LLPointer mFormattedImage; + S32 mDiscardLevel; + BOOL mNeedsAux; + // output + LLPointer mDecodedImageRaw; + LLPointer mDecodedImageAux; + BOOL mDecodedRaw; + BOOL mDecodedAux; + LLPointer 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 image, + S32 discard, BOOL needs_aux, LLPointer 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 image, + S32 discard, BOOL needs_aux, + LLPointer 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 mFormattedImage; - S32 mDiscardLevel; - BOOL mNeedsAux; - // output - LLPointer mDecodedImageRaw; - LLPointer mDecodedImageAux; - BOOL mDecodedRaw; - BOOL mDecodedAux; - LLPointer 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 image, S32 discard, BOOL needs_aux, - Responder* responder); + LLPointer responder); + S32 getPending(); S32 update(F32 max_time_ms); + void shutdown(); private: - struct creation_info - { - handle_t handle; - LLPointer image; - S32 discard; - BOOL needs_aux; - LLPointer 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 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_t; typedef imagedecodethread_t::object imagedecodethread_object_t; tut::imagedecodethread_t tut_imagedecodethread("LLImageDecodeThread"); - typedef test_group 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"); - } - } } -- cgit v1.2.3 From 3b043d90b60eca17dfcc014b21c4c8fc5b432384 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 13 Jun 2022 15:18:14 -0400 Subject: SL-17483: Per review feedback, avoid LLPointer refcount twiddling when passing LLPointers into functions. Only increment the refcount when storing in new ImageRequest. --- indra/llimage/llimageworker.cpp | 17 +++++++++++------ indra/llimage/llimageworker.h | 4 ++-- 2 files changed, 13 insertions(+), 8 deletions(-) (limited to 'indra/llimage') diff --git a/indra/llimage/llimageworker.cpp b/indra/llimage/llimageworker.cpp index 3c21499673..0093958e6d 100644 --- a/indra/llimage/llimageworker.cpp +++ b/indra/llimage/llimageworker.cpp @@ -34,15 +34,17 @@ class ImageRequest { public: - ImageRequest(LLPointer image, + ImageRequest(const LLPointer& image, S32 discard, BOOL needs_aux, - LLPointer responder); + const LLPointer& responder); virtual ~ImageRequest(); /*virtual*/ bool processRequest(); /*virtual*/ void finishRequest(bool completed); private: + // LLPointers stored in ImageRequest MUST be LLPointer instances rather + // than references: we need to increment the refcount when storing these. // input LLPointer mFormattedImage; S32 mDiscardLevel; @@ -82,8 +84,11 @@ S32 LLImageDecodeThread::getPending() return mThreadPool->getQueue().size(); } -LLImageDecodeThread::handle_t LLImageDecodeThread::decodeImage(LLPointer image, - S32 discard, BOOL needs_aux, LLPointer responder) +LLImageDecodeThread::handle_t LLImageDecodeThread::decodeImage( + const LLPointer& image, + S32 discard, + BOOL needs_aux, + const LLPointer& responder) { LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; @@ -113,9 +118,9 @@ LLImageDecodeThread::Responder::~Responder() //---------------------------------------------------------------------------- -ImageRequest::ImageRequest(LLPointer image, +ImageRequest::ImageRequest(const LLPointer& image, S32 discard, BOOL needs_aux, - LLPointer responder) + const LLPointer& responder) : mFormattedImage(image), mDiscardLevel(discard), mNeedsAux(needs_aux), diff --git a/indra/llimage/llimageworker.h b/indra/llimage/llimageworker.h index 6a0b2b4681..18398d9ae2 100644 --- a/indra/llimage/llimageworker.h +++ b/indra/llimage/llimageworker.h @@ -52,9 +52,9 @@ public: // meant to resemble LLQueuedThread::handle_t typedef U32 handle_t; - handle_t decodeImage(LLPointer image, + handle_t decodeImage(const LLPointer& image, S32 discard, BOOL needs_aux, - LLPointer responder); + const LLPointer& responder); S32 getPending(); S32 update(F32 max_time_ms); void shutdown(); -- cgit v1.2.3