-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add sliceHeader
for zero-copy parsing of message headers, use for client info
#6453
Conversation
server/client.go
Outdated
index++ | ||
} | ||
return value | ||
return hdr[start:index] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also set cap here? That way if someone does try to do an append it will force a copy at that point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, done.
ecf0047
to
99528d9
Compare
…lient info Signed-off-by: Neil Twigg <neil@nats.io>
99528d9
to
7f8075d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
if v == nil { | ||
return nil | ||
} | ||
return append(make([]byte, 0, len(v)), v...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe do allocation and copy? Unless we know append is as efficient for larger sizes..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just as efficient when the capacity is preallocated, so just makes for a tidier one-liner.
In the various places that we are handling client info from headers, like when queuing and pulling from the JS API queue or due to the service export, we could safely refer to the underlying header slice, instead of making a copy as
getHeader()
does today.This PR adds a new
sliceHeader()
that merely slices the input header instead, reducing copies considerably in any system where there is heavy usage of JS API requests.Signed-off-by: Neil Twigg neil@nats.io