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.

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

perf: replace unsafe with the raw method for string to bytes conversion #3936

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Aden-Q
Copy link

@Aden-Q Aden-Q commented Apr 26, 2024

Issue references: #3935

For string to bytes conversion
Two points to make this change:

  1. The raw method also has 0 memory allocation
  2. Unsafe is slightly slower than the raw method. And as the name suggests, it's "unsafe". There's no point to use the unsafe method to convert string to bytes

Benchmark result in: gin/internal/bytesconv/bytesconv_test.go:

╰─± go test -v -run=none -bench=^BenchmarkBytesConvStr -benchmem=true
goos: darwin
goarch: amd64
pkg: github.com/gin-gonic/gin/internal/bytesconv
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkBytesConvStrToBytesRaw
BenchmarkBytesConvStrToBytesRaw-12      1000000000               0.2542 ns/op          0 B/op          0 allocs/op
BenchmarkBytesConvStrToBytes
BenchmarkBytesConvStrToBytes-12         1000000000               0.5039 ns/op          0 B/op          0 allocs/op
PASS
ok      github.com/gin-gonic/gin/internal/bytesconv     1.163s

@Aden-Q Aden-Q changed the title chore: replace unsafe with the raw method for string to bytes conversion perf: replace unsafe with the raw method for string to bytes conversion Apr 30, 2024
Copy link

codecov bot commented May 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.18%. Comparing base (3dc1cd6) to head (835d074).
Report is 38 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3936      +/-   ##
==========================================
- Coverage   99.21%   99.18%   -0.03%     
==========================================
  Files          42       43       +1     
  Lines        3182     2709     -473     
==========================================
- Hits         3157     2687     -470     
+ Misses         17       12       -5     
- Partials        8       10       +2     
Flag Coverage Δ
?
-tags "sonic avx" 99.18% <100.00%> (?)
-tags go_json 99.18% <100.00%> (?)
-tags nomsgpack 99.17% <100.00%> (?)
go-1.18 99.11% <100.00%> (-0.01%) ⬇️
go-1.19 99.18% <100.00%> (-0.03%) ⬇️
go-1.20 99.18% <100.00%> (-0.03%) ⬇️
go-1.21 99.18% <100.00%> (-0.03%) ⬇️
go-1.22 99.18% <100.00%> (?)
macos-latest 99.18% <100.00%> (-0.03%) ⬇️
ubuntu-latest 99.18% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Aden-Q
Copy link
Author

Aden-Q commented May 1, 2024

@appleboy I also saw this article, it's true for go 1.21. As of go 1.22 now, it's still true for bytes-to-string conversion, but it's wrong for string-to-bytes conversion.

Here's the benchmark results they posted in the article (the author uses go 1.21.3):

[I] ⋊> ~/G/zerocast on main ⨯ go test -run=xxx -bench=.  ./...                                                                                                               21:18:22
goos: darwin
goarch: arm64
pkg: github.com/josestg/zerocast
BenchmarkStringToBytesStandard-10        7207893               151.1 ns/op          1792 B/op          1 allocs/op
BenchmarkBytesToStringStandard-10        8589217               143.8 ns/op          1792 B/op          1 allocs/op
BenchmarkStringToBytes-10               1000000000               0.3904 ns/op          0 B/op          0 allocs/op
BenchmarkBytesToString-10               1000000000               0.3912 ns/op          0 B/op          0 allocs/op
PASS
ok      github.com/josestg/zerocast     3.830s

Here's the benchmark results I get when I run it locally (I use go 1.22.2 which is the latest version):

╰─± go test -run=xxx -bench=.  ./...
goos: darwin
goarch: amd64
pkg: github.com/josestg/zerocast
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkStringToBytesStandard-12    	1000000000	         0.2581 ns/op	       0 B/op	       0 allocs/op
BenchmarkBytesToStringStandard-12    	 5414834	       223.4 ns/op	    1792 B/op	       1 allocs/op
BenchmarkStringToBytes-12            	1000000000	         0.4422 ns/op	       0 B/op	       0 allocs/op
BenchmarkBytesToString-12            	1000000000	         0.5160 ns/op	       0 B/op	       0 allocs/op

Please check the difference on this row specifically:

  • result from the article:
    BenchmarkStringToBytesStandard-10 7207893 151.1 ns/op 1792 B/op 1 allocs/op
  • my result:
    BenchmarkStringToBytesStandard-12 1000000000 0.2581 ns/op 0 B/op 0 allocs/op

After further digging into it, I found go1.22.0 introduced this performance improvement but cannot find it in their release notes or changelogs...

As of go 1.22 now, string to bytes conversion using the standard way is also 0 memory allocation, and faster. Please take another look at my issue and PR @appleboy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants