From 5ce70325ce207e31317346f1ed1405a21259e6dd Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 5 May 2023 09:52:22 -0400 Subject: [PATCH 01/12] DRTVWR-559: Hard tabs considered harmful --- indra/cmake/LLAddBuildTest.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/indra/cmake/LLAddBuildTest.cmake b/indra/cmake/LLAddBuildTest.cmake index 405128d661..2172b56da2 100644 --- a/indra/cmake/LLAddBuildTest.cmake +++ b/indra/cmake/LLAddBuildTest.cmake @@ -129,7 +129,7 @@ MACRO(LL_ADD_PROJECT_UNIT_TESTS project sources) if (DARWIN) # test binaries always need to be signed for local development set_target_properties(PROJECT_${project}_TEST_${name} - PROPERTIES + PROPERTIES XCODE_ATTRIBUTE_CODE_SIGN_IDENTITY "-") endif () @@ -232,7 +232,7 @@ FUNCTION(LL_ADD_INTEGRATION_TEST # test binaries always need to be signed for local development set_target_properties(INTEGRATION_TEST_${testname} PROPERTIES - XCODE_ATTRIBUTE_CODE_SIGN_IDENTITY "-") + XCODE_ATTRIBUTE_CODE_SIGN_IDENTITY "-") endif () # Add link deps to the executable From 307d31116dc4b04893652e859b86bfdd00d19c78 Mon Sep 17 00:00:00 2001 From: Cosmic Linden Date: Thu, 4 May 2023 18:27:08 -0700 Subject: [PATCH 02/12] SL-19644: De-virtualize pushBatch Bump still uses its own pushBumpBatch function - OK, works the same as before. --- indra/newview/lldrawpool.h | 13 ++++++++----- indra/newview/lldrawpoolbump.cpp | 25 +++++++++++++------------ indra/newview/lldrawpoolbump.h | 1 - 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/indra/newview/lldrawpool.h b/indra/newview/lldrawpool.h index 09c95a1705..eef19199b9 100644 --- a/indra/newview/lldrawpool.h +++ b/indra/newview/lldrawpool.h @@ -349,15 +349,18 @@ public: void resetDrawOrders() { } static void applyModelMatrix(const LLDrawInfo& params); - virtual void pushBatches(U32 type, bool texture = true, bool batch_textures = false); - virtual void pushRiggedBatches(U32 type, bool texture = true, bool batch_textures = false); + // Use before a non-GLTF batch if it is interleaved with GLTF batches that share the same shader + static void resetGLTFTextureTransform(); + void pushBatches(U32 type, bool texture = true, bool batch_textures = false); + void pushRiggedBatches(U32 type, bool texture = true, bool batch_textures = false); void pushGLTFBatches(U32 type); void pushGLTFBatch(LLDrawInfo& params); void pushRiggedGLTFBatches(U32 type); void pushRiggedGLTFBatch(LLDrawInfo& params, LLVOAvatar*& lastAvatar, U64& lastMeshId); - virtual void pushMaskBatches(U32 type, bool texture = true, bool batch_textures = false); - virtual void pushRiggedMaskBatches(U32 type, bool texture = true, bool batch_textures = false); - virtual void pushBatch(LLDrawInfo& params, bool texture, bool batch_textures = false); + void pushMaskBatches(U32 type, bool texture = true, bool batch_textures = false); + void pushRiggedMaskBatches(U32 type, bool texture = true, bool batch_textures = false); + void pushBatch(LLDrawInfo& params, bool texture, bool batch_textures = false); + void pushBumpBatch(LLDrawInfo& params, bool texture, bool batch_textures = false); static bool uploadMatrixPalette(LLDrawInfo& params); static bool uploadMatrixPalette(LLVOAvatar* avatar, LLMeshSkinInfo* skinInfo); virtual void renderGroup(LLSpatialGroup* group, U32 type, bool texture = true); diff --git a/indra/newview/lldrawpoolbump.cpp b/indra/newview/lldrawpoolbump.cpp index a548740ec4..bd0b05fb33 100644 --- a/indra/newview/lldrawpoolbump.cpp +++ b/indra/newview/lldrawpoolbump.cpp @@ -78,6 +78,7 @@ static LLGLSLShader* shader = NULL; static S32 cube_channel = -1; static S32 diffuse_channel = -1; static S32 bump_channel = -1; +static BOOL shiny = FALSE; // Enabled after changing LLViewerTexture::mNeedsCreateTexture to an // LLAtomicBool; this should work just fine, now. HB @@ -198,7 +199,7 @@ void LLStandardBumpmap::destroyGL() LLDrawPoolBump::LLDrawPoolBump() : LLRenderPass(LLDrawPool::POOL_BUMP) { - mShiny = FALSE; + shiny = FALSE; } @@ -347,7 +348,7 @@ void LLDrawPoolBump::beginFullbrightShiny() diffuse_channel = 0; } - mShiny = TRUE; + shiny = TRUE; } void LLDrawPoolBump::renderFullbrightShiny() @@ -399,7 +400,7 @@ void LLDrawPoolBump::endFullbrightShiny() diffuse_channel = -1; cube_channel = 0; - mShiny = FALSE; + shiny = FALSE; } void LLDrawPoolBump::renderGroup(LLSpatialGroup* group, U32 type, bool texture = true) @@ -551,7 +552,7 @@ void LLDrawPoolBump::renderDeferred(S32 pass) { LL_PROFILE_ZONE_SCOPED_CATEGORY_DRAWPOOL; //LL_RECORD_BLOCK_TIME(FTM_RENDER_BUMP); - mShiny = TRUE; + shiny = TRUE; for (int i = 0; i < 2; ++i) { bool rigged = i == 1; @@ -598,11 +599,11 @@ void LLDrawPoolBump::renderDeferred(S32 pass) avatar = params.mAvatar; skin = params.mSkinInfo->mHash; } - pushBatch(params, true, false); + pushBumpBatch(params, true, false); } else { - pushBatch(params, true, false); + pushBumpBatch(params, true, false); } } @@ -612,7 +613,7 @@ void LLDrawPoolBump::renderDeferred(S32 pass) gGL.getTexUnit(0)->activate(); } - mShiny = FALSE; + shiny = FALSE; } @@ -1249,12 +1250,12 @@ void LLDrawPoolBump::pushBumpBatches(U32 type) } } } - pushBatch(params, false); + pushBumpBatch(params, false); } } } -void LLDrawPoolBump::pushBatch(LLDrawInfo& params, bool texture, bool batch_textures) +void LLRenderPass::pushBumpBatch(LLDrawInfo& params, bool texture, bool batch_textures) { LL_PROFILE_ZONE_SCOPED_CATEGORY_DRAWPOOL; applyModelMatrix(params); @@ -1275,7 +1276,7 @@ void LLDrawPoolBump::pushBatch(LLDrawInfo& params, bool texture, bool batch_text { //not batching textures or batch has only 1 texture -- might need a texture matrix if (params.mTextureMatrix) { - if (mShiny) + if (shiny) { gGL.getTexUnit(0)->activate(); gGL.matrixMode(LLRender::MM_TEXTURE); @@ -1294,7 +1295,7 @@ void LLDrawPoolBump::pushBatch(LLDrawInfo& params, bool texture, bool batch_text tex_setup = true; } - if (mShiny && mShaderLevel > 1 && texture) + if (shiny && mShaderLevel > 1 && texture) { if (params.mTexture.notNull()) { @@ -1312,7 +1313,7 @@ void LLDrawPoolBump::pushBatch(LLDrawInfo& params, bool texture, bool batch_text if (tex_setup) { - if (mShiny) + if (shiny) { gGL.getTexUnit(0)->activate(); } diff --git a/indra/newview/lldrawpoolbump.h b/indra/newview/lldrawpoolbump.h index 840af0c99d..b1fe454c72 100644 --- a/indra/newview/lldrawpoolbump.h +++ b/indra/newview/lldrawpoolbump.h @@ -53,7 +53,6 @@ public: LLDrawPoolBump(); /*virtual*/ void prerender() override; - void pushBatch(LLDrawInfo& params, bool texture, bool batch_textures = false) override; void pushBumpBatches(U32 type); void renderGroup(LLSpatialGroup* group, U32 type, bool texture) override; From 6954aafb5d37060cbbe72d311333504a80c54964 Mon Sep 17 00:00:00 2001 From: Rye Mutt Date: Fri, 5 May 2023 17:20:48 -0400 Subject: [PATCH 03/12] Fix uncaught LLThreadSafeQueueInterrupt during ImageWorker threadpool shutdown --- indra/llimage/llimageworker.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/indra/llimage/llimageworker.cpp b/indra/llimage/llimageworker.cpp index 0093958e6d..9358a0ae2c 100644 --- a/indra/llimage/llimageworker.cpp +++ b/indra/llimage/llimageworker.cpp @@ -92,14 +92,21 @@ LLImageDecodeThread::handle_t LLImageDecodeThread::decodeImage( { LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; - // 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); - }); + try + { + // 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); + }); + } + catch (const LLThreadSafeQueueInterrupt&) + { + LL_DEBUGS() << "Tried to start decoding on shutdown" << LL_ENDL; + } // It's important to our consumer (LLTextureFetchWorker) that we return a // nonzero handle. It is NOT important that the nonzero handle be unique: From 855cae27ccde4896509e3e22c68c441d6404ccfb Mon Sep 17 00:00:00 2001 From: Rye Mutt Date: Fri, 5 May 2023 17:23:29 -0400 Subject: [PATCH 04/12] Fix LLThreadSafeQueueInterrupt in WorkQueueBase::postTo during shutdown by catching and returning false --- indra/llcommon/workqueue.h | 68 +++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/indra/llcommon/workqueue.h b/indra/llcommon/workqueue.h index 5461ce6c23..5b704e7198 100644 --- a/indra/llcommon/workqueue.h +++ b/indra/llcommon/workqueue.h @@ -516,37 +516,45 @@ namespace LL // Here we believe target WorkQueue still exists. Post to it a // lambda that packages our callable, our callback and a weak_ptr // to this originating WorkQueue. - tptr->post( - [reply = super::getWeak(), - callable = std::move(callable), - callback = std::move(callback)] - () mutable - { - // Use postMaybe() below in case this originating WorkQueue - // has been closed or destroyed. Remember, the outer lambda is - // now running on a thread servicing the target WorkQueue, and - // real time has elapsed since postTo()'s tptr->post() call. - try + try + { + tptr->post( + [reply = super::getWeak(), + callable = std::move(callable), + callback = std::move(callback)] + () mutable { - // Make a reply lambda to repost to THIS WorkQueue. - // Delegate to makeReplyLambda() so we can partially - // specialize on void return. - postMaybe(reply, makeReplyLambda(std::move(callable), std::move(callback))); - } - catch (...) - { - // Either variant of makeReplyLambda() is responsible for - // calling the caller's callable. If that throws, return - // the exception to the originating thread. - postMaybe( - reply, - // Bind the current exception to transport back to the - // originating WorkQueue. Once there, rethrow it. - [exc = std::current_exception()](){ std::rethrow_exception(exc); }); - } - }, - // if caller passed a TimePoint, pass it along to post() - std::forward(args)...); + // Use postMaybe() below in case this originating WorkQueue + // has been closed or destroyed. Remember, the outer lambda is + // now running on a thread servicing the target WorkQueue, and + // real time has elapsed since postTo()'s tptr->post() call. + try + { + // Make a reply lambda to repost to THIS WorkQueue. + // Delegate to makeReplyLambda() so we can partially + // specialize on void return. + postMaybe(reply, makeReplyLambda(std::move(callable), std::move(callback))); + } + catch (...) + { + // Either variant of makeReplyLambda() is responsible for + // calling the caller's callable. If that throws, return + // the exception to the originating thread. + postMaybe( + reply, + // Bind the current exception to transport back to the + // originating WorkQueue. Once there, rethrow it. + [exc = std::current_exception()](){ std::rethrow_exception(exc); }); + } + }, + // if caller passed a TimePoint, pass it along to post() + std::forward(args)...); + } + catch (const Closed&) + { + // target WorkQueue still exists, but is Closed + return false; + } // looks like we were able to post() return true; From 6d0d9b8e549c2bc600e6bf416d4614edc55e35c0 Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Mon, 8 May 2023 18:28:33 +0300 Subject: [PATCH 05/12] SL-19690 Updated contribution doc --- doc/contributions.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/contributions.txt b/doc/contributions.txt index 2f8641cab8..c56c4c1303 100755 --- a/doc/contributions.txt +++ b/doc/contributions.txt @@ -1404,6 +1404,7 @@ Sovereign Engineer SL-18497 SL-18525 SL-18534 + SL-19690 SpacedOut Frye VWR-34 VWR-45 From 026ef1935dbdb21ab79159d38fb78e126dd6ac95 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 8 May 2023 12:07:31 -0400 Subject: [PATCH 06/12] SL-19690: Follow up on Rye Mutt's fix for shutdown crashes. Rather than continuing to propagate try/catch (Closed) (aka LLThreadSafeQueueInterrupt) constructs through the code base, make WorkQueueBase::post() return bool indicating success (i.e. ! isClosed()). This obviates postIfOpen(), which no one was using anyway. In effect, postIfOpen() is renamed post(), bypassing the exception when isClosed(). Review existing try/catch blocks of that sort, changing to test for post() returning false. --- indra/llaudio/llaudiodecodemgr.cpp | 57 ++++++------ indra/llcommon/llqueuedthread.cpp | 2 +- indra/llcommon/workqueue.cpp | 21 +---- indra/llcommon/workqueue.h | 135 +++++++++++------------------ indra/llimage/llimageworker.cpp | 28 +++--- indra/llrender/llimagegl.h | 2 +- indra/llwindow/llwindowwin32.cpp | 13 +-- 7 files changed, 102 insertions(+), 156 deletions(-) diff --git a/indra/llaudio/llaudiodecodemgr.cpp b/indra/llaudio/llaudiodecodemgr.cpp index 38a6b41afe..190c5290cb 100755 --- a/indra/llaudio/llaudiodecodemgr.cpp +++ b/indra/llaudio/llaudiodecodemgr.cpp @@ -607,40 +607,37 @@ void LLAudioDecodeMgr::Impl::startMoreDecodes() // Kick off a decode mDecodes[decode_id] = LLPointer(NULL); - try - { - main_queue->postTo( - general_queue, - [decode_id]() // Work done on general queue + bool posted = main_queue->postTo( + general_queue, + [decode_id]() // Work done on general queue + { + LLPointer decode_state = beginDecodingAndWritingAudio(decode_id); + + if (!decode_state) { - LLPointer decode_state = beginDecodingAndWritingAudio(decode_id); - - if (!decode_state) - { - // Audio decode has errored - return decode_state; - } - - // Disk write of decoded audio is now in progress off-thread + // Audio decode has errored return decode_state; - }, - [decode_id, this](LLPointer decode_state) // Callback to main thread - mutable { - if (!gAudiop) - { - // There is no LLAudioEngine anymore. This might happen if - // an audio decode is enqueued just before shutdown. - return; - } + } - // At this point, we can be certain that the pointer to "this" - // is valid because the lifetime of "this" is dependent upon - // the lifetime of gAudiop. + // Disk write of decoded audio is now in progress off-thread + return decode_state; + }, + [decode_id, this](LLPointer decode_state) // Callback to main thread + mutable { + if (!gAudiop) + { + // There is no LLAudioEngine anymore. This might happen if + // an audio decode is enqueued just before shutdown. + return; + } - enqueueFinishAudio(decode_id, decode_state); - }); - } - catch (const LLThreadSafeQueueInterrupt&) + // At this point, we can be certain that the pointer to "this" + // is valid because the lifetime of "this" is dependent upon + // the lifetime of gAudiop. + + enqueueFinishAudio(decode_id, decode_state); + }); + if (! posted) { // Shutdown // Consider making processQueue() do a cleanup instead diff --git a/indra/llcommon/llqueuedthread.cpp b/indra/llcommon/llqueuedthread.cpp index 9b1de2e9a5..7da7c1e026 100644 --- a/indra/llcommon/llqueuedthread.cpp +++ b/indra/llcommon/llqueuedthread.cpp @@ -146,7 +146,7 @@ S32 LLQueuedThread::updateQueue(F32 max_time_ms) // schedule a call to threadedUpdate for every call to updateQueue if (!isQuitting()) { - mRequestQueue.postIfOpen([=]() + mRequestQueue.post([=]() { LL_PROFILE_ZONE_NAMED_CATEGORY_THREAD("qt - update"); mIdleThread = FALSE; diff --git a/indra/llcommon/workqueue.cpp b/indra/llcommon/workqueue.cpp index 83e0216ae7..5a77d517dd 100644 --- a/indra/llcommon/workqueue.cpp +++ b/indra/llcommon/workqueue.cpp @@ -161,12 +161,7 @@ bool LL::WorkQueue::done() return mQueue.done(); } -void LL::WorkQueue::post(const Work& callable) -{ - mQueue.push(callable); -} - -bool LL::WorkQueue::postIfOpen(const Work& callable) +bool LL::WorkQueue::post(const Work& callable) { return mQueue.pushIfOpen(callable); } @@ -215,26 +210,16 @@ bool LL::WorkSchedule::done() return mQueue.done(); } -void LL::WorkSchedule::post(const Work& callable) +bool LL::WorkSchedule::post(const Work& callable) { // Use TimePoint::clock::now() instead of TimePoint's representation of // the epoch because this WorkSchedule may contain a mix of past-due // TimedWork items and TimedWork items scheduled for the future. Sift this // new item into the correct place. - post(callable, TimePoint::clock::now()); -} - -void LL::WorkSchedule::post(const Work& callable, const TimePoint& time) -{ - mQueue.push(TimedWork(time, callable)); -} - -bool LL::WorkSchedule::postIfOpen(const Work& callable) -{ return postIfOpen(callable, TimePoint::clock::now()); } -bool LL::WorkSchedule::postIfOpen(const Work& callable, const TimePoint& time) +bool LL::WorkSchedule::post(const Work& callable, const TimePoint& time) { return mQueue.pushIfOpen(TimedWork(time, callable)); } diff --git a/indra/llcommon/workqueue.h b/indra/llcommon/workqueue.h index 5b704e7198..87fdd1517f 100644 --- a/indra/llcommon/workqueue.h +++ b/indra/llcommon/workqueue.h @@ -83,13 +83,10 @@ namespace LL /*---------------------- fire and forget API -----------------------*/ - /// fire-and-forget - virtual void post(const Work&) = 0; - /** * post work, unless the queue is closed before we can post */ - virtual bool postIfOpen(const Work&) = 0; + virtual bool post(const Work&) = 0; /** * post work, unless the queue is full @@ -247,13 +244,10 @@ namespace LL /*---------------------- fire and forget API -----------------------*/ - /// fire-and-forget - void post(const Work&) override; - /** * post work, unless the queue is closed before we can post */ - bool postIfOpen(const Work&) override; + bool post(const Work&) override; /** * post work, unless the queue is full @@ -320,22 +314,16 @@ namespace LL /*---------------------- fire and forget API -----------------------*/ - /// fire-and-forget - void post(const Work& callable) override; - - /// fire-and-forget, but at a particular (future?) time - void post(const Work& callable, const TimePoint& time); - /** * post work, unless the queue is closed before we can post */ - bool postIfOpen(const Work& callable) override; + bool post(const Work& callable) override; /** * post work for a particular time, unless the queue is closed before * we can post */ - bool postIfOpen(const Work& callable, const TimePoint& time); + bool post(const Work& callable, const TimePoint& time); /** * post work, unless the queue is full @@ -356,7 +344,7 @@ namespace LL * an LLCond variant, e.g. LLOneShotCond or LLBoolCond. */ template - void postEvery(const std::chrono::duration& interval, + bool postEvery(const std::chrono::duration& interval, CALLABLE&& callable); private: @@ -417,15 +405,10 @@ namespace LL // move-only callable; but naturally this statement must be // the last time we reference this instance, which may become // moved-from. - try - { - auto target{ std::dynamic_pointer_cast(mTarget.lock()) }; - target->post(std::move(*this), mStart); - } - catch (const Closed&) - { - // Once this queue is closed, oh well, just stop - } + auto target{ std::dynamic_pointer_cast(mTarget.lock()) }; + // Discard bool return: once this queue is closed, oh well, + // just stop + target->post(std::move(*this), mStart); } } @@ -437,7 +420,7 @@ namespace LL }; template - void WorkSchedule::postEvery(const std::chrono::duration& interval, + bool WorkSchedule::postEvery(const std::chrono::duration& interval, CALLABLE&& callable) { if (interval.count() <= 0) @@ -454,7 +437,7 @@ namespace LL // Instantiate and post a suitable BackJack, binding a weak_ptr to // self, the current time, the desired interval and the desired // callable. - post( + return post( BackJack( getWeak(), TimePoint::clock::now(), interval, std::move(callable))); } @@ -516,48 +499,37 @@ namespace LL // Here we believe target WorkQueue still exists. Post to it a // lambda that packages our callable, our callback and a weak_ptr // to this originating WorkQueue. - try - { - tptr->post( - [reply = super::getWeak(), - callable = std::move(callable), - callback = std::move(callback)] - () mutable + return tptr->post( + [reply = super::getWeak(), + callable = std::move(callable), + callback = std::move(callback)] + () mutable + { + // Use postMaybe() below in case this originating WorkQueue + // has been closed or destroyed. Remember, the outer lambda is + // now running on a thread servicing the target WorkQueue, and + // real time has elapsed since postTo()'s tptr->post() call. + try { - // Use postMaybe() below in case this originating WorkQueue - // has been closed or destroyed. Remember, the outer lambda is - // now running on a thread servicing the target WorkQueue, and - // real time has elapsed since postTo()'s tptr->post() call. - try - { - // Make a reply lambda to repost to THIS WorkQueue. - // Delegate to makeReplyLambda() so we can partially - // specialize on void return. - postMaybe(reply, makeReplyLambda(std::move(callable), std::move(callback))); - } - catch (...) - { - // Either variant of makeReplyLambda() is responsible for - // calling the caller's callable. If that throws, return - // the exception to the originating thread. - postMaybe( - reply, - // Bind the current exception to transport back to the - // originating WorkQueue. Once there, rethrow it. - [exc = std::current_exception()](){ std::rethrow_exception(exc); }); - } - }, - // if caller passed a TimePoint, pass it along to post() - std::forward(args)...); - } - catch (const Closed&) - { - // target WorkQueue still exists, but is Closed - return false; - } - - // looks like we were able to post() - return true; + // Make a reply lambda to repost to THIS WorkQueue. + // Delegate to makeReplyLambda() so we can partially + // specialize on void return. + postMaybe(reply, makeReplyLambda(std::move(callable), std::move(callback))); + } + catch (...) + { + // Either variant of makeReplyLambda() is responsible for + // calling the caller's callable. If that throws, return + // the exception to the originating thread. + postMaybe( + reply, + // Bind the current exception to transport back to the + // originating WorkQueue. Once there, rethrow it. + [exc = std::current_exception()](){ std::rethrow_exception(exc); }); + } + }, + // if caller passed a TimePoint, pass it along to post() + std::forward(args)...); } template @@ -568,18 +540,9 @@ namespace LL auto tptr = target.lock(); if (tptr) { - try - { - tptr->post(std::forward(args)...); - // we were able to post() - return true; - } - catch (const Closed&) - { - // target WorkQueue still exists, but is Closed - } + return tptr->post(std::forward(args)...); } - // either target no longer exists, or its WorkQueue is Closed + // target no longer exists return false; } @@ -591,7 +554,7 @@ namespace LL auto operator()(WorkQueueBase* self, CALLABLE&& callable, ARGS&&... args) { LLCoros::Promise promise; - self->post( + bool posted = self->post( // We dare to bind a reference to Promise because it's // specifically designed for cross-thread communication. [&promise, callable = std::move(callable)]() @@ -608,6 +571,10 @@ namespace LL }, // if caller passed a TimePoint, pass it to post() std::forward(args)...); + if (! posted) + { + LLTHROW(Closed); + } auto future{ LLCoros::getFuture(promise) }; // now, on the calling thread, wait for that result LLCoros::TempStatus st("waiting for WorkQueue::waitForResult()"); @@ -623,7 +590,7 @@ namespace LL void operator()(WorkQueueBase* self, CALLABLE&& callable, ARGS&&... args) { LLCoros::Promise promise; - self->post( + bool posted = self->post( // &promise is designed for cross-thread access [&promise, callable = std::move(callable)]() mutable { @@ -639,6 +606,10 @@ namespace LL }, // if caller passed a TimePoint, pass it to post() std::forward(args)...); + if (! posted) + { + LLTHROW(Closed); + } auto future{ LLCoros::getFuture(promise) }; // block until set_value() LLCoros::TempStatus st("waiting for void WorkQueue::waitForResult()"); diff --git a/indra/llimage/llimageworker.cpp b/indra/llimage/llimageworker.cpp index 9358a0ae2c..520c81a8ec 100644 --- a/indra/llimage/llimageworker.cpp +++ b/indra/llimage/llimageworker.cpp @@ -92,21 +92,19 @@ LLImageDecodeThread::handle_t LLImageDecodeThread::decodeImage( { LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE; - try - { - // 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); - }); - } - catch (const LLThreadSafeQueueInterrupt&) - { - LL_DEBUGS() << "Tried to start decoding on shutdown" << LL_ENDL; - } + // Instantiate the ImageRequest right in the lambda, why not? + bool posted = mThreadPool->getQueue().post( + [req = ImageRequest(image, discard, needs_aux, responder)] + () mutable + { + auto done = req.processRequest(); + req.finishRequest(done); + }); + if (! posted) + { + LL_DEBUGS() << "Tried to start decoding on shutdown" << LL_ENDL; + // should this return 0? + } // It's important to our consumer (LLTextureFetchWorker) that we return a // nonzero handle. It is NOT important that the nonzero handle be unique: diff --git a/indra/llrender/llimagegl.h b/indra/llrender/llimagegl.h index 87fb9e363e..243aeaea25 100644 --- a/indra/llrender/llimagegl.h +++ b/indra/llrender/llimagegl.h @@ -340,7 +340,7 @@ public: template bool post(CALLABLE&& func) { - return getQueue().postIfOpen(std::forward(func)); + return getQueue().post(std::forward(func)); } void run() override; diff --git a/indra/llwindow/llwindowwin32.cpp b/indra/llwindow/llwindowwin32.cpp index 43bef5ff68..c5a6a3fa8f 100644 --- a/indra/llwindow/llwindowwin32.cpp +++ b/indra/llwindow/llwindowwin32.cpp @@ -370,15 +370,10 @@ struct LLWindowWin32::LLWindowWin32Thread : public LL::ThreadPool template void post(CALLABLE&& func) { - try - { - getQueue().post(std::forward(func)); - } - catch (const LLThreadSafeQueueInterrupt&) - { - // Shutdown timing is tricky. The main thread can end up trying - // to post a cursor position after having closed the WorkQueue. - } + // Ignore bool return. Shutdown timing is tricky: the main thread can + // end up trying to post a cursor position after having closed the + // WorkQueue. + getQueue().post(std::forward(func)); } /** From d75ecde69c1289f4e3df8b75e9a74c5b05db318c Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 8 May 2023 12:31:57 -0400 Subject: [PATCH 07/12] SL-19690: Properly qualify exception type. --- indra/llcommon/workqueue.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/indra/llcommon/workqueue.h b/indra/llcommon/workqueue.h index 87fdd1517f..ec0700a718 100644 --- a/indra/llcommon/workqueue.h +++ b/indra/llcommon/workqueue.h @@ -573,7 +573,7 @@ namespace LL std::forward(args)...); if (! posted) { - LLTHROW(Closed); + LLTHROW(WorkQueueBase::Closed()); } auto future{ LLCoros::getFuture(promise) }; // now, on the calling thread, wait for that result @@ -608,7 +608,7 @@ namespace LL std::forward(args)...); if (! posted) { - LLTHROW(Closed); + LLTHROW(WorkQueueBase::Closed()); } auto future{ LLCoros::getFuture(promise) }; // block until set_value() From f728808d938666a01f73a039c861358fc4b02a9e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 8 May 2023 12:45:27 -0400 Subject: [PATCH 08/12] SL-19690: Fix a lingering reference to WorkSchedule::postIfOpen() --- indra/llcommon/workqueue.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indra/llcommon/workqueue.cpp b/indra/llcommon/workqueue.cpp index 5a77d517dd..cf80ce0656 100644 --- a/indra/llcommon/workqueue.cpp +++ b/indra/llcommon/workqueue.cpp @@ -216,7 +216,7 @@ bool LL::WorkSchedule::post(const Work& callable) // the epoch because this WorkSchedule may contain a mix of past-due // TimedWork items and TimedWork items scheduled for the future. Sift this // new item into the correct place. - return postIfOpen(callable, TimePoint::clock::now()); + return post(callable, TimePoint::clock::now()); } bool LL::WorkSchedule::post(const Work& callable, const TimePoint& time) From 3f1f1261869fbf0a40813b5492f49516e82af61a Mon Sep 17 00:00:00 2001 From: RunitaiLinden Date: Mon, 8 May 2023 15:21:04 -0500 Subject: [PATCH 09/12] DRTVWR-559 Remove main window proc printf debugging. --- indra/llwindow/llwindowwin32.cpp | 66 ++------------------------------ 1 file changed, 4 insertions(+), 62 deletions(-) diff --git a/indra/llwindow/llwindowwin32.cpp b/indra/llwindow/llwindowwin32.cpp index 43bef5ff68..4530e34369 100644 --- a/indra/llwindow/llwindowwin32.cpp +++ b/indra/llwindow/llwindowwin32.cpp @@ -2281,13 +2281,8 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ ASSERT_WINDOW_THREAD(); LL_PROFILE_ZONE_SCOPED_CATEGORY_WIN32; - LL_DEBUGS("Window") << "mainWindowProc(" << std::hex << h_wnd - << ", " << u_msg - << ", " << w_param << ")" << std::dec << LL_ENDL; - if (u_msg == WM_POST_FUNCTION_) { - LL_DEBUGS("Window") << "WM_POST_FUNCTION_" << LL_ENDL; // from LLWindowWin32Thread::Post() // Cast l_param back to the pointer to the heap FuncType // allocated by Post(). Capture in unique_ptr so we'll delete @@ -2334,8 +2329,6 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ case WM_DEVICECHANGE: { LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("mwp - WM_DEVICECHANGE"); - LL_INFOS("Window") << " WM_DEVICECHANGE: wParam=" << w_param - << "; lParam=" << l_param << LL_ENDL; if (w_param == DBT_DEVNODES_CHANGED || w_param == DBT_DEVICEARRIVAL) { WINDOW_IMP_POST(window_imp->mCallbacks->handleDeviceChange(window_imp)); @@ -2397,13 +2390,6 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ { // This message should be sent whenever the app gains or loses focus. BOOL activating = (BOOL)w_param; - BOOL minimized = window_imp->getMinimized(); - - LL_INFOS("Window") << "WINDOWPROC ActivateApp " - << " activating " << S32(activating) - << " minimized " << S32(minimized) - << " fullscreen " << S32(window_imp->mFullscreen) - << LL_ENDL; if (window_imp->mFullscreen) { @@ -2438,20 +2424,10 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ // Can be one of WA_ACTIVE, WA_CLICKACTIVE, or WA_INACTIVE BOOL activating = (LOWORD(w_param) != WA_INACTIVE); - BOOL minimized = BOOL(HIWORD(w_param)); - if (!activating && LLWinImm::isAvailable() && window_imp->mPreeditor) { window_imp->interruptLanguageTextInput(); } - - // JC - I'm not sure why, but if we don't report that we handled the - // WM_ACTIVATE message, the WM_ACTIVATEAPP messages don't work - // properly when we run fullscreen. - LL_INFOS("Window") << "WINDOWPROC Activate " - << " activating " << S32(activating) - << " minimized " << S32(minimized) - << LL_ENDL; }); break; @@ -2529,13 +2505,7 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ window_imp->mRawWParam = w_param; window_imp->mRawLParam = l_param; - { - LL_INFOS("Window") << "Debug WindowProc WM_KEYDOWN " - << " key " << S32(w_param) - << LL_ENDL; - - gKeyboard->handleKeyDown(w_param, mask); - } + gKeyboard->handleKeyDown(w_param, mask); }); if (eat_keystroke) return 0; // skip DefWindowProc() handling if we're consuming the keypress break; @@ -2555,11 +2525,7 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ window_imp->mRawLParam = l_param; { - LL_RECORD_BLOCK_TIME(FTM_KEYHANDLER); - - LL_INFOS("Window") << "Debug WindowProc WM_KEYUP " - << " key " << S32(w_param) - << LL_ENDL; + LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("mwp - WM_KEYUP"); gKeyboard->handleKeyUp(w_param, mask); } }); @@ -2569,7 +2535,6 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ case WM_IME_SETCONTEXT: { LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("mwp - WM_IME_SETCONTEXT"); - LL_INFOS("Window") << "WM_IME_SETCONTEXT" << LL_ENDL; if (LLWinImm::isAvailable() && window_imp->mPreeditor) { l_param &= ~ISC_SHOWUICOMPOSITIONWINDOW; @@ -2580,7 +2545,6 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ case WM_IME_STARTCOMPOSITION: { LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("mwp - WM_IME_STARTCOMPOSITION"); - LL_INFOS("Window") << "WM_IME_STARTCOMPOSITION" << LL_ENDL; if (LLWinImm::isAvailable() && window_imp->mPreeditor) { WINDOW_IMP_POST(window_imp->handleStartCompositionMessage()); @@ -2591,7 +2555,6 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ case WM_IME_ENDCOMPOSITION: { LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("mwp - WM_IME_ENDCOMPOSITION"); - LL_INFOS("Window") << "WM_IME_ENDCOMPOSITION" << LL_ENDL; if (LLWinImm::isAvailable() && window_imp->mPreeditor) { return 0; @@ -2601,7 +2564,6 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ case WM_IME_COMPOSITION: { LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("mwp - WM_IME_COMPOSITION"); - LL_INFOS("Window") << "WM_IME_COMPOSITION" << LL_ENDL; if (LLWinImm::isAvailable() && window_imp->mPreeditor) { WINDOW_IMP_POST(window_imp->handleCompositionMessage(l_param)); @@ -2612,7 +2574,6 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ case WM_IME_REQUEST: { LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("mwp - WM_IME_REQUEST"); - LL_INFOS("Window") << "WM_IME_REQUEST" << LL_ENDL; if (LLWinImm::isAvailable() && window_imp->mPreeditor) { LRESULT result; @@ -2641,9 +2602,7 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ // it is worth trying. The good old WM_CHAR works just fine even for supplementary // characters. We just need to take care of surrogate pairs sent as two WM_CHAR's // by ourselves. It is not that tough. -- Alissa Sabre @ SL - LL_INFOS("Window") << "Debug WindowProc WM_CHAR " - << " key " << S32(w_param) - << LL_ENDL; + // Even if LLWindowCallbacks::handleUnicodeChar(llwchar, BOOL) returned FALSE, // we *did* processed the event, so I believe we should not pass it to DefWindowProc... window_imp->handleUnicodeUTF16((U16)w_param, gKeyboard->currentMask(FALSE)); @@ -2967,21 +2926,6 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ { LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("mwp - WM_SIZE"); window_imp->updateWindowRect(); - S32 width = S32(LOWORD(l_param)); - S32 height = S32(HIWORD(l_param)); - - - LL_INFOS("Window"); - BOOL maximized = (w_param == SIZE_MAXIMIZED); - BOOL restored = (w_param == SIZE_RESTORED); - BOOL minimized = (w_param == SIZE_MINIMIZED); - - LL_CONT << "WINDOWPROC Size " - << width << "x" << height - << " max " << S32(maximized) - << " min " << S32(minimized) - << " rest " << S32(restored); - LL_ENDL; // There's an odd behavior with WM_SIZE that I would call a bug. If // the window is maximized, and you call MoveWindow() with a size smaller @@ -3047,7 +2991,6 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ case WM_SETFOCUS: { LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("mwp - WM_SETFOCUS"); - LL_INFOS("Window") << "WINDOWPROC SetFocus" << LL_ENDL; WINDOW_IMP_POST(window_imp->mCallbacks->handleFocus(window_imp)); return 0; } @@ -3055,7 +2998,6 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ case WM_KILLFOCUS: { LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("mwp - WM_KILLFOCUS"); - LL_INFOS("Window") << "WINDOWPROC KillFocus" << LL_ENDL; WINDOW_IMP_POST(window_imp->mCallbacks->handleFocusLost(window_imp)); return 0; } @@ -3176,7 +3118,7 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_ default: { LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("mwp - default"); - LL_INFOS("Window") << "Unhandled windows message code: 0x" << std::hex << U32(u_msg) << LL_ENDL; + LL_DEBUGS("Window") << "Unhandled windows message code: 0x" << std::hex << U32(u_msg) << LL_ENDL; } break; } From 8077d33ba05a46ea61914be686816be5b54bae6d Mon Sep 17 00:00:00 2001 From: RunitaiLinden Date: Thu, 11 May 2023 13:20:34 -0500 Subject: [PATCH 10/12] SL-19656 Remove LLPerfStats thread and fold into General thread. Hook avatar GPU time into LLPerfStats. Incidental decruft. --- indra/newview/llappviewer.cpp | 2 + indra/newview/llfloaterperformance.cpp | 11 +- indra/newview/llperfstats.cpp | 67 ++++++++---- indra/newview/llperfstats.h | 68 +++--------- indra/newview/llviewerstats.cpp | 2 +- indra/newview/llvoavatar.cpp | 146 +++++++++++++++---------- indra/newview/llvoavatar.h | 30 ++--- indra/newview/llworld.cpp | 6 +- indra/newview/pipeline.cpp | 42 ++++--- 9 files changed, 201 insertions(+), 173 deletions(-) diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 6efbaeacf7..c9fc0e2cc8 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -4698,6 +4698,8 @@ void LLAppViewer::idle() LLFrameTimer::updateFrameTime(); LLFrameTimer::updateFrameCount(); LLEventTimer::updateClass(); + LLPerfStats::updateClass(); + // LLApp::stepFrame() performs the above three calls plus mRunner.run(). // Not sure why we don't call stepFrame() here, except that LLRunner seems // completely redundant with LLEventTimer. diff --git a/indra/newview/llfloaterperformance.cpp b/indra/newview/llfloaterperformance.cpp index 8c5745aa43..19fc3e673e 100644 --- a/indra/newview/llfloaterperformance.cpp +++ b/indra/newview/llfloaterperformance.cpp @@ -456,15 +456,8 @@ void LLFloaterPerformance::populateNearbyList() row[1]["column"] = "complex_value"; row[1]["type"] = "text"; - if (is_slow && !showTunedART) - { - row[1]["value"] = llformat( "%.f", LLPerfStats::raw_to_us( avatar->getLastART() ) ); - } - else - { - // use GPU time in us - row[1]["value"] = llformat( "%.f", render_av_gpu_ms * 1000.f); - } + // use GPU time in us + row[1]["value"] = llformat( "%.f", render_av_gpu_ms * 1000.f); row[1]["font"]["name"] = "SANSSERIF"; row[3]["column"] = "name"; diff --git a/indra/newview/llperfstats.cpp b/indra/newview/llperfstats.cpp index b680d8761b..2281475d4a 100644 --- a/indra/newview/llperfstats.cpp +++ b/indra/newview/llperfstats.cpp @@ -39,6 +39,11 @@ extern LLControlGroup gSavedSettings; namespace LLPerfStats { + // avatar timing metrics in ms (updated once per mainloop iteration) + std::atomic sTotalAvatarTime = 0.f; + std::atomic sAverageAvatarTime = 0.f; + std::atomic sMaxAvatarTime = 0.f; + std::atomic tunedAvatars{0}; std::atomic renderAvatarMaxART_ns{(U64)(ART_UNLIMITED_NANOS)}; // highest render time we'll allow without culling features bool belowTargetFPS{false}; @@ -141,14 +146,12 @@ namespace LLPerfStats resetChanges(); } - StatsRecorder::StatsRecorder():q(1024*16),t(&StatsRecorder::run) + StatsRecorder::StatsRecorder():q(1024*16) { // create a queue - // create a thread to consume from the queue tunables.initialiseFromSettings(); LLPerfStats::cpu_hertz = (F64)LLTrace::BlockTimer::countsPerSecond(); LLPerfStats::vsync_max_fps = gViewerWindow->getWindow()->getRefreshRate(); - t.detach(); } // static @@ -200,20 +203,6 @@ namespace LLPerfStats } } - auto& statsMapAv = statsDoubleBuffer[writeBuffer][static_cast(ObjType_t::OT_AVATAR)]; - for(auto& stat_entry : statsMapAv) - { - for(auto& stat : avatarStatsToAvg) - { - auto val = stat_entry.second[static_cast(stat)]; - if(val > SMOOTHING_PERIODS) - { - auto avg = statsDoubleBuffer[writeBuffer ^ 1][static_cast(ObjType_t::OT_AVATAR)][stat_entry.first][static_cast(stat)]; - stat_entry.second[static_cast(stat)] = avg + (val / SMOOTHING_PERIODS) - (avg / SMOOTHING_PERIODS); - } - } - } - // swap the buffers if(enabled()) { @@ -293,6 +282,36 @@ namespace LLPerfStats } } + // called once per main loop iteration on main thread + void updateClass() + { + LL_PROFILE_ZONE_SCOPED_CATEGORY_STATS; + + sTotalAvatarTime = LLVOAvatar::getTotalGPURenderTime(); + sAverageAvatarTime = LLVOAvatar::getAverageGPURenderTime(); + sMaxAvatarTime = LLVOAvatar::getMaxGPURenderTime(); + + auto& general = LL::WorkQueue::getInstance("General"); + + if (general) + { + general->post([] { StatsRecorder::update(); }); + } + } + + // called once per main loop iteration on General thread + void StatsRecorder::update() + { + LL_PROFILE_ZONE_SCOPED_CATEGORY_STATS; + StatsRecord upd; + auto& instance{ StatsRecorder::getInstance() }; + + while (enabled() && !LLApp::isQuitting() && instance.q.tryPop(upd)) + { + instance.processUpdate(upd); + } + } + //static int StatsRecorder::countNearbyAvatars(S32 distance) { @@ -327,6 +346,8 @@ namespace LLPerfStats // static void StatsRecorder::updateAvatarParams() { + LL_PROFILE_ZONE_SCOPED_CATEGORY_STATS; + if(tunables.autoTuneTimeout) { LLPerfStats::lastSleepedFrame = gFrameCount; @@ -380,10 +401,10 @@ namespace LLPerfStats } } - auto av_render_max_raw = LLPerfStats::StatsRecorder::getMax(ObjType_t::OT_AVATAR, LLPerfStats::StatType_t::RENDER_COMBINED); + auto av_render_max_raw = ms_to_raw(sMaxAvatarTime); // Is our target frame time lower than current? If so we need to take action to reduce draw overheads. // cumulative avatar time (includes idle processing, attachments and base av) - auto tot_avatar_time_raw = LLPerfStats::StatsRecorder::getSum(ObjType_t::OT_AVATAR, LLPerfStats::StatType_t::RENDER_COMBINED); + auto tot_avatar_time_raw = ms_to_raw(sTotalAvatarTime); // The frametime budget we have based on the target FPS selected auto target_frame_time_raw = (U64)llround(LLPerfStats::cpu_hertz / (target_fps == 0 ? 1 : target_fps)); @@ -417,7 +438,7 @@ namespace LLPerfStats // if so we've got work to do // how much of the frame was spent on non avatar related work? - U64 non_avatar_time_raw = tot_frame_time_raw - tot_avatar_time_raw; + U64 non_avatar_time_raw = tot_frame_time_raw > tot_avatar_time_raw ? tot_frame_time_raw - tot_avatar_time_raw : 0; // If the target frame time < scene time (estimated as non_avatar time) U64 target_avatar_time_raw; @@ -475,7 +496,11 @@ namespace LLPerfStats { new_render_limit_ns = renderAvatarMaxART_ns; } - new_render_limit_ns -= LLPerfStats::ART_MIN_ADJUST_DOWN_NANOS; + + if (new_render_limit_ns > LLPerfStats::ART_MIN_ADJUST_DOWN_NANOS) + { + new_render_limit_ns -= LLPerfStats::ART_MIN_ADJUST_DOWN_NANOS; + } // bounce at the bottom to prevent "no limit" new_render_limit_ns = std::max((U64)new_render_limit_ns, (U64)LLPerfStats::ART_MINIMUM_NANOS); diff --git a/indra/newview/llperfstats.h b/indra/newview/llperfstats.h index a4768272b9..bb5677f237 100644 --- a/indra/newview/llperfstats.h +++ b/indra/newview/llperfstats.h @@ -42,6 +42,10 @@ extern U32 gFrameCount; extern LLUUID gAgentID; namespace LLPerfStats { + + // called once per main loop iteration + void updateClass(); + // Note if changing these, they should correspond with the log range of the correpsonding sliders static constexpr U64 ART_UNLIMITED_NANOS{50000000}; static constexpr U64 ART_MINIMUM_NANOS{100000}; @@ -68,7 +72,6 @@ namespace LLPerfStats enum class ObjType_t{ OT_GENERAL=0, // Also Unknown. Used for n/a type stats such as scenery - OT_AVATAR, OT_COUNT }; enum class StatType_t{ @@ -162,6 +165,9 @@ namespace LLPerfStats using Queue = LLThreadSafeQueue; public: + // called once per main loop iteration on General thread + static void update(); + static inline StatsRecorder& getInstance() { static StatsRecorder instance; @@ -241,7 +247,6 @@ namespace LLPerfStats auto ot{upd.objType}; auto& key{upd.objID}; - auto& avKey{upd.avID}; auto type {upd.statType}; auto val {upd.time}; @@ -251,13 +256,6 @@ namespace LLPerfStats doUpd(key, ot, type,val); return; } - - if (ot == ObjType_t::OT_AVATAR) - { - // LL_INFOS("perfstats") << "Avatar update:" << LL_ENDL; - doUpd(avKey, ot, type, val); - return; - } } static inline void doUpd(const LLUUID& key, ObjType_t ot, StatType_t type, uint64_t val) @@ -286,42 +284,7 @@ namespace LLPerfStats static void toggleBuffer(); static void clearStatsBuffers(); - // thread entry - static void run() - { - StatsRecord upd[10]; - auto & instance {StatsRecorder::getInstance()}; - LL_PROFILER_SET_THREAD_NAME("PerfStats"); - - while( enabled() && !LLApp::isExiting() ) - { - auto count = 0; - while (count < 10) - { - if (instance.q.tryPopFor(std::chrono::milliseconds(10), upd[count])) - { - count++; - } - else - { - break; - } - } - //LL_PROFILER_THREAD_BEGIN("PerfStats"); - if(count) - { - // LL_INFOS("perfstats") << "processing " << count << " updates." << LL_ENDL; - for(auto i =0; i < count; i++) - { - instance.processUpdate(upd[i]); - } - } - //LL_PROFILER_THREAD_END("PerfStats"); - } - } - Queue q; - std::thread t; ~StatsRecorder() = default; StatsRecorder(const StatsRecorder&) = delete; @@ -354,13 +317,6 @@ namespace LLPerfStats LL_PROFILE_ZONE_SCOPED_CATEGORY_STATS; }; - template < ObjType_t OD = ObjTypeDiscriminator, - std::enable_if_t * = nullptr> - RecordTime( const LLUUID & av, StatType_t type ):RecordTime(std::move(av), LLUUID::null, type) - { - //LL_PROFILE_ZONE_COLOR(tracy::Color::Purple); - }; - ~RecordTime() { if(!LLPerfStats::StatsRecorder::enabled()) @@ -375,13 +331,19 @@ namespace LLPerfStats }; }; - + inline double raw_to_ns(U64 raw) { return (static_cast(raw) * 1000000000.0) / LLPerfStats::cpu_hertz; }; inline double raw_to_us(U64 raw) { return (static_cast(raw) * 1000000.0) / LLPerfStats::cpu_hertz; }; inline double raw_to_ms(U64 raw) { return (static_cast(raw) * 1000.0) / LLPerfStats::cpu_hertz; }; + inline U64 ns_to_raw(double ns) { return (U64)(LLPerfStats::cpu_hertz * (ns / 1000000000.0)); } + inline U64 us_to_raw(double us) { return (U64)(LLPerfStats::cpu_hertz * (us / 1000000.0)); } + inline U64 ms_to_raw(double ms) { return (U64)(LLPerfStats::cpu_hertz * (ms / 1000.0)); + + } + + using RecordSceneTime = RecordTime; - using RecordAvatarTime = RecordTime; };// namespace LLPerfStats diff --git a/indra/newview/llviewerstats.cpp b/indra/newview/llviewerstats.cpp index 609e8290da..37999e58ff 100644 --- a/indra/newview/llviewerstats.cpp +++ b/indra/newview/llviewerstats.cpp @@ -472,7 +472,7 @@ void update_statistics() auto tot_frame_time_raw = LLPerfStats::StatsRecorder::getSceneStat(LLPerfStats::StatType_t::RENDER_FRAME); // cumulative avatar time (includes idle processing, attachments and base av) - auto tot_avatar_time_raw = LLPerfStats::StatsRecorder::getSum(LLPerfStats::ObjType_t::OT_AVATAR, LLPerfStats::StatType_t::RENDER_COMBINED); + auto tot_avatar_time_raw = LLPerfStats::us_to_raw(LLVOAvatar::getTotalGPURenderTime()); // cumulative avatar render specific time (a bit arbitrary as the processing is too.) // auto tot_av_idle_time_raw = LLPerfStats::StatsRecorder::getSum(AvType, LLPerfStats::StatType_t::RENDER_IDLE); // auto tot_avatar_render_time_raw = tot_avatar_time_raw - tot_av_idle_time_raw; diff --git a/indra/newview/llvoavatar.cpp b/indra/newview/llvoavatar.cpp index 95e9321d6f..af65588709 100644 --- a/indra/newview/llvoavatar.cpp +++ b/indra/newview/llvoavatar.cpp @@ -2562,7 +2562,6 @@ void LLVOAvatar::idleUpdate(LLAgent &agent, const F64 &time) return; } // record time and refresh "tooSlow" status - LLPerfStats::RecordAvatarTime T(getID(), LLPerfStats::StatType_t::RENDER_IDLE); // per avatar "idle" time. updateTooSlow(); static LLCachedControl disable_all_render_types(gSavedSettings, "DisableAllRenderTypes"); @@ -8310,14 +8309,6 @@ bool LLVOAvatar::isTooSlow() const return mTooSlow; } -// use Avatar Render Time as complexity metric -// markARTStale - Mark stale and set the frameupdate to now so that we can wait at least one frame to get a revised number. -void LLVOAvatar::markARTStale() -{ - mARTStale=true; - mLastARTUpdateFrame = LLFrameTimer::getFrameCount(); -} - // Udpate Avatar state based on render time void LLVOAvatar::updateTooSlow() { @@ -8328,41 +8319,9 @@ void LLVOAvatar::updateTooSlow() // mTooSlow - Is the avatar flagged as being slow (includes shadow time) // mTooSlowWithoutShadows - Is the avatar flagged as being slow even with shadows removed. - // mARTStale - the rendertime we have is stale because of an update. We need to force a re-render to re-assess slowness - - if( mARTStale ) - { - if ( LLFrameTimer::getFrameCount() - mLastARTUpdateFrame < 5 ) - { - // LL_INFOS() << this->getFullname() << " marked stale " << LL_ENDL; - // we've not had a chance to update yet (allow a few to be certain a full frame has passed) - return; - } - - mARTStale = false; - mTooSlow = false; - mTooSlowWithoutShadows = false; - // LL_INFOS() << this->getFullname() << " refreshed ART combined = " << mRenderTime << " @ " << mLastARTUpdateFrame << LL_ENDL; - } - - // Either we're not stale or we've updated. - - U64 render_time_raw; - U64 render_geom_time_raw; - - if( !mTooSlow ) - { - // we are fully rendered, so we use the live values - std::lock_guard lock{LLPerfStats::bufferToggleLock}; - render_time_raw = LLPerfStats::StatsRecorder::get(LLPerfStats::ObjType_t::OT_AVATAR, id, LLPerfStats::StatType_t::RENDER_COMBINED); - render_geom_time_raw = LLPerfStats::StatsRecorder::get(LLPerfStats::ObjType_t::OT_AVATAR, id, LLPerfStats::StatType_t::RENDER_GEOMETRY); - } - else - { - // use the cached values. - render_time_raw = mRenderTime; - render_geom_time_raw = mGeomTime; - } + + // get max render time in ms + F32 max_art_ms = (F32) (LLPerfStats::renderAvatarMaxART_ns / 1000000.0); bool autotune = LLPerfStats::tunables.userAutoTuneEnabled && !mIsControlAvatar && !isSelf(); @@ -8378,19 +8337,13 @@ void LLVOAvatar::updateTooSlow() } bool exceeds_max_ART = - ((LLPerfStats::renderAvatarMaxART_ns > 0) && (LLPerfStats::raw_to_ns(render_time_raw) >= LLPerfStats::renderAvatarMaxART_ns)); + ((LLPerfStats::renderAvatarMaxART_ns > 0) && + (mGPURenderTime >= max_art_ms)); // NOTE: don't use getGPURenderTime accessor here to avoid "isTooSlow" feedback loop if (exceeds_max_ART && !ignore_tune) { - if( !mTooSlow ) // if we were previously not slow (with or without shadows.) - { - // if we weren't capped, we are now - mLastARTUpdateFrame = LLFrameTimer::getFrameCount(); - mRenderTime = render_time_raw; - mGeomTime = render_geom_time_raw; - mARTStale = false; - mTooSlow = true; - } + mTooSlow = true; + if(!mTooSlowWithoutShadows) // if we were not previously above the full impostor cap { bool render_friend_or_exception = ( alwaysRenderFriends && LLAvatarTracker::instance().isBuddy( id ) ) || @@ -8398,13 +8351,12 @@ void LLVOAvatar::updateTooSlow() if( (!isSelf() || allowSelfImpostor) && !render_friend_or_exception ) { // Note: slow rendering Friends still get their shadows zapped. - mTooSlowWithoutShadows = (LLPerfStats::raw_to_ns(render_geom_time_raw) >= LLPerfStats::renderAvatarMaxART_ns); + mTooSlowWithoutShadows = getGPURenderTime()*2.f >= max_art_ms; // NOTE: assumes shadow rendering doubles render time } } } else { - // LL_INFOS() << this->getFullname() << " ("<< (combined?"combined":"geometry") << ") good render time = " << LLPerfStats::raw_to_ns(render_time_raw) << " vs ("<< LLVOAvatar::sRenderTimeCap_ns << " set @ " << mLastARTUpdateFrame << LL_ENDL; mTooSlow = false; mTooSlowWithoutShadows = false; @@ -11103,6 +11055,21 @@ void LLVOAvatar::calculateUpdateRenderComplexity() // HUD complexity LLHUDRenderNotifier::getInstance()->updateNotificationHUD(hud_complexity_list); } + + //schedule an update to ART next frame if needed + if (LLPerfStats::tunables.userAutoTuneEnabled && + LLPerfStats::tunables.userFPSTuningStrategy != LLPerfStats::TUNE_SCENE_ONLY && + !isVisuallyMuted()) + { + LLUUID id = getID(); // <== use id to make sure this avatar didn't get deleted between frames + LL::WorkQueue::getInstance("mainloop")->post([this, id]() + { + if (gObjectList.findObject(id) != nullptr) + { + gPipeline.profileAvatar(this); + } + }); + } } } @@ -11465,6 +11432,9 @@ void LLVOAvatar::readProfileQuery(S32 retries) glGetQueryObjectui64v(mGPUTimerQuery, GL_QUERY_RESULT, &time_elapsed); mGPURenderTime = time_elapsed / 1000000.f; mGPUProfilePending = false; + + setDebugText(llformat("%d", (S32)(mGPURenderTime * 1000.f))); + } else { // wait until next frame @@ -11477,3 +11447,67 @@ void LLVOAvatar::readProfileQuery(S32 retries) } } + +F32 LLVOAvatar::getGPURenderTime() +{ + return isVisuallyMuted() ? 0.f : mGPURenderTime; +} + +// static +F32 LLVOAvatar::getTotalGPURenderTime() +{ + LL_PROFILE_ZONE_SCOPED_CATEGORY_AVATAR; + + F32 ret = 0.f; + + for (LLCharacter* iter : LLCharacter::sInstances) + { + LLVOAvatar* inst = (LLVOAvatar*) iter; + ret += inst->getGPURenderTime(); + } + + return ret; +} + +F32 LLVOAvatar::getMaxGPURenderTime() +{ + LL_PROFILE_ZONE_SCOPED_CATEGORY_AVATAR; + + F32 ret = 0.f; + + for (LLCharacter* iter : LLCharacter::sInstances) + { + LLVOAvatar* inst = (LLVOAvatar*)iter; + ret = llmax(inst->getGPURenderTime(), ret); + } + + return ret; +} + +F32 LLVOAvatar::getAverageGPURenderTime() +{ + LL_PROFILE_ZONE_SCOPED_CATEGORY_AVATAR; + + F32 ret = 0.f; + + S32 count = 0; + + for (LLCharacter* iter : LLCharacter::sInstances) + { + LLVOAvatar* inst = (LLVOAvatar*)iter; + if (!inst->isTooSlow()) + { + ret += inst->getGPURenderTime(); + ++count; + } + } + + if (count > 0) + { + ret /= count; + } + + return ret; +} + + diff --git a/indra/newview/llvoavatar.h b/indra/newview/llvoavatar.h index 2ca44b041a..e8604464ae 100644 --- a/indra/newview/llvoavatar.h +++ b/indra/newview/llvoavatar.h @@ -309,11 +309,21 @@ public: void readProfileQuery(S32 retries); // get the GPU time in ms of rendering this avatar including all attachments - // returns -1 if this avatar has not been profiled using gPipeline.profileAvatar - F32 getGPURenderTime() { return mGPURenderTime; } + // returns 0.f if this avatar has not been profiled using gPipeline.profileAvatar + // or the avatar is visually muted + F32 getGPURenderTime(); + + // get the total GPU render time in ms of all avatars that have been benched + static F32 getTotalGPURenderTime(); + + // get the max GPU render time in ms of all avatars that have been benched + static F32 getMaxGPURenderTime(); + + // get the average GPU render time in ms of all avatars that have been benched + static F32 getAverageGPURenderTime(); // get the CPU time in ms of rendering this avatar including all attachments - // return -1 if this avatar has not been profiled using gPipeline.mProfileAvatar + // return 0.f if this avatar has not been profiled using gPipeline.mProfileAvatar F32 getCPURenderTime() { return mCPURenderTime; } @@ -401,8 +411,7 @@ public: void logMetricsTimerRecord(const std::string& phase_name, F32 elapsed, bool completed); void calcMutedAVColor(); - void markARTStale(); - + protected: LLViewerStats::PhaseMap& getPhases() { return mPhases; } BOOL updateIsFullyLoaded(); @@ -423,11 +432,6 @@ private: LLFrameTimer mFullyLoadedTimer; LLFrameTimer mRuthTimer; - U32 mLastARTUpdateFrame{0}; - U64 mRenderTime{0}; - U64 mGeomTime{0}; - bool mARTStale{true}; - bool mARTCapped{false}; // variables to hold "slowness" status bool mTooSlow{false}; bool mTooSlowWithoutShadows{false}; @@ -557,11 +561,11 @@ private: // profile results // GPU render time in ms - F32 mGPURenderTime = -1.f; + F32 mGPURenderTime = 0.f; bool mGPUProfilePending = false; // CPU render time in ms - F32 mCPURenderTime = -1.f; + F32 mCPURenderTime = 0.f; // the isTooComplex method uses these mutable values to avoid recalculating too frequently // DEPRECATED -- obsolete avatar render cost values @@ -1203,8 +1207,6 @@ public: // COF version of last appearance message received for this av. S32 mLastUpdateReceivedCOFVersion; - U64 getLastART() const { return mRenderTime; } - /** Diagnostics ** ** *******************************************************************************/ diff --git a/indra/newview/llworld.cpp b/indra/newview/llworld.cpp index 732ef1da6c..0e0dbdc071 100644 --- a/indra/newview/llworld.cpp +++ b/indra/newview/llworld.cpp @@ -1441,7 +1441,11 @@ F32 LLWorld::getNearbyAvatarsAndMaxGPUTime(std::vector &valid_near char_iter++; continue; } - gPipeline.profileAvatar(avatar); + + if (!avatar->isTooSlow()) + { + gPipeline.profileAvatar(avatar); + } nearby_max_complexity = llmax(nearby_max_complexity, avatar->getGPURenderTime()); valid_nearby_avs.push_back(*char_iter); } diff --git a/indra/newview/pipeline.cpp b/indra/newview/pipeline.cpp index 4d9a8a594a..6ea86c4cbb 100644 --- a/indra/newview/pipeline.cpp +++ b/indra/newview/pipeline.cpp @@ -10073,6 +10073,9 @@ void LLPipeline::profileAvatar(LLVOAvatar* avatar, bool profile_attachments) LL_PROFILE_ZONE_SCOPED_CATEGORY_PIPELINE; + // don't continue to profile an avatar that is known to be too slow + llassert(!avatar->isTooSlow()); + LLGLSLShader* cur_shader = LLGLSLShader::sCurBoundShaderPtr; mRT->deferredScreen.bindTarget(); @@ -10345,25 +10348,28 @@ void LLPipeline::generateImpostor(LLVOAvatar* avatar, bool preview_avatar, bool resY = llmin(nhpo2((U32) (fov*pa)), (U32) 512); resX = llmin(nhpo2((U32) (atanf(tdim.mV[0]/distance)*2.f*RAD_TO_DEG*pa)), (U32) 512); - if (!avatar->mImpostor.isComplete()) - { - avatar->mImpostor.allocate(resX, resY, GL_RGBA, true); + if (!for_profile) + { + if (!avatar->mImpostor.isComplete()) + { + avatar->mImpostor.allocate(resX, resY, GL_RGBA, true); - if (LLPipeline::sRenderDeferred) - { - addDeferredAttachments(avatar->mImpostor, true); - } - - gGL.getTexUnit(0)->bind(&avatar->mImpostor); - gGL.getTexUnit(0)->setTextureFilteringOption(LLTexUnit::TFO_POINT); - gGL.getTexUnit(0)->unbind(LLTexUnit::TT_TEXTURE); - } - else if(resX != avatar->mImpostor.getWidth() || resY != avatar->mImpostor.getHeight()) - { - avatar->mImpostor.resize(resX,resY); - } + if (LLPipeline::sRenderDeferred) + { + addDeferredAttachments(avatar->mImpostor, true); + } - avatar->mImpostor.bindTarget(); + gGL.getTexUnit(0)->bind(&avatar->mImpostor); + gGL.getTexUnit(0)->setTextureFilteringOption(LLTexUnit::TFO_POINT); + gGL.getTexUnit(0)->unbind(LLTexUnit::TT_TEXTURE); + } + else if (resX != avatar->mImpostor.getWidth() || resY != avatar->mImpostor.getHeight()) + { + avatar->mImpostor.resize(resX, resY); + } + + avatar->mImpostor.bindTarget(); + } } F32 old_alpha = LLDrawPoolAvatar::sMinimumAlpha; @@ -10466,7 +10472,7 @@ void LLPipeline::generateImpostor(LLVOAvatar* avatar, bool preview_avatar, bool gGL.popMatrix(); } - if (!preview_avatar) + if (!preview_avatar && !for_profile) { avatar->mImpostor.flush(); avatar->setImpostorDim(tdim); From 7d0903533c20a395e12c2d4c290fcde3ec6b2a53 Mon Sep 17 00:00:00 2001 From: Cosmic Linden Date: Thu, 11 May 2023 11:31:09 -0700 Subject: [PATCH 11/12] SL-19236: Fix HUDs not rendering when transparent water graphics setting is off --- indra/newview/lldrawpoolalpha.cpp | 2 +- indra/newview/pipeline.cpp | 8 +++++++- indra/newview/pipeline.h | 2 ++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/indra/newview/lldrawpoolalpha.cpp b/indra/newview/lldrawpoolalpha.cpp index ec57e20d35..f8d2a9e942 100644 --- a/indra/newview/lldrawpoolalpha.cpp +++ b/indra/newview/lldrawpoolalpha.cpp @@ -157,7 +157,7 @@ void LLDrawPoolAlpha::renderPostDeferred(S32 pass) { LL_PROFILE_ZONE_SCOPED_CATEGORY_DRAWPOOL; - if ((!LLPipeline::sRenderTransparentWater || gCubeSnapshot) && getType() == LLDrawPool::POOL_ALPHA_PRE_WATER) + if (LLPipeline::isWaterClip() && getType() == LLDrawPool::POOL_ALPHA_PRE_WATER) { // don't render alpha objects on the other side of the water plane if water is opaque return; } diff --git a/indra/newview/pipeline.cpp b/indra/newview/pipeline.cpp index 4d9a8a594a..73e568f1ae 100644 --- a/indra/newview/pipeline.cpp +++ b/indra/newview/pipeline.cpp @@ -2239,12 +2239,18 @@ bool LLPipeline::getVisibleExtents(LLCamera& camera, LLVector3& min, LLVector3& static LLTrace::BlockTimerStatHandle FTM_CULL("Object Culling"); +// static +bool LLPipeline::isWaterClip() +{ + return (!sRenderTransparentWater || gCubeSnapshot) && !sRenderingHUDs; +} + void LLPipeline::updateCull(LLCamera& camera, LLCullResult& result) { LL_PROFILE_ZONE_SCOPED_CATEGORY_PIPELINE; //LL_RECORD_BLOCK_TIME(FTM_CULL); LL_PROFILE_GPU_ZONE("updateCull"); // should always be zero GPU time, but drop a timer to flush stuff out - bool water_clip = !sRenderTransparentWater && !sRenderingHUDs; + bool water_clip = isWaterClip(); if (water_clip) { diff --git a/indra/newview/pipeline.h b/indra/newview/pipeline.h index e92fa32fc6..7eede30d8f 100644 --- a/indra/newview/pipeline.h +++ b/indra/newview/pipeline.h @@ -376,6 +376,8 @@ public: bool hasRenderType(const U32 type) const; bool hasAnyRenderType(const U32 type, ...) const; + static bool isWaterClip(); + void setRenderTypeMask(U32 type, ...); // This is equivalent to 'setRenderTypeMask' //void orRenderTypeMask(U32 type, ...); From 06bdee663433bf5b12eddcbcfcb8785546354c28 Mon Sep 17 00:00:00 2001 From: Brad Linden Date: Thu, 11 May 2023 12:18:45 -0700 Subject: [PATCH 12/12] Xcode build fixes for DRTVWR-559 --- indra/newview/llperfstats.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/indra/newview/llperfstats.cpp b/indra/newview/llperfstats.cpp index 2281475d4a..395ac0e788 100644 --- a/indra/newview/llperfstats.cpp +++ b/indra/newview/llperfstats.cpp @@ -175,11 +175,13 @@ namespace LLPerfStats // RENDER_MESHREPO, StatType_t::RENDER_IDLE }; +#if 0 static constexpr std::initializer_list avatarStatsToAvg = { StatType_t::RENDER_GEOMETRY, StatType_t::RENDER_SHADOWS, StatType_t::RENDER_COMBINED, StatType_t::RENDER_IDLE }; +#endif if( /*sceneStats[static_cast(StatType_t::RENDER_FPSLIMIT)] != 0 ||*/ sceneStats[static_cast(StatType_t::RENDER_SLEEP)] != 0 ) @@ -291,7 +293,7 @@ namespace LLPerfStats sAverageAvatarTime = LLVOAvatar::getAverageGPURenderTime(); sMaxAvatarTime = LLVOAvatar::getMaxGPURenderTime(); - auto& general = LL::WorkQueue::getInstance("General"); + auto general = LL::WorkQueue::getInstance("General"); if (general) {