Skip to content

Commit

Permalink
Check more types in core.IsAnyNilOrZero (#7162)
Browse files Browse the repository at this point in the history
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_<nil>-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_<nil>#1-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 #7153
  • Loading branch information
pgporada committed Nov 15, 2023
1 parent 104c393 commit 01f2336
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 0 deletions.
24 changes: 24 additions & 0 deletions core/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 49 additions & 0 deletions core/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
Expand Down

0 comments on commit 01f2336

Please sign in to comment.