From c22096846c98211e0bcd23dbbd155698fbd718b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Thu, 17 Jul 2025 13:13:08 +0200 Subject: [PATCH] Let reporters opt into fast path in RunContext::assertionStarting The fast path allows `RunContext` to skip disabling output redirect, and notifying the reporters, turning `RunContext::notifyAssertionStarted` into a no-op. This improves the overall performance of assertion handling significantly, and also prepares ground for future changes around assertion handling and thread safety. For simple 10M assertion run, this improves the running time by ~30% in Debug build and ~40% in Release build. For backwards-compatibility reasons, the fast path is disabled by default. However, none of the first party reporters use the `assertionStarting` event, so all first party reporters are opted-in. --- src/catch2/interfaces/catch_interfaces_reporter.hpp | 5 +++++ src/catch2/internal/catch_run_context.cpp | 7 +++++-- src/catch2/internal/catch_run_context.hpp | 3 +++ src/catch2/reporters/catch_reporter_automake.hpp | 8 +++++--- src/catch2/reporters/catch_reporter_compact.hpp | 5 ++++- src/catch2/reporters/catch_reporter_console.cpp | 5 ++++- src/catch2/reporters/catch_reporter_json.cpp | 2 ++ src/catch2/reporters/catch_reporter_junit.cpp | 1 + src/catch2/reporters/catch_reporter_multi.cpp | 2 ++ src/catch2/reporters/catch_reporter_multi.hpp | 5 +++++ src/catch2/reporters/catch_reporter_sonarqube.hpp | 1 + src/catch2/reporters/catch_reporter_tap.hpp | 1 + src/catch2/reporters/catch_reporter_teamcity.hpp | 1 + src/catch2/reporters/catch_reporter_xml.cpp | 1 + tests/SelfTest/TestRegistrations.cpp | 1 + 15 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/catch2/interfaces/catch_interfaces_reporter.hpp b/src/catch2/interfaces/catch_interfaces_reporter.hpp index 86264118..c7217889 100644 --- a/src/catch2/interfaces/catch_interfaces_reporter.hpp +++ b/src/catch2/interfaces/catch_interfaces_reporter.hpp @@ -115,6 +115,11 @@ namespace Catch { //! Catch2 should call `Reporter::assertionEnded` even for passing //! assertions bool shouldReportAllAssertions = false; + //! Catch2 should call `Reporter::assertionStarting` for all assertions + // Defaults to true for backwards compatibility, but none of our current + // reporters actually want this, and it enables a fast path in assertion + // handling. + bool shouldReportAllAssertionStarts = true; }; /** diff --git a/src/catch2/internal/catch_run_context.cpp b/src/catch2/internal/catch_run_context.cpp index b23c2146..df40d445 100644 --- a/src/catch2/internal/catch_run_context.cpp +++ b/src/catch2/internal/catch_run_context.cpp @@ -172,6 +172,7 @@ namespace Catch { m_lastKnownLineInfo("DummyLocation", static_cast(-1)), m_outputRedirect( makeOutputRedirect( m_reporter->getPreferences().shouldRedirectStdOut ) ), m_abortAfterXFailedAssertions( m_config->abortAfter() ), + m_reportAssertionStarting( m_reporter->getPreferences().shouldReportAllAssertionStarts ), m_includeSuccessfulResults( m_config->includeSuccessfulResults() || m_reporter->getPreferences().shouldReportAllAssertions ), m_shouldDebugBreak( m_config->shouldDebugBreak() ) { @@ -309,8 +310,10 @@ namespace Catch { } void RunContext::notifyAssertionStarted( AssertionInfo const& info ) { - auto _ = scopedDeactivate( *m_outputRedirect ); - m_reporter->assertionStarting( info ); + if (m_reportAssertionStarting) { + auto _ = scopedDeactivate( *m_outputRedirect ); + m_reporter->assertionStarting( info ); + } } bool RunContext::sectionStarted( StringRef sectionName, diff --git a/src/catch2/internal/catch_run_context.hpp b/src/catch2/internal/catch_run_context.hpp index ed3fda93..da320fe8 100644 --- a/src/catch2/internal/catch_run_context.hpp +++ b/src/catch2/internal/catch_run_context.hpp @@ -159,6 +159,9 @@ namespace Catch { size_t m_abortAfterXFailedAssertions; bool m_lastAssertionPassed = false; bool m_shouldReportUnexpected = true; + // Caches whether `assertionStarting` events should be sent to the reporter. + bool m_reportAssertionStarting; + // Caches whether `assertionEnded` events for successful assertions should be sent to the reporter bool m_includeSuccessfulResults; // Caches m_config->shouldDebugBreak() to avoid vptr calls/allow inlining bool m_shouldDebugBreak; diff --git a/src/catch2/reporters/catch_reporter_automake.hpp b/src/catch2/reporters/catch_reporter_automake.hpp index a639428c..d920adf6 100644 --- a/src/catch2/reporters/catch_reporter_automake.hpp +++ b/src/catch2/reporters/catch_reporter_automake.hpp @@ -19,9 +19,11 @@ namespace Catch { public: // GCC5 compat: we cannot use inherited constructor, because it // doesn't implement backport of P0136 - AutomakeReporter(ReporterConfig&& _config): - StreamingReporterBase(CATCH_MOVE(_config)) - {} + AutomakeReporter( ReporterConfig&& _config ): + StreamingReporterBase( CATCH_MOVE( _config ) ) { + m_preferences.shouldReportAllAssertionStarts = false; + } + ~AutomakeReporter() override; static std::string getDescription() { diff --git a/src/catch2/reporters/catch_reporter_compact.hpp b/src/catch2/reporters/catch_reporter_compact.hpp index d95bbff1..a6437c68 100644 --- a/src/catch2/reporters/catch_reporter_compact.hpp +++ b/src/catch2/reporters/catch_reporter_compact.hpp @@ -16,7 +16,10 @@ namespace Catch { class CompactReporter final : public StreamingReporterBase { public: - using StreamingReporterBase::StreamingReporterBase; + CompactReporter( ReporterConfig&& _config ): + StreamingReporterBase( CATCH_MOVE( _config ) ) { + m_preferences.shouldReportAllAssertionStarts = false; + } ~CompactReporter() override; diff --git a/src/catch2/reporters/catch_reporter_console.cpp b/src/catch2/reporters/catch_reporter_console.cpp index 0af47115..acb4ae8e 100644 --- a/src/catch2/reporters/catch_reporter_console.cpp +++ b/src/catch2/reporters/catch_reporter_console.cpp @@ -401,7 +401,10 @@ ConsoleReporter::ConsoleReporter(ReporterConfig&& config): { "est run time high mean high std dev", 14, Justification::Right } }; } - }())) {} + }())) { + m_preferences.shouldReportAllAssertionStarts = false; +} + ConsoleReporter::~ConsoleReporter() = default; std::string ConsoleReporter::getDescription() { diff --git a/src/catch2/reporters/catch_reporter_json.cpp b/src/catch2/reporters/catch_reporter_json.cpp index bf761245..4328d560 100644 --- a/src/catch2/reporters/catch_reporter_json.cpp +++ b/src/catch2/reporters/catch_reporter_json.cpp @@ -51,6 +51,8 @@ namespace Catch { // not, but for machine-parseable reporters I think the answer // should be yes. m_preferences.shouldReportAllAssertions = true; + // We only handle assertions when they end + m_preferences.shouldReportAllAssertionStarts = false; m_objectWriters.emplace( m_stream ); m_writers.emplace( Writer::Object ); diff --git a/src/catch2/reporters/catch_reporter_junit.cpp b/src/catch2/reporters/catch_reporter_junit.cpp index 27bdfe28..df34f17b 100644 --- a/src/catch2/reporters/catch_reporter_junit.cpp +++ b/src/catch2/reporters/catch_reporter_junit.cpp @@ -89,6 +89,7 @@ namespace Catch { { m_preferences.shouldRedirectStdOut = true; m_preferences.shouldReportAllAssertions = false; + m_preferences.shouldReportAllAssertionStarts = false; m_shouldStoreSuccesfulAssertions = false; } diff --git a/src/catch2/reporters/catch_reporter_multi.cpp b/src/catch2/reporters/catch_reporter_multi.cpp index 531902be..6bf9bcbd 100644 --- a/src/catch2/reporters/catch_reporter_multi.cpp +++ b/src/catch2/reporters/catch_reporter_multi.cpp @@ -19,6 +19,8 @@ namespace Catch { reporterish.getPreferences().shouldRedirectStdOut; m_preferences.shouldReportAllAssertions |= reporterish.getPreferences().shouldReportAllAssertions; + m_preferences.shouldReportAllAssertionStarts |= + reporterish.getPreferences().shouldReportAllAssertionStarts; } void MultiReporter::addListener( IEventListenerPtr&& listener ) { diff --git a/src/catch2/reporters/catch_reporter_multi.hpp b/src/catch2/reporters/catch_reporter_multi.hpp index 66113837..d206663e 100644 --- a/src/catch2/reporters/catch_reporter_multi.hpp +++ b/src/catch2/reporters/catch_reporter_multi.hpp @@ -29,6 +29,11 @@ namespace Catch { void updatePreferences(IEventListener const& reporterish); public: + MultiReporter( IConfig const* config ): + IEventListener( config ) { + m_preferences.shouldReportAllAssertionStarts = false; + } + using IEventListener::IEventListener; void addListener( IEventListenerPtr&& listener ); diff --git a/src/catch2/reporters/catch_reporter_sonarqube.hpp b/src/catch2/reporters/catch_reporter_sonarqube.hpp index 509f411e..98803a2a 100644 --- a/src/catch2/reporters/catch_reporter_sonarqube.hpp +++ b/src/catch2/reporters/catch_reporter_sonarqube.hpp @@ -22,6 +22,7 @@ namespace Catch { , xml(m_stream) { m_preferences.shouldRedirectStdOut = true; m_preferences.shouldReportAllAssertions = false; + m_preferences.shouldReportAllAssertionStarts = false; m_shouldStoreSuccesfulAssertions = false; } diff --git a/src/catch2/reporters/catch_reporter_tap.hpp b/src/catch2/reporters/catch_reporter_tap.hpp index e6889bb1..4664928f 100644 --- a/src/catch2/reporters/catch_reporter_tap.hpp +++ b/src/catch2/reporters/catch_reporter_tap.hpp @@ -18,6 +18,7 @@ namespace Catch { TAPReporter( ReporterConfig&& config ): StreamingReporterBase( CATCH_MOVE(config) ) { m_preferences.shouldReportAllAssertions = true; + m_preferences.shouldReportAllAssertionStarts = false; } static std::string getDescription() { diff --git a/src/catch2/reporters/catch_reporter_teamcity.hpp b/src/catch2/reporters/catch_reporter_teamcity.hpp index 662e9892..4adb0e37 100644 --- a/src/catch2/reporters/catch_reporter_teamcity.hpp +++ b/src/catch2/reporters/catch_reporter_teamcity.hpp @@ -26,6 +26,7 @@ namespace Catch { : StreamingReporterBase( CATCH_MOVE(_config) ) { m_preferences.shouldRedirectStdOut = true; + m_preferences.shouldReportAllAssertionStarts = false; } ~TeamCityReporter() override; diff --git a/src/catch2/reporters/catch_reporter_xml.cpp b/src/catch2/reporters/catch_reporter_xml.cpp index 5adf3acf..8b7b5a70 100644 --- a/src/catch2/reporters/catch_reporter_xml.cpp +++ b/src/catch2/reporters/catch_reporter_xml.cpp @@ -30,6 +30,7 @@ namespace Catch { { m_preferences.shouldRedirectStdOut = true; m_preferences.shouldReportAllAssertions = true; + m_preferences.shouldReportAllAssertionStarts = false; } XmlReporter::~XmlReporter() = default; diff --git a/tests/SelfTest/TestRegistrations.cpp b/tests/SelfTest/TestRegistrations.cpp index d7a6966f..1a7be39d 100644 --- a/tests/SelfTest/TestRegistrations.cpp +++ b/tests/SelfTest/TestRegistrations.cpp @@ -54,6 +54,7 @@ public: ValidatingTestListener(Catch::IConfig const* config) : EventListenerBase(config) { m_preferences.shouldReportAllAssertions = true; + m_preferences.shouldReportAllAssertionStarts = true; } void testRunStarting( Catch::TestRunInfo const& ) override {