From 584973a485160bea0feb8764c28bb65d03b988ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Mon, 20 Feb 2023 10:22:37 +0100 Subject: [PATCH] Early evaluate line loc in NameAndLoc::operator== I do not know if checking the tracker name or the tracker's file part of the location first would provide better results, but in the common case, the line part of the location check should be rather unique, because different `SECTION`s will have different source lines where they are defined. I also propagated this same check into `ITracker::findChild`, because this significantly improves performance of section tracking in Debug builds -> 10% in macro benchmark heavily focused on section tracking. In Release build there is usually no difference, because the inliner will inline `NameAndLoc::operator==` into `findChild`, and then eliminate the redundant check. (If the inliner decides against, then this still improves the performance on average). --- src/catch2/internal/catch_test_case_tracker.cpp | 7 ++++++- src/catch2/internal/catch_test_case_tracker.hpp | 13 +++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/catch2/internal/catch_test_case_tracker.cpp b/src/catch2/internal/catch_test_case_tracker.cpp index c48932b1..1470b91c 100644 --- a/src/catch2/internal/catch_test_case_tracker.cpp +++ b/src/catch2/internal/catch_test_case_tracker.cpp @@ -43,7 +43,12 @@ namespace TestCaseTracking { m_children.begin(), m_children.end(), [&nameAndLocation]( ITrackerPtr const& tracker ) { - return tracker->nameAndLocation() == nameAndLocation; + auto const& tnameAndLoc = tracker->nameAndLocation(); + if ( tnameAndLoc.location.line != + nameAndLocation.location.line ) { + return false; + } + return tnameAndLoc == nameAndLocation; } ); return ( it != m_children.end() ) ? it->get() : nullptr; } diff --git a/src/catch2/internal/catch_test_case_tracker.hpp b/src/catch2/internal/catch_test_case_tracker.hpp index 506e42de..beff8d6d 100644 --- a/src/catch2/internal/catch_test_case_tracker.hpp +++ b/src/catch2/internal/catch_test_case_tracker.hpp @@ -24,8 +24,12 @@ namespace TestCaseTracking { NameAndLocation( std::string&& _name, SourceLineInfo const& _location ); friend bool operator==(NameAndLocation const& lhs, NameAndLocation const& rhs) { - return lhs.name == rhs.name - && lhs.location == rhs.location; + // This is a very cheap check that should have a very high hit rate. + // If we get to SourceLineInfo::operator==, we will redo it, but the + // cost of repeating is trivial at that point (we will be paying + // multiple strcmp/memcmps at that point). + if ( lhs.location.line != rhs.location.line ) { return false; } + return lhs.name == rhs.name && lhs.location == rhs.location; } friend bool operator!=(NameAndLocation const& lhs, NameAndLocation const& rhs) { @@ -50,6 +54,11 @@ namespace TestCaseTracking { friend bool operator==( NameAndLocation const& lhs, NameAndLocationRef const& rhs ) { + // This is a very cheap check that should have a very high hit rate. + // If we get to SourceLineInfo::operator==, we will redo it, but the + // cost of repeating is trivial at that point (we will be paying + // multiple strcmp/memcmps at that point). + if ( lhs.location.line != rhs.location.line ) { return false; } return StringRef( lhs.name ) == rhs.name && lhs.location == rhs.location; }