Skip to content
This repository has been archived by the owner on Jan 10, 2024. It is now read-only.

Optimize randBool() and randString() #41

Merged
merged 4 commits into from
Jun 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions fuzz.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"time"

"github.com/google/gofuzz/bytesource"
"strings"
)

// fuzzFuncMap is a map from a type to a fuzzFunc that handles that type.
Expand Down Expand Up @@ -495,10 +496,7 @@ var fillFuncMap = map[reflect.Kind]func(reflect.Value, *rand.Rand){

// randBool returns true or false randomly.
func randBool(r *rand.Rand) bool {
if r.Int()&1 == 1 {
return true
}
return false
return r.Int31()&(1<<30) == 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bit 31 and not some other bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to pick one and I believe there was no better choice :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, they should all be the same. I just wasn't sure if the change from bit 0 was significant for the speed up--it shouldn't be, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my benchmarks, on my local machine I've found this bit to be faster from bit 0 by 1-2% - why, I don't know - probably some CPU arch black magic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm... you can use https://godoc.org/golang.org/x/tools/cmd/benchcmp to tell if it's significant or not. (not shown in the help: you can put multiple runs in both the old and new file and it will give you the std dev so you can tell if it's significant or not)

}

type int63nPicker interface {
Expand Down Expand Up @@ -526,11 +524,12 @@ var unicodeRanges = []charRange{
// may include a variety of (valid) UTF-8 encodings.
func randString(r *rand.Rand) string {
n := r.Intn(20)
runes := make([]rune, n)
for i := range runes {
runes[i] = unicodeRanges[r.Intn(len(unicodeRanges))].choose(r)
sb := strings.Builder{}
sb.Grow(n)
Copy link
Contributor

@lavalamp lavalamp Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I bet n*2 or n*3 would avoid an extra allocation or two.

for i := 0; i < n; i++ {
sb.WriteRune(unicodeRanges[r.Intn(len(unicodeRanges))].choose(r))
}
return string(runes)
return sb.String()
}

// randUint64 makes random 64 bit numbers.
Expand Down
25 changes: 22 additions & 3 deletions fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,12 @@ type customInt63 struct {

func (c customInt63) Int63n(n int64) int64 {
switch c.mode {
case modeFirst: return 0
case modeLast: return n-1
default: return rand.Int63n(n)
case modeFirst:
return 0
case modeLast:
return n - 1
default:
return rand.Int63n(n)
}
}

Expand Down Expand Up @@ -566,3 +569,19 @@ func TestNewFromGoFuzz(t *testing.T) {
t.Errorf("Fuzz(%q) = %d, want: %d", input, got, want)
}
}

func BenchmarkRandBool(b *testing.B) {
rs := rand.New(rand.NewSource(123))

for i := 0; i < b.N; i++ {
randBool(rs)
}
}

func BenchmarkRandString(b *testing.B) {
rs := rand.New(rand.NewSource(123))

for i := 0; i < b.N; i++ {
randString(rs)
}
}