From dcab8a59716d785ad2ed8512a35f27b671ae1b20 Mon Sep 17 00:00:00 2001 From: Neal Coombes Date: Wed, 21 Jun 2017 13:34:58 -0500 Subject: [PATCH] Performance improvement in AssertionInfo. By using char const * instead of std::string we avoid significant copying per assertion. In a simple loop with 10000000 CHECKS on my system, this reduces the run time from 9.8s to 6s. --- include/internal/catch_assertionresult.h | 12 ++++++---- include/internal/catch_assertionresult.hpp | 28 ++++++++++++++-------- include/internal/catch_result_builder.hpp | 11 +++------ include/internal/catch_run_context.hpp | 8 +++---- 4 files changed, 32 insertions(+), 27 deletions(-) diff --git a/include/internal/catch_assertionresult.h b/include/internal/catch_assertionresult.h index 22d1355e..5d6c86d1 100644 --- a/include/internal/catch_assertionresult.h +++ b/include/internal/catch_assertionresult.h @@ -40,15 +40,17 @@ namespace Catch { struct AssertionInfo { AssertionInfo() {} - AssertionInfo( std::string const& _macroName, + AssertionInfo( char const * _macroName, SourceLineInfo const& _lineInfo, - std::string const& _capturedExpression, - ResultDisposition::Flags _resultDisposition ); + char const * _capturedExpression, + ResultDisposition::Flags _resultDisposition, + char const * _secondArg = ""); - std::string macroName; + char const * macroName; SourceLineInfo lineInfo; - std::string capturedExpression; + char const * capturedExpression; ResultDisposition::Flags resultDisposition; + char const * secondArg; }; struct AssertionResultData diff --git a/include/internal/catch_assertionresult.hpp b/include/internal/catch_assertionresult.hpp index 9b637028..2fa3373d 100644 --- a/include/internal/catch_assertionresult.hpp +++ b/include/internal/catch_assertionresult.hpp @@ -13,14 +13,16 @@ namespace Catch { - AssertionInfo::AssertionInfo( std::string const& _macroName, + AssertionInfo::AssertionInfo( char const * _macroName, SourceLineInfo const& _lineInfo, - std::string const& _capturedExpression, - ResultDisposition::Flags _resultDisposition ) + char const * _capturedExpression, + ResultDisposition::Flags _resultDisposition, + char const * _secondArg) : macroName( _macroName ), lineInfo( _lineInfo ), capturedExpression( _capturedExpression ), - resultDisposition( _resultDisposition ) + resultDisposition( _resultDisposition ), + secondArg( _secondArg ) {} AssertionResult::AssertionResult() {} @@ -47,24 +49,30 @@ namespace Catch { } bool AssertionResult::hasExpression() const { - return !m_info.capturedExpression.empty(); + return m_info.capturedExpression[0] != 0; } bool AssertionResult::hasMessage() const { return !m_resultData.message.empty(); } + std::string capturedExpressionWithSecondArgument( char const * capturedExpression, char const * secondArg ) { + return (secondArg[0] == 0 || secondArg[0] == '"' && secondArg[1] == '"') + ? capturedExpression + : std::string(capturedExpression) + ", " + secondArg; + } + std::string AssertionResult::getExpression() const { if( isFalseTest( m_info.resultDisposition ) ) - return '!' + m_info.capturedExpression; + return '!' + capturedExpressionWithSecondArgument(m_info.capturedExpression, m_info.secondArg); else - return m_info.capturedExpression; + return capturedExpressionWithSecondArgument(m_info.capturedExpression, m_info.secondArg); } std::string AssertionResult::getExpressionInMacro() const { - if( m_info.macroName.empty() ) - return m_info.capturedExpression; + if( m_info.macroName[0] == 0 ) + return capturedExpressionWithSecondArgument(m_info.capturedExpression, m_info.secondArg); else - return m_info.macroName + "( " + m_info.capturedExpression + " )"; + return std::string(m_info.macroName) + "( " + capturedExpressionWithSecondArgument(m_info.capturedExpression, m_info.secondArg) + " )"; } bool AssertionResult::hasExpandedExpression() const { diff --git a/include/internal/catch_result_builder.hpp b/include/internal/catch_result_builder.hpp index 2874c568..510fbc3b 100644 --- a/include/internal/catch_result_builder.hpp +++ b/include/internal/catch_result_builder.hpp @@ -18,17 +18,12 @@ namespace Catch { - std::string capturedExpressionWithSecondArgument( std::string const& capturedExpression, std::string const& secondArg ) { - return secondArg.empty() || secondArg == "\"\"" - ? capturedExpression - : capturedExpression + ", " + secondArg; - } ResultBuilder::ResultBuilder( char const* macroName, SourceLineInfo const& lineInfo, char const* capturedExpression, ResultDisposition::Flags resultDisposition, char const* secondArg ) - : m_assertionInfo( macroName, lineInfo, capturedExpressionWithSecondArgument( capturedExpression, secondArg ), resultDisposition ), + : m_assertionInfo( macroName, lineInfo, capturedExpression, resultDisposition, secondArg ), m_shouldDebugBreak( false ), m_shouldThrow( false ), m_guardException( false ) @@ -82,7 +77,7 @@ namespace Catch { assert( !isFalseTest( m_assertionInfo.resultDisposition ) ); AssertionResultData data = m_data; data.resultType = ResultWas::Ok; - data.reconstructedExpression = m_assertionInfo.capturedExpression; + data.reconstructedExpression = capturedExpressionWithSecondArgument(m_assertionInfo.capturedExpression, m_assertionInfo.secondArg); std::string actualMessage = Catch::translateActiveException(); if( !matcher.match( actualMessage ) ) { @@ -154,7 +149,7 @@ namespace Catch { } void ResultBuilder::reconstructExpression( std::string& dest ) const { - dest = m_assertionInfo.capturedExpression; + dest = capturedExpressionWithSecondArgument(m_assertionInfo.capturedExpression, m_assertionInfo.secondArg); } void ResultBuilder::setExceptionGuard() { diff --git a/include/internal/catch_run_context.hpp b/include/internal/catch_run_context.hpp index 572b54d1..b3dee17f 100644 --- a/include/internal/catch_run_context.hpp +++ b/include/internal/catch_run_context.hpp @@ -150,7 +150,7 @@ namespace Catch { static_cast(m_reporter->assertionEnded(AssertionStats(result, m_messages, m_totals))); // Reset working state - m_lastAssertionInfo = AssertionInfo( std::string(), m_lastAssertionInfo.lineInfo, "{Unknown expression after the reported line}" , m_lastAssertionInfo.resultDisposition ); + m_lastAssertionInfo = AssertionInfo( "", m_lastAssertionInfo.lineInfo, "{Unknown expression after the reported line}" , m_lastAssertionInfo.resultDisposition ); m_lastResult = result; } @@ -280,7 +280,7 @@ namespace Catch { double duration = 0; m_shouldReportUnexpected = true; try { - m_lastAssertionInfo = AssertionInfo( "TEST_CASE", testCaseInfo.lineInfo, std::string(), ResultDisposition::Normal ); + m_lastAssertionInfo = AssertionInfo( "TEST_CASE", testCaseInfo.lineInfo, "", ResultDisposition::Normal ); seedRng( *m_config ); @@ -332,9 +332,9 @@ namespace Catch { private: ResultBuilder makeUnexpectedResultBuilder() const { - return ResultBuilder( m_lastAssertionInfo.macroName.c_str(), + return ResultBuilder( m_lastAssertionInfo.macroName, m_lastAssertionInfo.lineInfo, - m_lastAssertionInfo.capturedExpression.c_str(), + m_lastAssertionInfo.capturedExpression, m_lastAssertionInfo.resultDisposition ); }