From 01f23363176ef88afb88c1169265c275086086cd Mon Sep 17 00:00:00 2001 From: Phil Porada Date: Wed, 15 Nov 2023 17:50:59 -0500 Subject: [PATCH] Check more types in core.IsAnyNilOrZero (#7162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `core.IsAnyNilOrZero` function is used in the critical path for issuance, ocsp generation, specific ratelimit lookups, and before performing each domain validation. The original function heavily relies upon the default switch case which uses the full reflect package, rather than reflectlite which can't be used because it does not implement `IsZero()`. We can reduce CPU utilization and increase the speed of each call by performing some basic type checking. Benchmarking the main branch in `before.txt` and this PR in `after.txt` shows a noticeable reduction in CPU time between the two. I ran each test with `go test -bench=. -count=10 | tee ${FILENAME}` and compared them with [benchstat](https://pkg.go.dev/golang.org/x/perf/cmd/benchstat). The only negatively affected case happens to be when a bool is `true` which may just be due to a sample size of 10 test iterations. ``` $ benchstat before.txt after.txt goos: linux goarch: amd64 pkg: github.com/letsencrypt/boulder/core cpu: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz │ before.txt │ after.txt │ │ sec/op │ sec/op vs base │ IsAnyNilOrZero/input_0-8 6.349n ± 20% 4.660n ± 1% -26.60% (p=0.000 n=10) IsAnyNilOrZero/input_1-8 7.385n ± 15% 6.316n ± 4% -14.48% (p=0.000 n=10) IsAnyNilOrZero/input_0#01-8 6.803n ± 17% 4.284n ± 4% -37.02% (p=0.000 n=10) IsAnyNilOrZero/input_1#01-8 6.806n ± 7% 4.430n ± 5% -34.91% (p=0.000 n=10) IsAnyNilOrZero/input_0#02-8 6.321n ± 8% 4.232n ± 2% -33.06% (p=0.000 n=10) IsAnyNilOrZero/input_0.1-8 7.074n ± 10% 4.176n ± 5% -40.97% (p=0.000 n=10) IsAnyNilOrZero/input_-8 7.494n ± 9% 3.242n ± 2% -56.73% (p=0.000 n=10) IsAnyNilOrZero/input_ahoyhoy-8 9.839n ± 25% 3.990n ± 4% -59.45% (p=0.000 n=10) IsAnyNilOrZero/input_[]-8 3.898n ± 13% 3.405n ± 2% -12.63% (p=0.019 n=10) IsAnyNilOrZero/input_[0]-8 4.533n ± 6% 4.088n ± 2% -9.82% (p=0.001 n=10) IsAnyNilOrZero/input_[1]-8 4.626n ± 6% 4.075n ± 5% -11.92% (p=0.000 n=10) IsAnyNilOrZero/input_-8 2.854n ± 3% 2.039n ± 2% -28.55% (p=0.000 n=10) IsAnyNilOrZero/input_false-8 7.269n ± 2% 6.164n ± 3% -15.21% (p=0.000 n=10) IsAnyNilOrZero/input_true-8 8.047n ± 10% 9.090n ± 11% +12.96% (p=0.003 n=10) IsAnyNilOrZero/input_#01-8 9.114n ± 3% 7.582n ± 2% -16.81% (p=0.000 n=10) IsAnyNilOrZero/input_0001-01-01_00:00:00_+0000_UTC-8 10.630n ± 3% 3.871n ± 4% -63.58% (p=0.000 n=10) IsAnyNilOrZero/input_2015-06-04_11:04:38_+0000_UTC-8 10.900n ± 4% 4.720n ± 2% -56.70% (p=0.000 n=10) IsAnyNilOrZero/input_1s-8 9.509n ± 13% 9.024n ± 4% ~ (p=0.393 n=10) IsAnyNilOrZero/input_0s-8 8.972n ± 4% 7.608n ± 2% -15.20% (p=0.000 n=10) geomean 6.905n 4.772n -30.89% ``` Part of https://github.com/letsencrypt/boulder/issues/7153 --- core/util.go | 24 +++++++++++++++++++++++ core/util_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/core/util.go b/core/util.go index 6582b119c5b..66b34992c3e 100644 --- a/core/util.go +++ b/core/util.go @@ -213,10 +213,34 @@ func IsAnyNilOrZero(vals ...interface{}) bool { switch v := val.(type) { case nil: return true + case string: + if v == "" { + return true + } + case []string: + if len(v) == 0 { + return true + } + case byte: + if v == 0 { + return true + } case []byte: if len(v) == 0 { return true } + case int, int8, int16, int32, int64, uint, uint16, uint32, uint64: + if v == 0 { + return true + } + case float32, float64: + if v == 0 { + return true + } + case time.Time: + if v.IsZero() { + return true + } case *durationpb.Duration: if v == nil || v.AsDuration() == time.Duration(0) { return true diff --git a/core/util_test.go b/core/util_test.go index 5f8f71f9e50..6430635d58b 100644 --- a/core/util_test.go +++ b/core/util_test.go @@ -123,9 +123,15 @@ func TestIsAnyNilOrZero(t *testing.T) { test.Assert(t, IsAnyNilOrZero(""), "Empty string seen as non-zero") test.Assert(t, !IsAnyNilOrZero("string"), "Non-empty string seen as zero") + test.Assert(t, IsAnyNilOrZero([]string{}), "Empty string slice seen as non-zero") + test.Assert(t, !IsAnyNilOrZero([]string{"barncats"}), "Non-empty string slice seen as zero") + test.Assert(t, IsAnyNilOrZero([]byte{}), "Empty byte slice seen as non-zero") test.Assert(t, !IsAnyNilOrZero([]byte("byte")), "Non-empty byte slice seen as zero") + test.Assert(t, IsAnyNilOrZero(time.Time{}), "No specified time value seen as non-zero") + test.Assert(t, !IsAnyNilOrZero(time.Now()), "Current time seen as zero") + type Foo struct { foo int } @@ -144,6 +150,49 @@ func TestIsAnyNilOrZero(t *testing.T) { test.Assert(t, !IsAnyNilOrZero(durationpb.New(666)), "A *durationpb.Duration with valid inner duration is seen as zero") } +func BenchmarkIsAnyNilOrZero(b *testing.B) { + var thyme *time.Time + var sage *time.Duration + var table = []struct { + input interface{} + }{ + {input: int(0)}, + {input: int(1)}, + {input: uint32(0)}, + {input: uint64(1)}, + {input: float32(0)}, + {input: float32(0.1)}, + {input: ""}, + {input: "ahoyhoy"}, + {input: []string{}}, + {input: []string{""}}, + {input: []string{"oodley_doodley"}}, + {input: []byte{}}, + {input: []byte{0}}, + {input: []byte{1}}, + {input: []rune{}}, + {input: []rune{2}}, + {input: []rune{3}}, + {input: nil}, + {input: false}, + {input: true}, + {input: thyme}, + {input: time.Time{}}, + {input: time.Date(2015, time.June, 04, 11, 04, 38, 0, time.UTC)}, + {input: sage}, + {input: time.Duration(1)}, + {input: time.Duration(0)}, + } + + for _, v := range table { + b.Run(fmt.Sprintf("input_%v", v.input), func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = IsAnyNilOrZero(v.input) + } + }) + } +} + func TestUniqueLowerNames(t *testing.T) { u := UniqueLowerNames([]string{"foobar.com", "fooBAR.com", "baz.com", "foobar.com", "bar.com", "bar.com", "a.com"}) sort.Strings(u)