Avoid allocations when looking for trackers

Now we delay allocating owning `NameAndLocation` instances until
we construct a new tracker (because a tracker's lifetime can be
significantly different from the underlying tracked-thing's name).

This saves 4239 allocations (436948 -> 432709) when running
`./tests/SelfTest -o /dev/null`, at some cost to code clarity
due to introducing a new ref type, `NameAndLocationRef`.
This commit is contained in:
Martin Hořeňovský 2023-01-27 22:12:57 +01:00
parent 906552f8c8
commit 43f02027e4
No known key found for this signature in database
GPG Key ID: DE48307B8B0D381A
4 changed files with 71 additions and 39 deletions

View File

@ -29,12 +29,12 @@ namespace Catch {
struct GeneratorTracker : TestCaseTracking::TrackerBase, IGeneratorTracker { struct GeneratorTracker : TestCaseTracking::TrackerBase, IGeneratorTracker {
GeneratorBasePtr m_generator; GeneratorBasePtr m_generator;
GeneratorTracker( TestCaseTracking::NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent ) GeneratorTracker( TestCaseTracking::NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent )
: TrackerBase( nameAndLocation, ctx, parent ) : TrackerBase( CATCH_MOVE(nameAndLocation), ctx, parent )
{} {}
~GeneratorTracker() override; ~GeneratorTracker() override;
static GeneratorTracker* acquire( TrackerContext& ctx, TestCaseTracking::NameAndLocation const& nameAndLocation ) { static GeneratorTracker* acquire( TrackerContext& ctx, TestCaseTracking::NameAndLocationRef nameAndLocation ) {
GeneratorTracker* tracker; GeneratorTracker* tracker;
ITracker& currentTracker = ctx.currentTracker(); ITracker& currentTracker = ctx.currentTracker();
@ -226,7 +226,7 @@ namespace Catch {
uint64_t testRuns = 0; uint64_t testRuns = 0;
do { do {
m_trackerContext.startCycle(); m_trackerContext.startCycle();
m_testCaseTracker = &SectionTracker::acquire(m_trackerContext, TestCaseTracking::NameAndLocation(testInfo.name, testInfo.lineInfo)); m_testCaseTracker = &SectionTracker::acquire(m_trackerContext, TestCaseTracking::NameAndLocationRef(testInfo.name, testInfo.lineInfo));
m_reporter->testCasePartialStarting(testInfo, testRuns); m_reporter->testCasePartialStarting(testInfo, testRuns);
@ -298,7 +298,8 @@ namespace Catch {
} }
bool RunContext::sectionStarted(SectionInfo const & sectionInfo, Counts & assertions) { bool RunContext::sectionStarted(SectionInfo const & sectionInfo, Counts & assertions) {
ITracker& sectionTracker = SectionTracker::acquire(m_trackerContext, TestCaseTracking::NameAndLocation(sectionInfo.name, sectionInfo.lineInfo)); ITracker& sectionTracker = SectionTracker::acquire(m_trackerContext, TestCaseTracking::NameAndLocationRef(sectionInfo.name, sectionInfo.lineInfo));
if (!sectionTracker.isOpen()) if (!sectionTracker.isOpen())
return false; return false;
m_activeSections.push_back(&sectionTracker); m_activeSections.push_back(&sectionTracker);
@ -317,8 +318,8 @@ namespace Catch {
using namespace Generators; using namespace Generators;
GeneratorTracker* tracker = GeneratorTracker::acquire( GeneratorTracker* tracker = GeneratorTracker::acquire(
m_trackerContext, m_trackerContext,
TestCaseTracking::NameAndLocation( TestCaseTracking::NameAndLocationRef(
static_cast<std::string>( generatorName ), lineInfo ) ); generatorName, lineInfo ) );
m_lastAssertionInfo.lineInfo = lineInfo; m_lastAssertionInfo.lineInfo = lineInfo;
return tracker; return tracker;
} }
@ -335,7 +336,7 @@ namespace Catch {
"Trying to create tracker for a genreator that already has one" ); "Trying to create tracker for a genreator that already has one" );
auto newTracker = Catch::Detail::make_unique<Generators::GeneratorTracker>( auto newTracker = Catch::Detail::make_unique<Generators::GeneratorTracker>(
nameAndLoc, m_trackerContext, &currentTracker ); CATCH_MOVE(nameAndLoc), m_trackerContext, &currentTracker );
auto ret = newTracker.get(); auto ret = newTracker.get();
currentTracker.addChild( CATCH_MOVE( newTracker ) ); currentTracker.addChild( CATCH_MOVE( newTracker ) );

View File

@ -22,8 +22,8 @@
namespace Catch { namespace Catch {
namespace TestCaseTracking { namespace TestCaseTracking {
NameAndLocation::NameAndLocation( std::string const& _name, SourceLineInfo const& _location ) NameAndLocation::NameAndLocation( std::string&& _name, SourceLineInfo const& _location )
: name( _name ), : name( CATCH_MOVE(_name) ),
location( _location ) location( _location )
{} {}
@ -38,14 +38,12 @@ namespace TestCaseTracking {
m_children.push_back( CATCH_MOVE(child) ); m_children.push_back( CATCH_MOVE(child) );
} }
ITracker* ITracker::findChild( NameAndLocation const& nameAndLocation ) { ITracker* ITracker::findChild( NameAndLocationRef nameAndLocation ) {
auto it = std::find_if( auto it = std::find_if(
m_children.begin(), m_children.begin(),
m_children.end(), m_children.end(),
[&nameAndLocation]( ITrackerPtr const& tracker ) { [&nameAndLocation]( ITrackerPtr const& tracker ) {
return tracker->nameAndLocation().location == return tracker->nameAndLocation() == nameAndLocation;
nameAndLocation.location &&
tracker->nameAndLocation().name == nameAndLocation.name;
} ); } );
return ( it != m_children.end() ) ? it->get() : nullptr; return ( it != m_children.end() ) ? it->get() : nullptr;
} }
@ -108,8 +106,8 @@ namespace TestCaseTracking {
} }
TrackerBase::TrackerBase( NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent ): TrackerBase::TrackerBase( NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent ):
ITracker(nameAndLocation, parent), ITracker(CATCH_MOVE(nameAndLocation), parent),
m_ctx( ctx ) m_ctx( ctx )
{} {}
@ -169,13 +167,14 @@ namespace TestCaseTracking {
m_ctx.setCurrentTracker( this ); m_ctx.setCurrentTracker( this );
} }
SectionTracker::SectionTracker( NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent ) SectionTracker::SectionTracker( NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent )
: TrackerBase( nameAndLocation, ctx, parent ), : TrackerBase( CATCH_MOVE(nameAndLocation), ctx, parent ),
m_trimmed_name(trim(nameAndLocation.name)) m_trimmed_name(trim(ITracker::nameAndLocation().name))
{ {
if( parent ) { if( parent ) {
while( !parent->isSectionTracker() ) while ( !parent->isSectionTracker() ) {
parent = parent->parent(); parent = parent->parent();
}
SectionTracker& parentSection = static_cast<SectionTracker&>( *parent ); SectionTracker& parentSection = static_cast<SectionTracker&>( *parent );
addNextFilters( parentSection.m_filters ); addNextFilters( parentSection.m_filters );
@ -195,24 +194,30 @@ namespace TestCaseTracking {
bool SectionTracker::isSectionTracker() const { return true; } bool SectionTracker::isSectionTracker() const { return true; }
SectionTracker& SectionTracker::acquire( TrackerContext& ctx, NameAndLocation const& nameAndLocation ) { SectionTracker& SectionTracker::acquire( TrackerContext& ctx, NameAndLocationRef nameAndLocation ) {
SectionTracker* section; SectionTracker* tracker;
ITracker& currentTracker = ctx.currentTracker(); ITracker& currentTracker = ctx.currentTracker();
if ( ITracker* childTracker = if ( ITracker* childTracker =
currentTracker.findChild( nameAndLocation ) ) { currentTracker.findChild( nameAndLocation ) ) {
assert( childTracker ); assert( childTracker );
assert( childTracker->isSectionTracker() ); assert( childTracker->isSectionTracker() );
section = static_cast<SectionTracker*>( childTracker ); tracker = static_cast<SectionTracker*>( childTracker );
} else { } else {
auto newSection = Catch::Detail::make_unique<SectionTracker>( auto newTracker = Catch::Detail::make_unique<SectionTracker>(
nameAndLocation, ctx, &currentTracker ); NameAndLocation{ static_cast<std::string>(nameAndLocation.name),
section = newSection.get(); nameAndLocation.location },
currentTracker.addChild( CATCH_MOVE( newSection ) ); ctx,
&currentTracker );
tracker = newTracker.get();
currentTracker.addChild( CATCH_MOVE( newTracker ) );
} }
if( !ctx.completedCycle() )
section->tryOpen(); if ( !ctx.completedCycle() ) {
return *section; tracker->tryOpen();
}
return *tracker;
} }
void SectionTracker::tryOpen() { void SectionTracker::tryOpen() {

View File

@ -22,7 +22,7 @@ namespace TestCaseTracking {
std::string name; std::string name;
SourceLineInfo location; SourceLineInfo location;
NameAndLocation( std::string const& _name, SourceLineInfo const& _location ); NameAndLocation( std::string&& _name, SourceLineInfo const& _location );
friend bool operator==(NameAndLocation const& lhs, NameAndLocation const& rhs) { friend bool operator==(NameAndLocation const& lhs, NameAndLocation const& rhs) {
return lhs.name == rhs.name return lhs.name == rhs.name
&& lhs.location == rhs.location; && lhs.location == rhs.location;
@ -33,6 +33,32 @@ namespace TestCaseTracking {
} }
}; };
/**
* This is a variant of `NameAndLocation` that does not own the name string
*
* This avoids extra allocations when trying to locate a tracker by its
* name and location, as long as we make sure that trackers only keep
* around the owning variant.
*/
struct NameAndLocationRef {
StringRef name;
SourceLineInfo location;
constexpr NameAndLocationRef( StringRef name_,
SourceLineInfo location_ ):
name( name_ ), location( location_ ) {}
friend bool operator==( NameAndLocation const& lhs,
NameAndLocationRef rhs ) {
return StringRef( lhs.name ) == rhs.name &&
lhs.location == rhs.location;
}
friend bool operator==( NameAndLocationRef lhs,
NameAndLocation const& rhs ) {
return rhs == lhs;
}
};
class ITracker; class ITracker;
using ITrackerPtr = Catch::Detail::unique_ptr<ITracker>; using ITrackerPtr = Catch::Detail::unique_ptr<ITracker>;
@ -57,8 +83,8 @@ namespace TestCaseTracking {
CycleState m_runState = NotStarted; CycleState m_runState = NotStarted;
public: public:
ITracker( NameAndLocation const& nameAndLoc, ITracker* parent ): ITracker( NameAndLocation&& nameAndLoc, ITracker* parent ):
m_nameAndLocation( nameAndLoc ), m_nameAndLocation( CATCH_MOVE(nameAndLoc) ),
m_parent( parent ) m_parent( parent )
{} {}
@ -97,7 +123,7 @@ namespace TestCaseTracking {
* *
* Returns nullptr if not found. * Returns nullptr if not found.
*/ */
ITracker* findChild( NameAndLocation const& nameAndLocation ); ITracker* findChild( NameAndLocationRef nameAndLocation );
//! Have any children been added? //! Have any children been added?
bool hasChildren() const { bool hasChildren() const {
return !m_children.empty(); return !m_children.empty();
@ -154,7 +180,7 @@ namespace TestCaseTracking {
TrackerContext& m_ctx; TrackerContext& m_ctx;
public: public:
TrackerBase( NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent ); TrackerBase( NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent );
bool isComplete() const override; bool isComplete() const override;
@ -172,13 +198,13 @@ namespace TestCaseTracking {
std::vector<StringRef> m_filters; std::vector<StringRef> m_filters;
std::string m_trimmed_name; std::string m_trimmed_name;
public: public:
SectionTracker( NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent ); SectionTracker( NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent );
bool isSectionTracker() const override; bool isSectionTracker() const override;
bool isComplete() const override; bool isComplete() const override;
static SectionTracker& acquire( TrackerContext& ctx, NameAndLocation const& nameAndLocation ); static SectionTracker& acquire( TrackerContext& ctx, NameAndLocationRef nameAndLocation );
void tryOpen(); void tryOpen();

View File

@ -14,8 +14,8 @@
using namespace Catch; using namespace Catch;
namespace { namespace {
Catch::TestCaseTracking::NameAndLocation makeNAL( std::string const& name ) { Catch::TestCaseTracking::NameAndLocationRef makeNAL( StringRef name ) {
return Catch::TestCaseTracking::NameAndLocation( name, Catch::SourceLineInfo("",0) ); return Catch::TestCaseTracking::NameAndLocationRef( name, Catch::SourceLineInfo("",0) );
} }
} }