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

String overhead #121

Closed
dim opened this issue Jun 5, 2015 · 3 comments · Fixed by #118
Closed

String overhead #121

dim opened this issue Jun 5, 2015 · 3 comments · Fixed by #118

Comments

@dim
Copy link
Collaborator

dim commented Jun 5, 2015

Soo, this is something I wanted to discuss a long time ago, now that you have started working on #118, I think it's the right time. The client is currently using strings for arguments across the board. These strings are then converted to []byte and sent across the wire. Responses are received as byte streams and then converted (mostly) to strings again. This is quite inefficient if you are dealing with larger objects at higher frequency as it causes many allocations and therefore GC overhead. I was wondering if it would be sensible to implement most command methods using low-overhead []byte and then have MethodNameString/MethodNameValue accessors. Example:

func (c *commandable) Set(key, value []byte)
func (c *commandable) SetString(key, value string)
func (c *commandable) SetValue(key string, value interface{})

Alternatively:

func (c *commandable) SetBytes(key, value []byte)
func (c *commandable) Set(key, value string) // calls SetBytes internally
func (c *commandable) SetValue(key string, value interface{})  // calls SetBytes internally

I don't know how valuable this would be for everyone else, but it would make a huge difference for us.

@vmihailenco
Copy link
Collaborator

I think strings usage in Set(key, value string) should not cause any additional allocations, because string can be appended to []byte without conversion, e.g.:

var data []byte
data = append(data, "string"...)

See https://github.com/go-redis/redis/blob/master/parser.go#L20 for details.

But if you have large []byte value and you want to pass it to go-redis then indeed there is an overhead, because you need to convert it to string. It is possible that #118 can help if:

  1. we use unsafe to cast []byte -> string
  2. we change args type from []string to []interface{} in appendArgs and all New[String|Int|...]Cmd methods

@vmihailenco
Copy link
Collaborator

As for the API I think that default should be Set(key string, value []interface{}). We can consider adding SetBytes(key, value []byte) if benchmark shows it is considerably faster, but I wonder if you have a real use case for key []byte?

@dim
Copy link
Collaborator Author

dim commented Jun 5, 2015

That API sounds good to me. Your'e right, there is no real use-case for key []byte but I would recommend using http://golang.org/pkg/bytes/#Buffer.WriteString instead of data = append(data, "string"...) to generate the socket message (or http://golang.org/pkg/bufio/#Writer.WriteString)

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 a pull request may close this issue.

2 participants