Skip to content

Commit e44d95f

Browse files
DiegoKrupitzaDiegoKrupitza
committed
Incorporate pull request feedback
1 parent ba711a7 commit e44d95f

File tree

6 files changed

+121
-98
lines changed

6 files changed

+121
-98
lines changed

‎src/benchmark_api_internal.cc‎

Lines changed: 9 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -47,19 +47,12 @@ BenchmarkInstance::BenchmarkInstance(Benchmark* benchmark, int family_idx,
4747
}
4848

4949
// formatting args either in default mode or in human-readable
50-
auto formattedArgument = StrFormat("%" PRId64, arg);
51-
5250
if (FLAGS_benchmark_human_readable) {
53-
if ((arg & (arg - 1)) == 0 && (arg > 1)) {
54-
// power of two human format
55-
formattedArgument = StrFormat("2^%.0f", std::log2(arg));
56-
} else if (arg % 10 == 0 && (arg > 0)) {
57-
// base 10 human format
58-
formattedArgument = Base10HumanReadableFormat::get(arg);
59-
}
51+
name_.args += FormatHumanReadable(arg);
52+
} else {
53+
name_.args += StrFormat("%" PRId64, arg);
6054
}
6155

62-
name_.args += formattedArgument;
6356
++arg_i;
6457
}
6558

@@ -131,53 +124,12 @@ void BenchmarkInstance::Teardown() const {
131124
}
132125
}
133126

134-
const std::array<Base10HumanReadableFormat, 5>
135-
Base10HumanReadableFormat::ranges = {
136-
Base10HumanReadableFormat{1, ""}, // from 0 to 10^3-1
137-
Base10HumanReadableFormat{1000, "k"}, // from 10^3 to 10^6-1
138-
Base10HumanReadableFormat{1000000, "m"}, // from 10^6 to 10^9-1
139-
Base10HumanReadableFormat{1000000000, "bn"}, // from 10^9 to 10^12-1
140-
Base10HumanReadableFormat{1000000000000, ""} // above show as full
141-
};
142-
143-
std::string Base10HumanReadableFormat::get(const int64_t& arg) {
144-
// binary search for the best match
145-
std::size_t left = 0;
146-
std::size_t right = ranges.size() - 1;
147-
148-
while (left <= right) {
149-
const std::size_t middle = left + std::floor((right - left) / 2);
150-
const auto& middle_val = ranges[middle].entry_val_;
151-
152-
if (middle_val == arg) {
153-
left = middle;
154-
break;
155-
}
156-
157-
if (arg < middle_val) {
158-
right = middle - 1;
159-
} else {
160-
left = middle + 1;
161-
}
162-
}
163-
164-
const auto min_index = std::min(left, right);
165-
166-
// in case we get the "above" we return the '1' just for simplicity.
167-
// because we use the "entry_val_" in later stages for division.
168-
const auto& readable_format =
169-
ranges[(min_index == ranges.size() - 1 ? 0 : min_index)];
170-
171-
// if it is not a perfect match we return the value
172-
// e.g. 1030 we do not want 10k we want 1030
173-
if (arg % readable_format.entry_val_ != 0) {
174-
return StrFormat("%" PRId64, arg);
175-
}
176-
177-
// for everything else we want the abbreviation
178-
const auto reduced_val = arg / readable_format.entry_val_;
179-
return StrFormat("%" PRId64 "%s", reduced_val,
180-
readable_format.abbreviation_.c_str());
127+
/**
128+
* Check whether the given value is a power of two or not.
129+
* @param val the value to check
130+
*/
131+
bool IsPowerOfTwo(const int64_t& val) {
132+
return (val & (val - 1)) == 0 && (val > 1);
181133
}
182134

183135
} // namespace internal

