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/llimageworker.cpp | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) (limited to 'indra/llimage/llimageworker.cpp') 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()) { -- 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 -------- 1 file changed, 8 deletions(-) (limited to 'indra/llimage/llimageworker.cpp') 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() { } -- 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 +++++++++++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 34 deletions(-) (limited to 'indra/llimage/llimageworker.cpp') 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(); -} -- 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 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'indra/llimage/llimageworker.cpp') 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), -- cgit v1.2.3