From f80956a43a3e276686ed59531b41617282d75a0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Mon, 1 Dec 2025 22:23:51 +0100 Subject: [PATCH] Use magic statics for non-trivial thread-local globals This avoids calling the global's constructor on threads that will never interact with them. Calling the constructor can have surprising overhead, as e.g. MSVC's Debug mode `std::vector` will allocate in the default constructor. Closes #3050 --- src/catch2/internal/catch_message_info.cpp | 14 ++++--- src/catch2/internal/catch_message_info.hpp | 3 -- src/catch2/internal/catch_run_context.cpp | 48 ++++++++++++++-------- 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/catch2/internal/catch_message_info.cpp b/src/catch2/internal/catch_message_info.cpp index 2352f36c..9f8690a4 100644 --- a/src/catch2/internal/catch_message_info.cpp +++ b/src/catch2/internal/catch_message_info.cpp @@ -7,20 +7,24 @@ // SPDX-License-Identifier: BSL-1.0 #include +#include namespace Catch { + namespace { + // Messages are owned by their individual threads, so the counter should + // be thread-local as well. Alternative consideration: atomic counter, + // so threads don't share IDs and things are easier to debug. + static CATCH_INTERNAL_THREAD_LOCAL unsigned int messageIDCounter = 0; + } + MessageInfo::MessageInfo( StringRef _macroName, SourceLineInfo const& _lineInfo, ResultWas::OfType _type ) : macroName( _macroName ), lineInfo( _lineInfo ), type( _type ), - sequence( ++globalCount ) + sequence( ++messageIDCounter ) {} - // Messages are owned by their individual threads, so the counter should be thread-local as well. - // Alternative consideration: atomic, so threads don't share IDs and things are easier to debug. - CATCH_INTERNAL_THREAD_LOCAL unsigned int MessageInfo::globalCount = 0; - } // end namespace Catch diff --git a/src/catch2/internal/catch_message_info.hpp b/src/catch2/internal/catch_message_info.hpp index 803a0c61..8443c18c 100644 --- a/src/catch2/internal/catch_message_info.hpp +++ b/src/catch2/internal/catch_message_info.hpp @@ -12,7 +12,6 @@ #include #include #include -#include #include @@ -38,8 +37,6 @@ namespace Catch { bool operator < (MessageInfo const& other) const { return sequence < other.sequence; } - private: - static CATCH_INTERNAL_THREAD_LOCAL unsigned int globalCount; }; } // end namespace Catch diff --git a/src/catch2/internal/catch_run_context.cpp b/src/catch2/internal/catch_run_context.cpp index 7fce10da..1321d822 100644 --- a/src/catch2/internal/catch_run_context.cpp +++ b/src/catch2/internal/catch_run_context.cpp @@ -174,6 +174,12 @@ namespace Catch { // should also be thread local. For now we just use naked globals // below, in the future we will want to allocate piece of memory // from heap, to avoid consuming too much thread-local storage. + // + // Note that we also don't want non-trivial the thread-local variables + // below be initialized for every thread, only for those that touch + // Catch2. To make this work with both GCC/Clang and MSVC, we have to + // make them thread-local magic statics. (Class-level statics have the + // desired semantics on GCC, but not on MSVC). // This is used for the "if" part of CHECKED_IF/CHECKED_ELSE static CATCH_INTERNAL_THREAD_LOCAL bool g_lastAssertionPassed = false; @@ -181,7 +187,8 @@ namespace Catch { // This is the source location for last encountered macro. It is // used to provide the users with more precise location of error // when an unexpected exception/fatal error happens. - static CATCH_INTERNAL_THREAD_LOCAL SourceLineInfo g_lastKnownLineInfo("DummyLocation", static_cast(-1)); + static CATCH_INTERNAL_THREAD_LOCAL SourceLineInfo + g_lastKnownLineInfo( "DummyLocation", static_cast( -1 ) ); // Should we clear message scopes before sending off the messages to // reporter? Set in `assertionPassedFastPath` to avoid doing the full @@ -191,10 +198,16 @@ namespace Catch { CATCH_INTERNAL_START_WARNINGS_SUPPRESSION CATCH_INTERNAL_SUPPRESS_GLOBALS_WARNINGS // Actual messages to be provided to the reporter - static CATCH_INTERNAL_THREAD_LOCAL std::vector g_messages; + static std::vector& g_messages() { + static CATCH_INTERNAL_THREAD_LOCAL std::vector value; + return value; + } // Owners for the UNSCOPED_X information macro - static CATCH_INTERNAL_THREAD_LOCAL std::vector g_messageScopes; + static std::vector& g_messageScopes() { + static CATCH_INTERNAL_THREAD_LOCAL std::vector value; + return value; + } CATCH_INTERNAL_STOP_WARNINGS_SUPPRESSION } // namespace Detail @@ -341,7 +354,7 @@ namespace Catch { } if ( Detail::g_clearMessageScopes ) { - Detail::g_messageScopes.clear(); + Detail::g_messageScopes().clear(); Detail::g_clearMessageScopes = false; } @@ -350,11 +363,11 @@ namespace Catch { { auto _ = scopedDeactivate( *m_outputRedirect ); updateTotalsFromAtomics(); - m_reporter->assertionEnded( AssertionStats( result, Detail::g_messages, m_totals ) ); + m_reporter->assertionEnded( AssertionStats( result, Detail::g_messages(), m_totals ) ); } if ( result.getResultType() != ResultWas::Warning ) { - Detail::g_messageScopes.clear(); + Detail::g_messageScopes().clear(); } // Reset working state. assertion info will be reset after @@ -643,10 +656,10 @@ namespace Catch { m_testCaseTracker->close(); handleUnfinishedSections(); - Detail::g_messageScopes.clear(); + Detail::g_messageScopes().clear(); // TBD: At this point, m_messages should be empty. Do we want to // assert that this is true, or keep the defensive clear call? - Detail::g_messages.clear(); + Detail::g_messages().clear(); SectionStats testCaseSectionStats(CATCH_MOVE(testCaseSection), assertions, duration, missingAssertions); m_reporter->sectionEnded(testCaseSectionStats); @@ -814,7 +827,7 @@ namespace Catch { } void IResultCapture::pushScopedMessage( MessageInfo&& message ) { - Detail::g_messages.push_back( CATCH_MOVE( message ) ); + Detail::g_messages().push_back( CATCH_MOVE( message ) ); } void IResultCapture::popScopedMessage( unsigned int messageId ) { @@ -823,12 +836,13 @@ namespace Catch { // messages than low single digits, so the optimization is tiny, // and we would have to hand-write the loop to avoid terrible // codegen of reverse iterators in debug mode. - Detail::g_messages.erase( std::find_if( Detail::g_messages.begin(), - Detail::g_messages.end(), - [=]( MessageInfo const& msg ) { - return msg.sequence == - messageId; - } ) ); + auto& messages = Detail::g_messages(); + messages.erase( std::find_if( messages.begin(), + messages.end(), + [=]( MessageInfo const& msg ) { + return msg.sequence == + messageId; + } ) ); } void IResultCapture::emplaceUnscopedMessage( MessageBuilder&& builder ) { @@ -837,10 +851,10 @@ namespace Catch { // delayed clear in assertion handling will erase the valid ones // as well. if ( Detail::g_clearMessageScopes ) { - Detail::g_messageScopes.clear(); + Detail::g_messageScopes().clear(); Detail::g_clearMessageScopes = false; } - Detail::g_messageScopes.emplace_back( CATCH_MOVE( builder ) ); + Detail::g_messageScopes().emplace_back( CATCH_MOVE( builder ) ); } void seedRng(IConfig const& config) {