diff options
author | Abseil Team <absl-team@google.com> | 2019-09-30 16:54:48 -0700 |
---|---|---|
committer | CJ Johnson <johnsoncj@google.com> | 2019-10-01 13:58:29 -0400 |
commit | 8fe7214fe2d7a45ecc4d85f6a524c6b1532426de (patch) | |
tree | 6f7a1b24017a3a197e883cdc7a85f19cfd3376cb | |
parent | debac94cfb5a0fa75d1d97c399682bd1c72e3191 (diff) |
Export of internal Abseil changes
--
406622c43f296eeedf00e0e9246acfb4ea6ecd5e by Abseil Team <absl-team@google.com>:
Avoid the compiler reloading __is_long() on string_view(const string&)
The underlying cause is that the compiler assume a scenario where string_view is created with placement new into memory occupied by the input, so the store to 'ptr' can affect the value / result of size(); i.e., __is_long() reloads the __size value).
Example code: string_view1 demonstrates the problem, string_view2 DTRT.
=== string_view1
struct string_view1 {
string_view1(const char* ptr, size_t n) : ptr(ptr), n(n) {}
string_view1(const std::string& s) : ptr(s.data()), n(s.length()) {}
const char* ptr;
size_t n;
};
struct S1 {
S1(const std::string& s);
string_view1 sv;
};
S1::S1(const std::string& s) : sv(s) {}
S1::S1
test byte ptr [rsi], 1
je .LBB0_1
mov rax, qword ptr [rsi + 16]
mov qword ptr [rdi], rax
movzx eax, byte ptr [rsi]
test al, 1
jne .LBB0_5
.LBB0_4:
shr rax
mov qword ptr [rdi + 8], rax
ret
.LBB0_1:
lea rax, [rsi + 1]
mov qword ptr [rdi], rax
movzx eax, byte ptr [rsi]
test al, 1
je .LBB0_4
.LBB0_5:
mov rax, qword ptr [rsi + 8]
mov qword ptr [rdi + 8], rax
ret
=== string_view2
struct string_view2 {
string_view2(const char* ptr, size_t n) : ptr(ptr), n(n) {}
string_view2(const std::string& s) : string_view2(s.data(), s.size()) {}
const char* ptr;
size_t n;
};
struct S2 {
S2(const std::string& s);
string_view2 sv;
};
S2::S2(const std::string& s) : sv(s) {}
S2::S2
movzx eax, byte ptr [rsi]
test al, 1
je .LBB1_1
mov rax, qword ptr [rsi + 8]
mov rsi, qword ptr [rsi + 16]
mov qword ptr [rdi], rsi
mov qword ptr [rdi + 8], rax
ret
.LBB1_1:
add rsi, 1
shr rax
mov qword ptr [rdi], rsi
mov qword ptr [rdi + 8], rax
ret
PiperOrigin-RevId: 272096771
GitOrigin-RevId: 406622c43f296eeedf00e0e9246acfb4ea6ecd5e
Change-Id: I70173a2db68cd9b597fff1c09e00198c632cfe95
-rw-r--r-- | absl/strings/string_view.h | 4 | ||||
-rw-r--r-- | absl/strings/string_view_benchmark.cc | 18 |
2 files changed, 21 insertions, 1 deletions
diff --git a/absl/strings/string_view.h b/absl/strings/string_view.h index 68b90aa3..841570b6 100644 --- a/absl/strings/string_view.h +++ b/absl/strings/string_view.h @@ -168,7 +168,9 @@ class string_view { string_view( // NOLINT(runtime/explicit) const std::basic_string<char, std::char_traits<char>, Allocator>& str) noexcept - : ptr_(str.data()), length_(CheckLengthInternal(str.size())) {} + // This is implement in terms of `string_view(p, n)` so `str.size()` + // doesn't need to be reevaluated after `ptr_` is set. + : string_view(str.data(), str.size()) {} // Implicit constructor of a `string_view` from nul-terminated `str`. When // accepting possibly null strings, use `absl::NullSafeStringView(str)` diff --git a/absl/strings/string_view_benchmark.cc b/absl/strings/string_view_benchmark.cc index 46909cb0..f2e4214c 100644 --- a/absl/strings/string_view_benchmark.cc +++ b/absl/strings/string_view_benchmark.cc @@ -30,6 +30,24 @@ namespace { +void BM_StringViewFromString(benchmark::State& state) { + std::string s(state.range(0), 'x'); + std::string* ps = &s; + struct SV { + SV() = default; + explicit SV(const std::string& s) : sv(s) {} + absl::string_view sv; + } sv; + SV* psv = &sv; + benchmark::DoNotOptimize(ps); + benchmark::DoNotOptimize(psv); + for (auto _ : state) { + new (psv) SV(*ps); + benchmark::DoNotOptimize(sv); + } +} +BENCHMARK(BM_StringViewFromString)->Arg(12)->Arg(128); + // Provide a forcibly out-of-line wrapper for operator== that can be used in // benchmarks to measure the impact of inlining. ABSL_ATTRIBUTE_NOINLINE |