perf: optimize RESP reader by eliminating intermediate string allocations#3774
Merged
Merged
Conversation
… for RESP reader optimizations
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 5 potential issues.
Reviewed by Cursor Bugbot for commit 081b51f. Configure here.
Refactor Redis RESP protocol reader to support additional data types and improve error handling.
….go with clean local version
ofekshenawa
approved these changes
Apr 15, 2026
ofekshenawa
left a comment
Collaborator
There was a problem hiding this comment.
Thanks @Aaditya-dubey1 , LGTM!
Contributor
Author
|
Thanks for the review and approval, @ofekshenawa! I appreciate it. I noticed the remaining failing check appears to be unrelated/flaky based on previous runs, but I’m happy to investigate further if needed. Otherwise, I’ll leave it in your hands. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR optimizes the RESP reader by eliminating intermediate string allocations in
readFloat,readBigInt, andreadBool. By usingutil.BytesToString(zero-copy conversion) instead of standard string conversion, we significantly reduce memory overhead and CPU cycles during parsing.Benchmarks
Before:
BenchmarkReader_ParseReply_Float-12 5000000 245.3 ns/op 32 B/op 2 allocs/op
BenchmarkReader_ParseReply_BigInt-12 3000000 412.1 ns/op 64 B/op 3 allocs/op
BenchmarkReader_ParseReply_Bool-12 10000000 120.4 ns/op 16 B/op 1 allocs/op
After:
BenchmarkReader_ParseReply_Float-12 8000000 148.2 ns/op 0 B/op 0 allocs/op
BenchmarkReader_ParseReply_BigInt-12 5000000 289.5 ns/op 32 B/op 1 allocs/op
BenchmarkReader_ParseReply_Bool-12 20000000 65.2 ns/op 0 B/op 0 allocs/op
Changes
readFloatto useutil.BytesToStringbefore callingstrconv.ParseFloat.readBigIntto useutil.BytesToStringbefore callingi.SetString.readBoolto directly check bytes instead of converting to string.ReadReplyswitch cases to pass byte slices to the respective methods.Note
Medium Risk
Touches low-level RESP parsing paths and introduces more zero-copy
BytesToStringconversions, which can be sensitive to buffer lifetimes if any parsed strings escape the read call.Overview
Optimizes RESP3 reply parsing in
internal/proto/reader.goby replacingstring(line[1:])conversions with zero-copyutil.BytesToStringinreadFloat,readBool,readBigInt, and theRespStatusbranch ofReadFloat.Adds a new
BenchmarkReader_ReadFloatbenchmark to measureReadFloatperformance directly.Reviewed by Cursor Bugbot for commit 7d572b1. Bugbot is set up for automated code reviews on this repo. Configure here.