Skip to content

Commit eb46a63

Browse files
rogeeffcopybara-github
rogeeff
authored and
copybara-github
committed
Optimize the absl::GetFlag cost for most non built-in flag types (including string).
PiperOrigin-RevId: 649200175 Change-Id: Ic6741d9fe5e0b1853ed8bb37b585d38b51d15581
1 parent 6e70150 commit eb46a63

File tree

5 files changed

+470
-82
lines changed

5 files changed

+470
-82
lines changed

‎absl/flags/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,7 @@ cc_test(
390390
":flag",
391391
":flag_internal",
392392
":marshalling",
393+
":parse",
393394
":reflection",
394395
"//absl/base:core_headers",
395396
"//absl/base:malloc_internal",

‎absl/flags/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ absl_cc_test(
340340
absl::flags_config
341341
absl::flags_internal
342342
absl::flags_marshalling
343+
absl::flags_parse
343344
absl::flags_reflection
344345
absl::int128
345346
absl::optional

‎absl/flags/flag_test.cc

Lines changed: 173 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "absl/flags/declare.h"
3232
#include "absl/flags/internal/flag.h"
3333
#include "absl/flags/marshalling.h"
34+
#include "absl/flags/parse.h"
3435
#include "absl/flags/reflection.h"
3536
#include "absl/flags/usage_config.h"
3637
#include "absl/numeric/int128.h"
@@ -126,9 +127,9 @@ TEST_F(FlagTest, Traits) {
126127
#endif
127128

128129
EXPECT_EQ(flags::StorageKind<std::string>(),
129-
flags::FlagValueStorageKind::kAlignedBuffer);
130+
flags::FlagValueStorageKind::kHeapAllocated);
130131
EXPECT_EQ(flags::StorageKind<std::vector<std::string>>(),
131-
flags::FlagValueStorageKind::kAlignedBuffer);
132+
flags::FlagValueStorageKind::kHeapAllocated);
132133

133134
EXPECT_EQ(flags::StorageKind<absl::int128>(),
134135
flags::FlagValueStorageKind::kSequenceLocked);
@@ -267,7 +268,7 @@ TEST_F(FlagTest, TestFlagDeclaration) {
267268
#if ABSL_FLAGS_STRIP_NAMES
268269
// The intent of this helper struct and an expression below is to make sure that
269270
// in the configuration where ABSL_FLAGS_STRIP_NAMES=1 registrar construction
270-
// (in cases of of no Tail calls like OnUpdate) is constexpr and thus can and
271+
// (in cases of no Tail calls like OnUpdate) is constexpr and thus can and
271272
// should be completely optimized away, thus avoiding the cost/overhead of
272273
// static initializers.
273274
struct VerifyConsteval {
@@ -515,8 +516,10 @@ TEST_F(FlagTest, TestDefault) {
515516

516517
struct NonTriviallyCopyableAggregate {
517518
NonTriviallyCopyableAggregate() = default;
519+
// NOLINTNEXTLINE
518520
NonTriviallyCopyableAggregate(const NonTriviallyCopyableAggregate& rhs)
519521
: value(rhs.value) {}
522+
// NOLINTNEXTLINE
520523
NonTriviallyCopyableAggregate& operator=(
521524
const NonTriviallyCopyableAggregate& rhs) {
522525
value = rhs.value;
@@ -975,51 +978,194 @@ bool AbslParseFlag(absl::string_view, SmallAlignUDT*, std::string*) {
975978
}
976979
std::string AbslUnparseFlag(const SmallAlignUDT&) { return ""; }
977980

978-
// User-defined type with small size, but not trivially copyable.
981+
} // namespace
982+
983+
ABSL_FLAG(SmallAlignUDT, test_flag_sa_udt, {}, "help");
984+
985+
namespace {
986+
987+
TEST_F(FlagTest, TestSmallAlignUDT) {
988+
EXPECT_EQ(flags::StorageKind<SmallAlignUDT>(),
989+
flags::FlagValueStorageKind::kSequenceLocked);
990+
SmallAlignUDT value = absl::GetFlag(FLAGS_test_flag_sa_udt);
991+
EXPECT_EQ(value.c, 'A');
992+
EXPECT_EQ(value.s, 12);
993+
994+
value.c = 'B';
995+
value.s = 45;
996+
absl::SetFlag(&FLAGS_test_flag_sa_udt, value);
997+
value = absl::GetFlag(FLAGS_test_flag_sa_udt);
998+
EXPECT_EQ(value.c, 'B');
999+
EXPECT_EQ(value.s, 45);
1000+
}
1001+
} // namespace
1002+
1003+
// --------------------------------------------------------------------
1004+
1005+
namespace {
1006+
1007+
// User-defined not trivially copyable type.
1008+
template <int id>
9791009
struct NonTriviallyCopyableUDT {
980-
NonTriviallyCopyableUDT() : c('A') {}
981-
NonTriviallyCopyableUDT(const NonTriviallyCopyableUDT& rhs) : c(rhs.c) {}
1010+
NonTriviallyCopyableUDT() : c('A') { s_num_instance++; }
1011+
NonTriviallyCopyableUDT(const NonTriviallyCopyableUDT& rhs) : c(rhs.c) {
1012+
s_num_instance++;
1013+
}
9821014
NonTriviallyCopyableUDT& operator=(const NonTriviallyCopyableUDT& rhs) {
9831015
c = rhs.c;
9841016
return *this;
9851017
}
1018+
~NonTriviallyCopyableUDT() { s_num_instance--; }
9861019

1020+
static uint64_t s_num_instance;
9871021
char c;
9881022
};
9891023

990-
bool AbslParseFlag(absl::string_view, NonTriviallyCopyableUDT*, std::string*) {
1024+
template <int id>
1025+
uint64_t NonTriviallyCopyableUDT<id>::s_num_instance = 0;
1026+
1027+
template <int id>
1028+
bool AbslParseFlag(absl::string_view txt, NonTriviallyCopyableUDT<id>* f,
1029+
std::string*) {
1030+
f->c = txt.empty() ? '\0' : txt[0];
9911031
return true;
9921032
}
993-
std::string AbslUnparseFlag(const NonTriviallyCopyableUDT&) { return ""; }
1033+
template <int id>
1034+
std::string AbslUnparseFlag(const NonTriviallyCopyableUDT<id>&) {
1035+
return "";
1036+
}
9941037

1038+
template <int id, typename F>
1039+
void TestExpectedLeaks(
1040+
F&& f, uint64_t num_leaks,
1041+
absl::optional<uint64_t> num_new_instances = absl::nullopt) {
1042+
if (!num_new_instances.has_value()) num_new_instances = num_leaks;
1043+
1044+
auto num_leaked_before = flags::NumLeakedFlagValues();
1045+
auto num_instances_before = NonTriviallyCopyableUDT<id>::s_num_instance;
1046+
f();
1047+
EXPECT_EQ(num_leaked_before + num_leaks, flags::NumLeakedFlagValues());
1048+
EXPECT_EQ(num_instances_before + num_new_instances.value(),
1049+
NonTriviallyCopyableUDT<id>::s_num_instance);
1050+
}
9951051
} // namespace
9961052

997-
ABSL_FLAG(SmallAlignUDT, test_flag_sa_udt, {}, "help");
998-
ABSL_FLAG(NonTriviallyCopyableUDT, test_flag_ntc_udt, {}, "help");
1053+
ABSL_FLAG(NonTriviallyCopyableUDT<1>, test_flag_ntc_udt1, {}, "help");
1054+
ABSL_FLAG(NonTriviallyCopyableUDT<2>, test_flag_ntc_udt2, {}, "help");
1055+
ABSL_FLAG(NonTriviallyCopyableUDT<3>, test_flag_ntc_udt3, {}, "help");
1056+
ABSL_FLAG(NonTriviallyCopyableUDT<4>, test_flag_ntc_udt4, {}, "help");
1057+
ABSL_FLAG(NonTriviallyCopyableUDT<5>, test_flag_ntc_udt5, {}, "help");
9991058

10001059
namespace {
10011060

1002-
TEST_F(FlagTest, TestSmallAlignUDT) {
1003-
SmallAlignUDT value = absl::GetFlag(FLAGS_test_flag_sa_udt);
1004-
EXPECT_EQ(value.c, 'A');
1005-
EXPECT_EQ(value.s, 12);
1061+
TEST_F(FlagTest, TestNonTriviallyCopyableGetSetSet) {
1062+
EXPECT_EQ(flags::StorageKind<NonTriviallyCopyableUDT<1>>(),
1063+
flags::FlagValueStorageKind::kHeapAllocated);
1064+
1065+
TestExpectedLeaks<1>(
1066+
[&] {
1067+
NonTriviallyCopyableUDT<1> value =
1068+
absl::GetFlag(FLAGS_test_flag_ntc_udt1);
1069+
EXPECT_EQ(value.c, 'A');
1070+
},
1071+
0);
1072+
1073+
TestExpectedLeaks<1>(
1074+
[&] {
1075+
NonTriviallyCopyableUDT<1> value;
1076+
value.c = 'B';
1077+
absl::SetFlag(&FLAGS_test_flag_ntc_udt1, value);
1078+
EXPECT_EQ(value.c, 'B');
1079+
},
1080+
1);
1081+
1082+
TestExpectedLeaks<1>(
1083+
[&] {
1084+
NonTriviallyCopyableUDT<1> value;
1085+
value.c = 'C';
1086+
absl::SetFlag(&FLAGS_test_flag_ntc_udt1, value);
1087+
},
1088+
0);
1089+
}
10061090

1007-
value.c = 'B';
1008-
value.s = 45;
1009-
absl::SetFlag(&FLAGS_test_flag_sa_udt, value);
1010-
value = absl::GetFlag(FLAGS_test_flag_sa_udt);
1011-
EXPECT_EQ(value.c, 'B');
1012-
EXPECT_EQ(value.s, 45);
1091+
TEST_F(FlagTest, TestNonTriviallyCopyableParseSet) {
1092+
TestExpectedLeaks<2>(
1093+
[&] {
1094+
const char* in_argv[] = {"testbin", "--test_flag_ntc_udt2=A"};
1095+
absl::ParseCommandLine(2, const_cast<char**>(in_argv));
1096+
},
1097+
0);
1098+
1099+
TestExpectedLeaks<2>(
1100+
[&] {
1101+
NonTriviallyCopyableUDT<2> value;
1102+
value.c = 'B';
1103+
absl::SetFlag(&FLAGS_test_flag_ntc_udt2, value);
1104+
EXPECT_EQ(value.c, 'B');
1105+
},
1106+
0);
10131107
}
10141108

1015-
TEST_F(FlagTest, TestNonTriviallyCopyableUDT) {
1016-
NonTriviallyCopyableUDT value = absl::GetFlag(FLAGS_test_flag_ntc_udt);
1017-
EXPECT_EQ(value.c, 'A');
1109+
TEST_F(FlagTest, TestNonTriviallyCopyableSet) {
1110+
TestExpectedLeaks<3>(
1111+
[&] {
1112+
NonTriviallyCopyableUDT<3> value;
1113+
value.c = 'B';
1114+
absl::SetFlag(&FLAGS_test_flag_ntc_udt3, value);
1115+
EXPECT_EQ(value.c, 'B');
1116+
},
1117+
0);
1118+
}
10181119

1019-
value.c = 'B';
1020-
absl::SetFlag(&FLAGS_test_flag_ntc_udt, value);
1021-
value = absl::GetFlag(FLAGS_test_flag_ntc_udt);
1022-
EXPECT_EQ(value.c, 'B');
1120+
// One new instance created during initialization and stored in the flag.
1121+
auto premain_utd4_get =
1122+
(TestExpectedLeaks<4>([] { (void)absl::GetFlag(FLAGS_test_flag_ntc_udt4); },
1123+
0, 1),
1124+
false);
1125+
1126+
TEST_F(FlagTest, TestNonTriviallyCopyableGetBeforeMainParseGet) {
1127+
TestExpectedLeaks<4>(
1128+
[&] {
1129+
const char* in_argv[] = {"testbin", "--test_flag_ntc_udt4=C"};
1130+
absl::ParseCommandLine(2, const_cast<char**>(in_argv));
1131+
},
1132+
1);
1133+
1134+
TestExpectedLeaks<4>(
1135+
[&] {
1136+
NonTriviallyCopyableUDT<4> value =
1137+
absl::GetFlag(FLAGS_test_flag_ntc_udt4);
1138+
EXPECT_EQ(value.c, 'C');
1139+
},
1140+
0);
1141+
}
1142+
1143+
// One new instance created during initialization, which is reused since it was
1144+
// never read.
1145+
auto premain_utd5_set = (TestExpectedLeaks<5>(
1146+
[] {
1147+
NonTriviallyCopyableUDT<5> value;
1148+
value.c = 'B';
1149+
absl::SetFlag(&FLAGS_test_flag_ntc_udt5, value);
1150+
},
1151+
0, 1),
1152+
false);
1153+
1154+
TEST_F(FlagTest, TestNonTriviallyCopyableSetParseGet) {
1155+
TestExpectedLeaks<5>(
1156+
[&] {
1157+
const char* in_argv[] = {"testbin", "--test_flag_ntc_udt5=C"};
1158+
absl::ParseCommandLine(2, const_cast<char**>(in_argv));
1159+
},
1160+
0);
1161+
1162+
TestExpectedLeaks<5>(
1163+
[&] {
1164+
NonTriviallyCopyableUDT<5> value =
1165+
absl::GetFlag(FLAGS_test_flag_ntc_udt5);
1166+
EXPECT_EQ(value.c, 'C');
1167+
},
1168+
0);
10231169
}
10241170

10251171
} // namespace

0 commit comments

Comments
 (0)