Skip to content

Commit

Permalink
Reland: Optimize Google Test process startup
Browse files Browse the repository at this point in the history
Google Test performs hidden test registration during process
startup. For test binaries that contain a large number of tests, this
registration can be costly. In this CL, we reduce the overhead of
registration via several tactics:

- Treat CodeLocation and FilePath as value types, using std::move to
  pass them around.
- Reduce string copies in various places by either passing std::string
  values via std::move, or passing const-refs to std::string instances.
- Use std::to_string to stringify an int in DefaultParamName rather than
  a std::stringstream.
- Pull some std::string instances out of nested loops in
  ParameterizedTestSuiteInfo::RegisterTests so as to reuse some
  allocations, and replace stringstream with ordinary string appends.
- Use std::unordered_map in UnitTestImpl::GetTestSuite and
  ParameterizedTestSuiteRegistry::GetTestSuitePatternHolder to spend a
  little memory to turn O(N) lookups into constant time lookpus.
- Use range-based for loops in a few places.
- Use emplace-ish methods to add to containers where appropriate.

All together, these changes reduce the overall runtime of a series of 50
death tests in a single Chromium test executable by ~38% due to the
fact that the registration costs are paid in every death test's child
process.

PiperOrigin-RevId: 613833210
Change-Id: I51a262a770edff98ffa1e3b60c4d78a8308f9a9f
  • Loading branch information
