Add `--benchmark_human_readable` flag #1607

DiegoKrupitza · 2023-06-07T23:16:50Z

Allows used to add a command line flag called --benchmark_human_readable. By adding this flag the arguments passed to benchmarks are formated in a human friendly format. This means that numbers that are the power of 2 are formatted as 2^x (e.g., 64 will be 2^6). For numbers that are the power of 10 a different formatting style is used. Numbers 0-999 no formatting is used. For numbers 1000-999999 the format k is used (e.g., 32000 -> 32k). This also works for millions and billions. For numbers greater than 999 billion no special formatting is used.
The design is rather simple allowing to by easily extendable.

Closes: #1006

src/benchmark_api_internal.cc

src/benchmark_api_internal.h

dmah42 · 2023-06-08T13:28:04Z

"test/human_readable_formatting_test.cc:34: CheckRun: Check `name == run.benchmark_name()' failed. expected BM_base_two_args/2^1 got BM_base_two_args/2" test is failing

DiegoKrupitza · 2023-06-19T17:43:43Z

I squashed all the commits into a single one such that there is only one commit to merge.

dmah42 · 2023-06-20T11:45:21Z

I squashed all the commits into a single one such that there is only one commit to merge.

thanks.. i generally "squash and merge" anyway, but this means less editing of commit messages for me :D

src/benchmark_api_internal.h

src/benchmark_api_internal.cc

dmah42 · 2023-06-20T11:50:10Z

src/string_util.cc

@@ -44,7 +51,8 @@ void ToExponentAndMantissa(double val, double thresh, int precision,
  // in 'precision' digits.
  const double adjusted_threshold =
      std::max(thresh, 1.0 / std::pow(10.0, precision));
-  const double big_threshold = adjusted_threshold * one_k;
+  const double big_threshold =
+      (adjusted_threshold * one_k) - inclusiveBigThreshhold;


you appear to be subtracting a boolean from a double.. that's surprising and maybe you could make this explicit instead of relying on an implicit conversion.

I used this neat little hack to make the code a litte bit more concise. But I can make it more explicit and add a little bit of docu why this trick is needed.

dmah42 · 2023-06-20T11:51:13Z

src/string_util.h

+ * Check whether the given value is a power of two or not.
+ * @param val the value to check
+ */
+bool IsPowerOfTwo(const int64_t& val);


do these need to be exposed in the API? i think only FormatHumanReadable is accessed by a different implementation file.

Yes this method doesnt need to be exposed. Exposing Base10 and Base2 is needed for tests and may be needed by other functions in the future.

test/human_readable_gtest.cc

Allows used to add a command line flag called `--benchmark_human_readable`. By adding this flag the arguments passed to benchmarks are formatted in a human friendly format. This means that numbers that are the power of 2 are formatted as `2^x` (e.g., 64 will be `2^6`). For numbers that are the power of 10 a different formatting style is used. Numbers 0-999 no formatting is used. For numbers 1000-999999 the format `k` is used (e.g., `32000` -> `32k`). This also works for millions, billions, trillions, ... For numbers greater than septillions no special formatting is used. The design is rather simple allowing to by easily extendable. Closes: google#1006

DexterIV · 2023-06-23T22:13:43Z

im not a big fan of "human readable format". Id consider binary output to be not human readable but not base10 numbers. Asked chatgpt what it "thinks" about that and here is the answer:
The current name "Human Readable Format" seems appropriate for describing the functionality of the option. However, if you are looking for an alternative name that captures the essence of the feature, you could consider something like "Formatted Number Display" or "Enhanced Numeric Representation."
just a thing to consider :)

dmah42 · 2023-06-26T10:15:29Z

im not a big fan of "human readable format". Id consider binary output to be not human readable but not base10 numbers. Asked chatgpt what it "thinks" about that and here is the answer: The current name "Human Readable Format" seems appropriate for describing the functionality of the option. However, if you are looking for an alternative name that captures the essence of the feature, you could consider something like "Formatted Number Display" or "Enhanced Numeric Representation." just a thing to consider :)

i think it's a pretty well-defined name at this point. df -h, for instance:

       -h, --human-readable
              print sizes in powers of 1024 (e.g., 1023M)

or du -h:

       -h, --human-readable
              print sizes in human readable format (e.g., 1K 234M 2G)

dmah42 · 2023-06-26T10:17:19Z

src/benchmark_api_internal.h

@@ -1,6 +1,8 @@
 #ifndef BENCHMARK_API_INTERNAL_H
 #define BENCHMARK_API_INTERNAL_H

+#include <array>


still seeing these two headers. i'm sure they don't need to be added and i'd rather minimise any dependencies.

dmah42 · 2023-06-26T10:21:58Z

src/string_util.cc

@@ -32,7 +38,8 @@ static const int64_t kUnitsSize = arraysize(kBigSIUnits);

 void ToExponentAndMantissa(double val, double thresh, int precision,
                           double one_k, std::string* mantissa,
-                           int64_t* exponent) {
+                           int64_t* exponent,
+                           bool inclusiveBigThreshhold = false) {


seems like we only set this to true when we're doing base-10. it might be worth having a comment why this is necessary for base-10. or even better, renaming the variable to indicate that directly.