yarpc-go icon indicating copy to clipboard operation
yarpc-go copied to clipboard

feat: Opt for concatenation instead of using fmt.Sprintf

Open moisesvega opened this issue 1 year ago • 1 comments

Summary

The procedure.ToName function, a frequently used pathway, currently relies on fmt.Sprintf to merge strings. However, this method could inadvertently cause additional memory allocations. By directly concatenating these strings instead of utilizing fmt and constructing a format template, we can reduce unnecessary allocations.

This minor modification has the potential to enhance the efficiency of this library for all users.

Benchmark strategy

The strategy involves generating a string for both the serviceName and methodName, running a series of benchmarks, and then comparing the results using benchstat.

before.txt

goos: darwin
goarch: arm64
pkg: go.uber.org/yarpc/pkg/procedure
BenchmarkToName-10      12544957                95.07 ns/op           96 B/op          3 allocs/op
PASS
ok      go.uber.org/yarpc/pkg/procedure 1.421s

after.txt

goos: darwin
goarch: arm64
pkg: go.uber.org/yarpc/pkg/procedure
BenchmarkToName-10      36751287                28.92 ns/op           64 B/op          1 allocs/op
PASS
ok      go.uber.org/yarpc/pkg/procedure 1.549s

Results

bechstat result after running it with go test -bench=. -count=100

name       old time/op    new time/op    delta
ToName-10    95.4ns ± 3%    28.8ns ± 6%  -69.79%  (p=0.000 n=90+97)

name       old alloc/op   new alloc/op   delta
ToName-10     96.0B ± 0%     64.0B ± 0%  -33.33%  (p=0.000 n=100+100)

name       old allocs/op  new allocs/op  delta
ToName-10      3.00 ± 0%      1.00 ± 0%  -66.67%  (p=0.000 n=100+100)

moisesvega avatar May 02 '24 23:05 moisesvega

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: biosvs
:x: moisesvega
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar May 02 '24 23:05 CLAassistant