‎src/benchmark_api_internal.h‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,24 +17,6 @@
1717
namespace benchmark {
1818
namespace internal {
1919

20-
struct Base10HumanReadableFormat {
21-
const int64_t entry_val_;
22-
const std::string abbreviation_;
23-
24-
// holds all the possible abbreviation limits, easily expandable by adding a
25-
// new line. Keep in mind they need to be in ascending order such that binary
26-
// search works.
27-
const static std::array<Base10HumanReadableFormat, 5> ranges;
28-
29-
/**
30-
* Gets the human readable format for a given base10 value.
31-
* In other words converts 1_000 to 1k, 40_000_000 to 40m etc
32-
* @param arg the positive value to convert
33-
* @return human readable formatted string
34-
*/
35-
static std::string get(const int64_t& arg);
36-
};
37-
3820
// Information kept per benchmark we may want to run
3921
class BenchmarkInstance {
4022
public:

‎src/string_util.cc‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "string_util.h"
22

33
#include <array>
4+
#include <cinttypes>
45
#ifdef BENCHMARK_STL_ANDROID_GNUSTL
56
#include <cerrno>
67
#endif
@@ -15,6 +16,11 @@
1516
namespace benchmark {
1617
namespace {
1718

19+
// Thousands, Millions, Billions, Trillions, Quadrillions, Quintillions,
20+
// Sextillions, Septillions.
21+
const std::array<std::string, 8> base10Units = {"k", "M", "B", "T",
22+
"Q", "Qi", "Sx", "Sp"};
23+
1824
// kilo, Mega, Giga, Tera, Peta, Exa, Zetta, Yotta.
1925
const char kBigSIUnits[] = "kMGTPEZY";
2026
// Kibi, Mebi, Gibi, Tebi, Pebi, Exbi, Zebi, Yobi.
@@ -32,7 +38,8 @@ static const int64_t kUnitsSize = arraysize(kBigSIUnits);
3238

3339
void ToExponentAndMantissa(double val, double thresh, int precision,
3440
double one_k, std::string* mantissa,
35-
int64_t* exponent) {
41+
int64_t* exponent,
42+
bool inclusiveBigThreshhold = false) {
3643
std::stringstream mantissa_stream;
3744

3845
if (val < 0) {
@@ -44,7 +51,8 @@ void ToExponentAndMantissa(double val, double thresh, int precision,
4451
// in 'precision' digits.
4552
const double adjusted_threshold =
4653
std::max(thresh, 1.0 / std::pow(10.0, precision));
47-
const double big_threshold = adjusted_threshold * one_k;
54+
const double big_threshold =
55+
(adjusted_threshold * one_k) - inclusiveBigThreshhold;
4856
const double small_threshold = adjusted_threshold;
4957
// Values in ]simple_threshold,small_threshold[ will be printed as-is
5058
const double simple_threshold = 0.01;
@@ -262,4 +270,35 @@ double stod(const std::string& str, size_t* pos) {
262270
}
263271
#endif
264272

273+
std::string ExponentToBase10Prefix(int64_t exponent) {
274+
if (exponent == 0) return "";
275+
276+
const int64_t index = (exponent > 0 ? exponent - 1 : -exponent - 1);
277+
if (index >= kUnitsSize) return "";
278+
279+
return base10Units[index];
280+
}
281+
282+
std::string Base10HumanReadableFormat(const int64_t& arg) {
283+
std::string mantissa;
284+
int64_t exponent;
285+
ToExponentAndMantissa(arg, 1, 1, 1000, &mantissa, &exponent, true);
286+
return mantissa + ExponentToBase10Prefix(exponent);
287+
}
288+
289+
bool IsPowerOfTwo(const int64_t& val) {
290+
return (val & (val - 1)) == 0 && (val > 1);
291+
}
292+
293+
std::string Base2HumanReadableFormat(const int64_t& arg) {
294+
return StrFormat("2^%.0f", std::log2(arg));
295+
}
296+
297+
std::string FormatHumanReadable(const int64_t& arg) {
298+
if (IsPowerOfTwo(arg)) {
299+
return Base2HumanReadableFormat(arg);
300+
}
301+
return Base10HumanReadableFormat(arg);
302+
}
303+
265304
} // end namespace benchmark

‎src/string_util.h‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,35 @@ using std::stoul; // NOLINT(misc-unused-using-decls)
6565
#endif
6666
// NOLINTEND
6767

68+
/**
69+
* Check whether the given value is a power of two or not.
70+
* @param val the value to check
71+
*/
72+
bool IsPowerOfTwo(const int64_t& val);
73+
74+
/**
75+
* Gets the human readable format for a given base10 value.
76+
* In other words converts 1_000 to 1k, 40_000_000 to 40m etc
77+
* @param arg the positive value to convert
78+
* @return human readable formatted string
79+
*/
80+
std::string Base10HumanReadableFormat(const int64_t& arg);
81+
82+
/**
83+
* Gets the human readable format for a given base2 value.
84+
* In other words converts 64 to 2^6, 1024 to 2^10 etc
85+
* @param arg the positive value to convert
86+
* @return human readable formatted string
87+
*/
88+
std::string Base2HumanReadableFormat(const int64_t& arg);
89+
90+
/**
91+
* Formats an argument into a human readable format.
92+
* @param arg the argument to format
93+
* @return the argument formatted as human readable
94+
*/
95+
std::string FormatHumanReadable(const int64_t& arg);
96+
6897
} // end namespace benchmark
6998

7099
#endif // BENCHMARK_STRING_UTIL_H_

‎test/human_readable_formatting_test.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ void BM_non_base_two_args(benchmark::State& state) {
5858
for (auto _ : state) {
5959
}
6060
}
61-
BENCHMARK(BM_non_base_two_args)->Arg(9)->Arg(19)->Arg(24)->Arg(1023);
62-
ADD_CASES("BM_non_base_two_args", {{"/9"}, {"/19"}, {"/24"}, {"/1023"}});
61+
BENCHMARK(BM_non_base_two_args)->Arg(9)->Arg(19)->Arg(24)->Arg(65);
62+
ADD_CASES("BM_non_base_two_args", {{"/9"}, {"/19"}, {"/24"}, {"/65"}});
6363

6464
// ============== test case we base 2 args ============== //
6565
void BM_base_two_args(benchmark::State& state) {
@@ -98,8 +98,8 @@ ADD_CASES("BM_base_ten_args", {{"/1"},
9898
{"/10k"},
9999
{"/32k"},
100100
{"/100k"},
101-
{"/1m"},
102-
{"/1bn"}});
101+
{"/1M"},
102+
{"/1B"}});
103103

104104
int main(int argc, char* argv[]) {
105105
benchmark::Initialize(&argc, argv);

‎test/human_readable_gtest.cc‎

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,47 +2,68 @@
22
// human_readable_test - Unit tests for human readable converters
33
//===---------------------------------------------------------------------===//
44

5-
#include "../src/benchmark_api_internal.h"
5+
#include "../src/string_util.h"
66
#include "gtest/gtest.h"
77

88
namespace {
9+
10+
TEST(HumanReadableTest, base2) {
11+
for (int i = 2; i < 10; ++i) {
12+
const auto res = benchmark::Base2HumanReadableFormat(1 << i);
13+
14+
const std::string expected = "2^" + std::to_string(i);
15+
EXPECT_STREQ(res.c_str(), expected.c_str());
16+
}
17+
}
18+
919
TEST(HumanReadableTest, base10) {
10-
using Base10Human = benchmark::internal::Base10HumanReadableFormat;
1120
{
12-
const auto res = Base10Human::get(10000);
21+
const auto res = benchmark::Base10HumanReadableFormat(100);
22+
EXPECT_STREQ(res.c_str(), "100");
23+
}
24+
{
25+
const auto res = benchmark::Base10HumanReadableFormat(1000);
26+
EXPECT_STREQ(res.c_str(), "1k");
27+
}
28+
{
29+
const auto res = benchmark::Base10HumanReadableFormat(10000);
1330
EXPECT_STREQ(res.c_str(), "10k");
1431
}
1532
{
16-
const auto res = Base10Human::get(20000);
33+
const auto res = benchmark::Base10HumanReadableFormat(20000);
1734
EXPECT_STREQ(res.c_str(), "20k");
1835
}
1936
{
20-
const auto res = Base10Human::get(32000);
37+
const auto res = benchmark::Base10HumanReadableFormat(32000);
2138
EXPECT_STREQ(res.c_str(), "32k");
2239
}
2340
{
24-
const auto res = Base10Human::get(100);
25-
EXPECT_STREQ(res.c_str(), "100");
41+
const auto res = benchmark::Base10HumanReadableFormat(1000000);
42+
EXPECT_STREQ(res.c_str(), "1M");
43+
}
44+
{
45+
const auto res = benchmark::Base10HumanReadableFormat(42000000);
46+
EXPECT_STREQ(res.c_str(), "42M");
2647
}
2748
{
28-
const auto res = Base10Human::get(1000000);
29-
EXPECT_STREQ(res.c_str(), "1m");
49+
const auto res = benchmark::Base10HumanReadableFormat(1000000000);
50+
EXPECT_STREQ(res.c_str(), "1B");
3051
}
3152
{
32-
const auto res = Base10Human::get(42000000);
33-
EXPECT_STREQ(res.c_str(), "42m");
53+
const auto res = benchmark::Base10HumanReadableFormat(4000000000);
54+
EXPECT_STREQ(res.c_str(), "4B");
3455
}
3556
{
36-
const auto res = Base10Human::get(4000000000);
37-
EXPECT_STREQ(res.c_str(), "4bn");
57+
const auto res = benchmark::Base10HumanReadableFormat(4200000000);
58+
EXPECT_STREQ(res.c_str(), "4.2B");
3859
}
3960
{
40-
const auto res = Base10Human::get(4200000000);
41-
EXPECT_STREQ(res.c_str(), "4200000000");
61+
const auto res = benchmark::Base10HumanReadableFormat(40200000);
62+
EXPECT_STREQ(res.c_str(), "40.2M");
4263
}
4364
{
44-
const auto res = Base10Human::get(40200000);
45-
EXPECT_STREQ(res.c_str(), "40200000");
65+
const auto res = benchmark::Base10HumanReadableFormat(4200000000000000000);
66+
EXPECT_STREQ(res.c_str(), "4.2Qi");
4667
}
4768
}
4869

0 commit comments

Comments
 (0)