From fecf978c7f94ed188ab8b4bc800808c0dd3801c1 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Wed, 28 Sep 2022 11:37:37 +0000 Subject: [PATCH] hlc: improve `Timestamp.IsEmpty()` performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Timestamp.IsEmpty()` saw a significant performance regression with Go 1.19, in particular with empty timestamps, apparently due to the compiler's use of `memeqbody` for struct comparisons. On `EncodeMVCCKey` benchmarks, the empty timestamp case showed a ~50% regression. This patch changes `IsEmpty()` to avoid struct comparison, instead comparing the member fields directly. This yields a significant performance improvement (using Go 1.19): ``` name old time/op new time/op delta TimestampIsEmpty/empty-24 6.90ns ± 2% 0.79ns ± 0% -88.56% (p=0.000 n=10+9) TimestampIsEmpty/walltime-24 2.97ns ± 1% 0.75ns ± 1% -74.90% (p=0.000 n=10+10) TimestampIsEmpty/all-24 2.97ns ± 1% 0.75ns ± 1% -74.77% (p=0.000 n=10+10) ``` This in turn significantly improves `EncodeMVCCKey` performance: ``` name old time/op new time/op delta EncodeMVCCKey/key=empty/ts=empty-24 22.0ns ± 0% 15.7ns ± 5% -28.67% (p=0.000 n=9+10) EncodeMVCCKey/key=empty/ts=walltime-24 21.2ns ± 0% 17.8ns ± 0% -16.42% (p=0.000 n=10+10) EncodeMVCCKey/key=empty/ts=walltime+logical-24 21.8ns ± 0% 18.6ns ± 0% -15.02% (p=0.000 n=10+10) EncodeMVCCKey/key=empty/ts=all-24 22.2ns ± 0% 18.8ns ± 0% -15.07% (p=0.000 n=9+9) EncodeMVCCKey/key=short/ts=empty-24 22.3ns ± 0% 15.8ns ± 0% -29.06% (p=0.000 n=8+10) EncodeMVCCKey/key=short/ts=walltime-24 21.2ns ± 0% 18.0ns ± 0% -15.02% (p=0.000 n=9+8) EncodeMVCCKey/key=short/ts=walltime+logical-24 22.6ns ± 0% 18.8ns ± 0% -16.53% (p=0.000 n=9+8) EncodeMVCCKey/key=short/ts=all-24 22.6ns ± 0% 19.1ns ± 0% -15.32% (p=0.000 n=10+10) EncodeMVCCKey/key=long/ts=empty-24 65.2ns ± 1% 58.6ns ± 0% -10.11% (p=0.000 n=9+9) EncodeMVCCKey/key=long/ts=walltime-24 62.6ns ± 0% 59.6ns ± 1% -4.72% (p=0.000 n=10+10) EncodeMVCCKey/key=long/ts=walltime+logical-24 63.5ns ± 0% 60.2ns ± 0% -5.16% (p=0.000 n=10+10) EncodeMVCCKey/key=long/ts=all-24 63.9ns ± 0% 59.6ns ± 0% -6.81% (p=0.000 n=9+9) ``` It also yields a nice improvement over 22.1 with Go 1.18: ``` name old time/op new time/op delta EncodeMVCCKey/key=empty/ts=empty-24 15.7ns ± 0% 15.7ns ± 5% ~ (p=0.497 n=8+10) EncodeMVCCKey/key=empty/ts=walltime-24 19.5ns ± 0% 17.8ns ± 0% -8.99% (p=0.000 n=10+10) EncodeMVCCKey/key=empty/ts=walltime+logical-24 20.2ns ± 0% 18.6ns ± 0% -8.03% (p=0.000 n=10+10) EncodeMVCCKey/key=empty/ts=all-24 20.4ns ± 0% 18.8ns ± 0% -7.67% (p=0.000 n=9+9) EncodeMVCCKey/key=short/ts=empty-24 16.0ns ± 0% 15.8ns ± 0% -1.47% (p=0.000 n=10+10) EncodeMVCCKey/key=short/ts=walltime-24 20.6ns ± 0% 18.0ns ± 0% -12.51% (p=0.000 n=8+8) EncodeMVCCKey/key=short/ts=walltime+logical-24 20.9ns ± 0% 18.8ns ± 0% -9.82% (p=0.000 n=10+8) EncodeMVCCKey/key=short/ts=all-24 20.9ns ± 0% 19.1ns ± 0% -8.79% (p=0.000 n=9+10) EncodeMVCCKey/key=long/ts=walltime-24 60.4ns ± 0% 59.6ns ± 1% -1.30% (p=0.000 n=9+10) EncodeMVCCKey/key=long/ts=walltime+logical-24 61.4ns ± 0% 60.2ns ± 0% -1.84% (p=0.000 n=10+10) EncodeMVCCKey/key=long/ts=all-24 61.4ns ± 0% 59.6ns ± 0% -2.88% (p=0.000 n=10+9) EncodeMVCCKey/key=long/ts=empty-24 59.1ns ± 0% 58.6ns ± 0% -0.77% (p=0.000 n=10+9) ``` Release note: None --- pkg/util/hlc/timestamp.go | 4 +++- pkg/util/hlc/timestamp_test.go | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pkg/util/hlc/timestamp.go b/pkg/util/hlc/timestamp.go index dc7beed28137..8ec5394e6731 100644 --- a/pkg/util/hlc/timestamp.go +++ b/pkg/util/hlc/timestamp.go @@ -194,11 +194,13 @@ func (t Timestamp) AsOfSystemTime() string { } // IsEmpty returns true if t is an empty Timestamp. +// gcassert:inline func (t Timestamp) IsEmpty() bool { - return t == Timestamp{} + return t.WallTime == 0 && t.Logical == 0 && !t.Synthetic } // IsSet returns true if t is not an empty Timestamp. +// gcassert:inline func (t Timestamp) IsSet() bool { return !t.IsEmpty() } diff --git a/pkg/util/hlc/timestamp_test.go b/pkg/util/hlc/timestamp_test.go index 465d45ee2905..04d2867f5e6a 100644 --- a/pkg/util/hlc/timestamp_test.go +++ b/pkg/util/hlc/timestamp_test.go @@ -405,3 +405,23 @@ func BenchmarkTimestampStringSynthetic(b *testing.B) { _ = ts.String() } } + +func BenchmarkTimestampIsEmpty(b *testing.B) { + cases := map[string]Timestamp{ + "empty": {}, + "walltime": {WallTime: 1664364012528805328}, + "all": {WallTime: 1664364012528805328, Logical: 65535, Synthetic: true}, + } + + var result bool + + for name, ts := range cases { + b.Run(name, func(b *testing.B) { + for i := 0; i < b.N; i++ { + result = ts.IsEmpty() + } + }) + } + + _ = result // make sure compiler doesn't optimize away the call +}