Abseil Team authored and copybara-github committed Mar 8, 2024
1 parent 31993df commit e1a38bc
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 144 deletions.
6 changes: 3 additions & 3 deletions googletest/include/gtest/gtest.h
Original file line number Diff line number Diff line change
Expand Up @@ -607,15 +607,15 @@ class GTEST_API_ TestInfo {
friend class internal::UnitTestImpl;
friend class internal::StreamingListenerTest;
friend TestInfo* internal::MakeAndRegisterTestInfo(
const char* test_suite_name, const char* name, const char* type_param,
std::string test_suite_name, const char* name, const char* type_param,
const char* value_param, internal::CodeLocation code_location,
internal::TypeId fixture_class_id, internal::SetUpTestSuiteFunc set_up_tc,
internal::TearDownTestSuiteFunc tear_down_tc,
internal::TestFactoryBase* factory);

// Constructs a TestInfo object. The newly constructed instance assumes
// ownership of the factory object.
TestInfo(const std::string& test_suite_name, const std::string& name,
TestInfo(std::string test_suite_name, std::string name,
const char* a_type_param, // NULL if not a type-parameterized test
const char* a_value_param, // NULL if not a value-parameterized test
internal::CodeLocation a_code_location,
Expand Down Expand Up @@ -683,7 +683,7 @@ class GTEST_API_ TestSuite {
// this is not a type-parameterized test.
// set_up_tc: pointer to the function that sets up the test suite
// tear_down_tc: pointer to the function that tears down the test suite
TestSuite(const char* name, const char* a_type_param,
TestSuite(const std::string& name, const char* a_type_param,
internal::SetUpTestSuiteFunc set_up_tc,
internal::TearDownTestSuiteFunc tear_down_tc);

Expand Down
8 changes: 7 additions & 1 deletion googletest/include/gtest/internal/gtest-filepath.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#define GOOGLETEST_INCLUDE_GTEST_INTERNAL_GTEST_FILEPATH_H_

#include <string>
#include <utility>

#include "gtest/internal/gtest-port.h"
#include "gtest/internal/gtest-string.h"
Expand Down Expand Up @@ -70,15 +71,20 @@ class GTEST_API_ FilePath {
public:
FilePath() : pathname_("") {}
FilePath(const FilePath& rhs) : pathname_(rhs.pathname_) {}
FilePath(FilePath&& rhs) : pathname_(std::move(rhs.pathname_)) {}

explicit FilePath(const std::string& pathname) : pathname_(pathname) {
explicit FilePath(std::string pathname) : pathname_(std::move(pathname)) {
Normalize();
}

FilePath& operator=(const FilePath& rhs) {
Set(rhs);
return *this;
}
FilePath& operator=(FilePath&& rhs) {
pathname_ = std::move(rhs.pathname_);
return *this;
}

void Set(const FilePath& rhs) { pathname_ = rhs.pathname_; }

Expand Down
29 changes: 12 additions & 17 deletions googletest/include/gtest/internal/gtest-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -474,8 +474,8 @@ using SetUpTestSuiteFunc = void (*)();
using TearDownTestSuiteFunc = void (*)();

struct CodeLocation {
CodeLocation(const std::string& a_file, int a_line)
: file(a_file), line(a_line) {}
CodeLocation(std::string a_file, int a_line)
: file(std::move(a_file)), line(a_line) {}

std::string file;
int line;
Expand Down Expand Up @@ -564,7 +564,7 @@ struct SuiteApiResolver : T {
// The newly created TestInfo instance will assume
// ownership of the factory object.
GTEST_API_ TestInfo* MakeAndRegisterTestInfo(
const char* test_suite_name, const char* name, const char* type_param,
std::string test_suite_name, const char* name, const char* type_param,
const char* value_param, CodeLocation code_location,
TypeId fixture_class_id, SetUpTestSuiteFunc set_up_tc,
TearDownTestSuiteFunc tear_down_tc, TestFactoryBase* factory);
Expand Down Expand Up @@ -595,8 +595,7 @@ class GTEST_API_ TypedTestSuitePState {
fflush(stderr);
posix::Abort();
}
registered_tests_.insert(
::std::make_pair(test_name, CodeLocation(file, line)));
registered_tests_.emplace(test_name, CodeLocation(file, line));
return true;
}

Expand Down Expand Up @@ -700,7 +699,7 @@ class TypeParameterizedTest {
// specified in INSTANTIATE_TYPED_TEST_SUITE_P(Prefix, TestSuite,
// Types). Valid values for 'index' are [0, N - 1] where N is the
// length of Types.
static bool Register(const char* prefix, const CodeLocation& code_location,
static bool Register(const char* prefix, CodeLocation code_location,
const char* case_name, const char* test_names, int index,
const std::vector<std::string>& type_names =
GenerateNames<DefaultNameGenerator, Types>()) {
Expand All @@ -712,8 +711,7 @@ class TypeParameterizedTest {
// list.
MakeAndRegisterTestInfo(
(std::string(prefix) + (prefix[0] == '\0' ? "" : "/") + case_name +
"/" + type_names[static_cast<size_t>(index)])
.c_str(),
"/" + type_names[static_cast<size_t>(index)]),
StripTrailingSpaces(GetPrefixUntilComma(test_names)).c_str(),
GetTypeName<Type>().c_str(),
nullptr, // No value parameter.
Expand All @@ -725,21 +723,17 @@ class TypeParameterizedTest {
new TestFactoryImpl<TestClass>);

// Next, recurses (at compile time) with the tail of the type list.
return TypeParameterizedTest<Fixture, TestSel,
typename Types::Tail>::Register(prefix,
code_location,
case_name,
test_names,
index + 1,
type_names);
return TypeParameterizedTest<Fixture, TestSel, typename Types::Tail>::
Register(prefix, std::move(code_location), case_name, test_names,
index + 1, type_names);
}
};

// The base case for the compile time recursion.
template <GTEST_TEMPLATE_ Fixture, class TestSel>
class TypeParameterizedTest<Fixture, TestSel, internal::None> {
public:
static bool Register(const char* /*prefix*/, const CodeLocation&,
static bool Register(const char* /*prefix*/, CodeLocation,
const char* /*case_name*/, const char* /*test_names*/,
int /*index*/,
const std::vector<std::string>& =
Expand Down Expand Up @@ -786,7 +780,8 @@ class TypeParameterizedTestSuite {

// Next, recurses (at compile time) with the tail of the test list.
return TypeParameterizedTestSuite<Fixture, typename Tests::Tail,
Types>::Register(prefix, code_location,
Types>::Register(prefix,
std::move(code_location),
state, case_name,
SkipComma(test_names),
type_names);
Expand Down
Loading

0 comments on commit e1a38bc

Please sign in to comment.