Skip to content

Commit

Permalink
Fix benchmarks in escaping_benchmark.cc by properly calling `benchm…
Browse files Browse the repository at this point in the history
…ark::DoNotOptimize` on both inputs and outputs and by removing the unnecessary and wrong `ABSL_RAW_CHECK` condition (`check != 0`) of `BM_ByteStringFromAscii_Fail` benchmark.

Relevant comment:
```
// The DoNotOptimize(...) function can be used to prevent a value or
// expression from being optimized away by the compiler. This function is
// intended to add little to no overhead.
// See: http://stackoverflow.com/questions/28287064
//
// The specific guarantees of DoNotOptimize(x) are:
//  1) x, and any data it transitively points to, will exist (in a register or
//     in memory) at the current point in the program.
//  2) The optimizer will assume that DoNotOptimize(x) could mutate x or
//     anything it transitively points to (although it actually doesn't).
//
// To see this in action:
//
//   void BM_multiply(benchmark::State& state) {
//     int a = 2;
//     int b = 4;
//     for (auto s : state) {
//       testing::DoNotOptimize(a);
//       testing::DoNotOptimize(b);
//       int c = a * b;
//       testing::DoNotOptimize(c);
//     }
//   }
//   BENCHMARK(BM_multiply);
//
// Guarantee (2) applied to 'a' and 'b' prevents the compiler lifting the
// multiplication outside of the loop. Guarantee (1) applied to 'c' prevents the
// compiler from optimizing away 'c' as dead code.
```
To see #1 and #2 in action, see: https://godbolt.org/z/ned1578ve

PiperOrigin-RevId: 676588185
Change-Id: I7ed3e4bed8274b54ac7877316f2d82c33d68f00f
  • Loading branch information
nafi3000 authored and copybara-github committed Sep 19, 2024
1 parent 162dc78 commit b345425
Showing 1 changed file with 12 additions and 15 deletions.
27 changes: 12 additions & 15 deletions absl/strings/escaping_benchmark.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
#include "absl/strings/escaping.h"

#include <cstdint>
#include <cstdio>
#include <cstring>
#include <memory>
#include <random>
#include <string>
Expand All @@ -25,6 +23,7 @@
#include "absl/base/internal/raw_logging.h"
#include "absl/strings/internal/escaping_test_common.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"

namespace {

Expand All @@ -33,9 +32,12 @@ void BM_CUnescapeHexString(benchmark::State& state) {
for (int i = 0; i < 50; i++) {
src += "\\x55";
}
std::string dest;
for (auto _ : state) {
absl::CUnescape(src, &dest);
std::string dest;
benchmark::DoNotOptimize(src);
bool result = absl::CUnescape(src, &dest);
benchmark::DoNotOptimize(result);
benchmark::DoNotOptimize(dest);
}
}
BENCHMARK(BM_CUnescapeHexString);
Expand All @@ -47,19 +49,12 @@ void BM_WebSafeBase64Escape_string(benchmark::State& state) {
raw += std::string(test_set.plaintext);
}
}

// The actual benchmark loop is tiny...
std::string escaped;
for (auto _ : state) {
std::string escaped;
benchmark::DoNotOptimize(raw);
absl::WebSafeBase64Escape(raw, &escaped);
benchmark::DoNotOptimize(escaped);
}

// We want to be sure the compiler doesn't throw away the loop above,
// and the easiest way to ensure that is to round-trip the results and verify
// them.
std::string round_trip;
absl::WebSafeBase64Unescape(escaped, &round_trip);
ABSL_RAW_CHECK(round_trip == raw, "");
}
BENCHMARK(BM_WebSafeBase64Escape_string);

Expand All @@ -76,7 +71,9 @@ void CEscapeBenchmarkHelper(benchmark::State& state, const char* string_value,
}

for (auto _ : state) {
absl::CEscape(src);
benchmark::DoNotOptimize(src);
std::string result = absl::CEscape(src);
benchmark::DoNotOptimize(result);
}
}

Expand Down

0 comments on commit b345425

Please sign in to comment.