Skip to content

Commit c220968

Browse files
committed
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.
1 parent ae33e5b commit c220968

15 files changed

Lines changed: 41 additions & 7 deletions

src/catch2/interfaces/catch_interfaces_reporter.hpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,11 @@ namespace Catch {
115115
//! Catch2 should call `Reporter::assertionEnded` even for passing
116116
//! assertions
117117
bool shouldReportAllAssertions = false;
118+
//! Catch2 should call `Reporter::assertionStarting` for all assertions
119+
// Defaults to true for backwards compatibility, but none of our current
120+
// reporters actually want this, and it enables a fast path in assertion
121+
// handling.
122+
bool shouldReportAllAssertionStarts = true;
118123
};
119124

120125
/**

src/catch2/internal/catch_run_context.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ namespace Catch {
172172
m_lastKnownLineInfo("DummyLocation", static_cast<size_t>(-1)),
173173
m_outputRedirect( makeOutputRedirect( m_reporter->getPreferences().shouldRedirectStdOut ) ),
174174
m_abortAfterXFailedAssertions( m_config->abortAfter() ),
175+
m_reportAssertionStarting( m_reporter->getPreferences().shouldReportAllAssertionStarts ),
175176
m_includeSuccessfulResults( m_config->includeSuccessfulResults() || m_reporter->getPreferences().shouldReportAllAssertions ),
176177
m_shouldDebugBreak( m_config->shouldDebugBreak() )
177178
{
@@ -309,8 +310,10 @@ namespace Catch {
309310
}
310311

311312
void RunContext::notifyAssertionStarted( AssertionInfo const& info ) {
312-
auto _ = scopedDeactivate( *m_outputRedirect );
313-
m_reporter->assertionStarting( info );
313+
if (m_reportAssertionStarting) {
314+
auto _ = scopedDeactivate( *m_outputRedirect );
315+
m_reporter->assertionStarting( info );
316+
}
314317
}
315318

316319
bool RunContext::sectionStarted( StringRef sectionName,

src/catch2/internal/catch_run_context.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ namespace Catch {
159159
size_t m_abortAfterXFailedAssertions;
160160
bool m_lastAssertionPassed = false;
161161
bool m_shouldReportUnexpected = true;
162+
// Caches whether `assertionStarting` events should be sent to the reporter.
163+
bool m_reportAssertionStarting;
164+
// Caches whether `assertionEnded` events for successful assertions should be sent to the reporter
162165
bool m_includeSuccessfulResults;
163166
// Caches m_config->shouldDebugBreak() to avoid vptr calls/allow inlining
164167
bool m_shouldDebugBreak;

src/catch2/reporters/catch_reporter_automake.hpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ namespace Catch {
1919
public:
2020
// GCC5 compat: we cannot use inherited constructor, because it
2121
// doesn't implement backport of P0136
22-
AutomakeReporter(ReporterConfig&& _config):
23-
StreamingReporterBase(CATCH_MOVE(_config))
24-
{}
22+
AutomakeReporter( ReporterConfig&& _config ):
23+
StreamingReporterBase( CATCH_MOVE( _config ) ) {
24+
m_preferences.shouldReportAllAssertionStarts = false;
25+
}
26+
2527
~AutomakeReporter() override;
2628

2729
static std::string getDescription() {

src/catch2/reporters/catch_reporter_compact.hpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ namespace Catch {
1616

1717
class CompactReporter final : public StreamingReporterBase {
1818
public:
19-
using StreamingReporterBase::StreamingReporterBase;
19+
CompactReporter( ReporterConfig&& _config ):
20+
StreamingReporterBase( CATCH_MOVE( _config ) ) {
21+
m_preferences.shouldReportAllAssertionStarts = false;
22+
}
2023

2124
~CompactReporter() override;
2225

src/catch2/reporters/catch_reporter_console.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,10 @@ ConsoleReporter::ConsoleReporter(ReporterConfig&& config):
401401
{ "est run time high mean high std dev", 14, Justification::Right }
402402
};
403403
}
404-
}())) {}
404+
}())) {
405+
m_preferences.shouldReportAllAssertionStarts = false;
406+
}
407+
405408
ConsoleReporter::~ConsoleReporter() = default;
406409

407410
std::string ConsoleReporter::getDescription() {

src/catch2/reporters/catch_reporter_json.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ namespace Catch {
5151
// not, but for machine-parseable reporters I think the answer
5252
// should be yes.
5353
m_preferences.shouldReportAllAssertions = true;
54+
// We only handle assertions when they end
55+
m_preferences.shouldReportAllAssertionStarts = false;
5456

5557
m_objectWriters.emplace( m_stream );
5658
m_writers.emplace( Writer::Object );

src/catch2/reporters/catch_reporter_junit.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ namespace Catch {
8989
{
9090
m_preferences.shouldRedirectStdOut = true;
9191
m_preferences.shouldReportAllAssertions = false;
92+
m_preferences.shouldReportAllAssertionStarts = false;
9293
m_shouldStoreSuccesfulAssertions = false;
9394
}
9495

src/catch2/reporters/catch_reporter_multi.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ namespace Catch {
1919
reporterish.getPreferences().shouldRedirectStdOut;
2020
m_preferences.shouldReportAllAssertions |=
2121
reporterish.getPreferences().shouldReportAllAssertions;
22+
m_preferences.shouldReportAllAssertionStarts |=
23+
reporterish.getPreferences().shouldReportAllAssertionStarts;
2224
}
2325

2426
void MultiReporter::addListener( IEventListenerPtr&& listener ) {

src/catch2/reporters/catch_reporter_multi.hpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ namespace Catch {
2929
void updatePreferences(IEventListener const& reporterish);
3030

3131
public:
32+
MultiReporter( IConfig const* config ):
33+
IEventListener( config ) {
34+
m_preferences.shouldReportAllAssertionStarts = false;
35+
}
36+
3237
using IEventListener::IEventListener;
3338

3439
void addListener( IEventListenerPtr&& listener );

0 commit comments

Comments
 (0)