Skip to content

chore: use go 1.20 idiomatic string<->byte conversion #3435

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

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

Conversation

justinhwang
Copy link

@justinhwang justinhwang commented Jul 16, 2025

As of Go 1.20, this is the idiomatic way to convert from Bytes to String and vice-versa.

This updates go.mod to 1.20, builds have already been running on 1.23.x and 1.24.x since #3274.

Benchmarks

Benchmarks show no change:

goos: darwin
goarch: arm64
pkg: github.com/redis/go-redis/v9/internal/util
cpu: Apple M3 Max
BenchmarkStringToBytes/copy-16         	112308339	        10.61 ns/op	      24 B/op	       1 allocs/op
BenchmarkStringToBytes/unsafe-16       	1000000000	         0.9967 ns/op	       0 B/op	       0 allocs/op
BenchmarkBytesToString/copy-16         	134565944	         8.884 ns/op	      24 B/op	       1 allocs/op
BenchmarkBytesToString/unsafe-16       	1000000000	         0.4511 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/redis/go-redis/v9/internal/util	6.001s
goos: darwin
goarch: arm64
pkg: github.com/redis/go-redis/v9/internal/util
cpu: Apple M3 Max
BenchmarkStringToBytes/copy-16         	112317182	        10.56 ns/op	      24 B/op	       1 allocs/op
BenchmarkStringToBytes/unsafe-16       	1000000000	         0.6711 ns/op	       0 B/op	       0 allocs/op
BenchmarkBytesToString/copy-16         	135204112	         8.874 ns/op	      24 B/op	       1 allocs/op
BenchmarkBytesToString/unsafe-16       	1000000000	         0.5666 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/redis/go-redis/v9/internal/util	5.753s
name                     old time/op    new time/op    delta
StringToBytes/copy-16      10.6ns ± 0%    10.6ns ± 0%   ~     (p=1.000 n=1+1)
StringToBytes/unsafe-16    1.00ns ± 0%    0.67ns ± 0%   ~     (p=1.000 n=1+1)
BytesToString/copy-16      8.88ns ± 0%    8.87ns ± 0%   ~     (p=1.000 n=1+1)
BytesToString/unsafe-16    0.45ns ± 0%    0.57ns ± 0%   ~     (p=1.000 n=1+1)

name                     old alloc/op   new alloc/op   delta
StringToBytes/copy-16       24.0B ± 0%     24.0B ± 0%   ~     (all equal)
StringToBytes/unsafe-16     0.00B          0.00B        ~     (all equal)
BytesToString/copy-16       24.0B ± 0%     24.0B ± 0%   ~     (all equal)
BytesToString/unsafe-16     0.00B          0.00B        ~     (all equal)

name                     old allocs/op  new allocs/op  delta
StringToBytes/copy-16        1.00 ± 0%      1.00 ± 0%   ~     (all equal)
StringToBytes/unsafe-16      0.00           0.00        ~     (all equal)
BytesToString/copy-16        1.00 ± 0%      1.00 ± 0%   ~     (all equal)
BytesToString/unsafe-16      0.00           0.00        ~     (all equal)

As of Go 1.20, this is the idiomatic way to convert from Bytes to
String and vice-versa.

This updates `go.mod` to 1.20, builds have already been running on
1.23.x and 1.24.x since redis#3274.
@justinhwang
Copy link
Author

@ndyakov do you think you could help take a look? thanks!

@ndyakov
Copy link
Member

ndyakov commented Jul 18, 2025

@justinhwang Thank you for your contribution. I do like the idea and would like to upgrade to a newer go version, but we discussed to put this one back a bit since it would be a breaking change and we would have to release a new major version. I will tag this PR and review / merge it once we get there (hopefully sooner and previously expected).

@ndyakov ndyakov added the v10 label Jul 18, 2025
@justinhwang
Copy link
Author

@ndyakov It isn't a breaking change no? Semantically it does the same thing under the hood, just uses Go 1.20's safer way of doing it. Unless you're referring to the Go version bump as a breaking change

@ndyakov
Copy link
Member

ndyakov commented Jul 18, 2025

@justinhwang yes, I am referring to bumping go to a new version as a "breaking change" since users will have to upgrade to a new version and this may actually not work well for all of them.

@justinhwang
Copy link
Author

ack yeah that makes sense. Please feel free to merge whenever you're ready for v10!

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

Successfully merging this pull request may close these issues.

2 participants