Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

all: avoid string(byteslice) conversion before passing arguments to fmt.*rintf with "%s" verb #10363

Closed
4 tasks done
odeke-em opened this issue Oct 14, 2021 · 0 comments · Fixed by #10364
Closed
4 tasks done

all: avoid string(byteslice) conversion before passing arguments to fmt.*rintf with "%s" verb #10363

odeke-em opened this issue Oct 14, 2021 · 0 comments · Fixed by #10364
Assignees
@odeke-em
Labels
T: Performance Performance improvements

Comments

@odeke-em
Copy link
Collaborator

If we run this pattern

$ git grep 'fmt..\+rintf.\+%s.\+string('
client/keys/add_ledger_test.go:         fmt.Sprintf("--%s=%s", flags.FlagKeyAlgorithm, string(hd.Secp256k1Type)),
client/keys/add_ledger_test.go:         fmt.Sprintf("--%s=%s", flags.FlagKeyAlgorithm, string(hd.Secp256k1Type)),
client/keys/add_test.go:                fmt.Sprintf("--%s=%s", flags.FlagKeyAlgorithm, string(hd.Secp256k1Type)),
client/keys/add_test.go:                fmt.Sprintf("--%s=%s", flags.FlagKeyAlgorithm, string(hd.Secp256k1Type)),
client/keys/add_test.go:                fmt.Sprintf("--%s=%s", flags.FlagKeyAlgorithm, string(hd.Secp256k1Type)),
client/keys/add_test.go:                fmt.Sprintf("--%s=%s", flags.FlagKeyAlgorithm, string(hd.Secp256k1Type)),
client/keys/add_test.go:                fmt.Sprintf("--%s=%s", flags.FlagKeyAlgorithm, string(hd.Secp256k1Type)),
crypto/keyring/signing_algorithms_test.go:      require.Equal(t, fmt.Sprintf("%s,notSupported", string(hd.Secp256k1Type)), list.String())
x/capability/simulation/decoder.go:                     panic(fmt.Sprintf("invalid %s key prefix %X (%s)", types.ModuleName, kvA.Key, string(kvA.Key)))
x/genutil/client/cli/init.go:   _, err = fmt.Fprintf(os.Stderr, "%s\n", string(sdk.MustSortJSON(out)))
x/params/types/subspace.go:             panic(fmt.Sprintf("parameter %s not registered", string(key)))
x/params/types/subspace.go:             panic(fmt.Sprintf("parameter %s not registered", string(key)))
x/upgrade/client/cli/query.go:                  return clientCtx.PrintString(fmt.Sprintf("%s\n", string(bz)))

Notice how many hits we've got. fmt.Printf or fmt.Sprintf know how to convert a byteslice into a string when building the output, and we shouldn't incur the unnecessary string(byteslice) conversion. Using Bencher, we can see improvements such as https://dashboard.github.orijtech.com/benchmark/3245b8e4bbbd44a597480319aaa4b9fe
Screen Shot 2021-10-13 at 9 26 44 PM
Screen Shot 2021-10-13 at 9 26 49 PM
Screen Shot 2021-10-13 at 9 26 56 PM
Screen Shot 2021-10-13 at 9 27 02 PM

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@odeke-em odeke-em added the T: Performance Performance improvements label Oct 14, 2021
@odeke-em odeke-em self-assigned this Oct 14, 2021
@odeke-em odeke-em mentioned this issue Oct 14, 2021
Merged
odeke-em added a commit that referenced this issue Oct 14, 2021
@odeke-em
perf: avoid unnecessary byteslice->string before fmt %s verb
239e6c6 
fmt.Printf or fmt.Sprintf already know how to convert a
byteslice into a string when building the output; we shouldn't
incur the unnecessary string(byteslice) conversion.
Using Bencher, we can see improvements such as
https://dashboard.github.orijtech.com/benchmark/3245b8e4bbbd44a597480319aaa4b9fe
which in independent experiments show:

* time/op (ns/op)
FormatIt-8	1.2µs ± 2%	1.1µs ± 10%	-11.77%	(p=0.000 n=10+9)

* speed (MB/s)
FormatIt-8	0.71GB/s ± 2%	0.80GB/s ± 9%	+13.59%	(p=0.000 n=10+9)

* allocs/op (B/op)
FormatIt-8	2.0kB ± 0%	1.1kB ± 0%	-45.62%	(p=0.000 n=10+10)

* allocs/op (count/op)
FormatIt-8	11 ± 0%	        9.0 ± 0%	-18.18%	(p=0.000 n=10+10)

Fixes #10363
@mergify mergify bot closed this as completed in #10364 Oct 14, 2021
mergify bot pushed a commit that referenced this issue Oct 14, 2021
@odeke-em
perf: avoid unnecessary byteslice->string before fmt %s verb (#10364)
d0f64df 
fmt.Printf or fmt.Sprintf already know how to convert a
byteslice into a string when building the output; we shouldn't
incur the unnecessary string(byteslice) conversion.
Using Bencher, we can see improvements such as
https://dashboard.github.orijtech.com/benchmark/3245b8e4bbbd44a597480319aaa4b9fe
which in independent experiments show:

* time/op (ns/op)
FormatIt-8	1.2µs ± 2%	1.1µs ± 10%	-11.77%	(p=0.000 n=10+9)

* speed (MB/s)
FormatIt-8	0.71GB/s ± 2%	0.80GB/s ± 9%	+13.59%	(p=0.000 n=10+9)

* allocs/op (B/op)
FormatIt-8	2.0kB ± 0%	1.1kB ± 0%	-45.62%	(p=0.000 n=10+10)

* allocs/op (count/op)
FormatIt-8	11 ± 0%	        9.0 ± 0%	-18.18%	(p=0.000 n=10+10)

Fixes #10363
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@odeke-em odeke-em

Labels
T: Performance Performance improvements
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

perf: avoid unnecessary byteslice->string before fmt %s verb
1 participant
@odeke-em