hlc: improve Timestamp.IsEmpty() performance

[erikgrinaker]

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)

Resolves #88818.

Release note: None

[cockroach-teamcity]

This change is 

[sumeerbhola]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @erikgrinaker and @nicktrav)

---

pkg/util/hlc/timestamp.go line 199 at r1 (raw file):

// gcassert:inline
func (t Timestamp) IsEmpty() bool {
	return t.WallTime == 0 && t.Logical == 0 && !t.Synthetic

Should we add a reminder to ourselves with a comment in timestamp.proto, that states that the code in timestamp.go relies on the set of fields being unchanged, so remember to update if adding fields?
May be overkill given that this is an extremely rarely changed part of the code.

[erikgrinaker]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @nicktrav and @sumeerbhola)

---

pkg/util/hlc/timestamp.go line 199 at r1 (raw file):

Previously, sumeerbhola wrote…

Should we add a reminder to ourselves with a comment in timestamp.proto, that states that the code in timestamp.go relies on the set of fields being unchanged, so remember to update if adding fields?
May be overkill given that this is an extremely rarely changed part of the code.

We'd have to adapt a bunch of other methods as well, such as Add and Compare, so I think we'd be careful to audit the small set of methods here for any changes.

[erikgrinaker]

bors r=sumeerbhola

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)