You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In function func (nc *Conn) processMsg(data []byte) there's a snippet of code
// FIXME(dlc): Need to copy, should/can do COW?msgPayload:=make([]byte, len(data))
copy(msgPayload, data)
It appears that every time we see a new message coming in, the payload gets copied into a new buffer. This is reasonable for small payloads since we are reusing the scratch buffer scratch [MAX_CONTROL_LINE_SIZE]byte. But for large payloads a new buffer area sized as that of the actual message payload has already been created by invoking make([]byte)
// If we will overflow the scratch buffer, just create a// new buffer to hold the split message.ifnc.ps.ma.size>cap(nc.ps.scratch)-len(nc.ps.argBuf) {
lrem:=len(buf[nc.ps.as:])
nc.ps.msgBuf=make([]byte, lrem, nc.ps.ma.size)
copy(nc.ps.msgBuf, buf[nc.ps.as:])
} else {
nc.ps.msgBuf=nc.ps.scratch[len(nc.ps.argBuf):len(nc.ps.argBuf)]
nc.ps.msgBuf=append(nc.ps.msgBuf, (buf[nc.ps.as:])...)
}
Here I think we may avoid this unnecessary copy for large message payloads within processMsg(data []byte) by introducting a bool flag, say largeMsgBuf, indicating that we don't need to copy the message payload a second time
func (nc*Conn) processMsg(data []byte)
...// FIXME(dlc): Need to copy, should/can do COW?varmsgPayload=dataif!nc.ps.largeMsgBuf {
msgPayload=make([]byte, len(data))
copy(msgPayload, data)
}
I've done some preliminary benchmark tests locally. The result showed that this fix could gain approximately 5%~6% performance boost for large message mayloads sized at 5K.
How does this small fix look here?
The text was updated successfully, but these errors were encountered:
I think this could work indeed. I would not call the boolean largeMsgBuf since actually any time we make a copy of the message buffer in the parser, processMsg() should not have to copy it. So copied or something like that may be better. For instance, it could be that the message arguments (PUB ...) takes quite a bit of the scratch buffer and the message payload itself is not that big, but does not fit into the scratch buffer.
Also, you are not showing where the new boolean is set, but I would think that you need to set it in this "if" statement:
if nc.ps.ma.size > cap(nc.ps.scratch)-len(nc.ps.argBuf) {
nc.ps.copied = true
...
Also, need to reset this boolean to false after processing the message in the parser code.
In function
func (nc *Conn) processMsg(data []byte)
there's a snippet of codeIt appears that every time we see a new message coming in, the payload gets copied into a new buffer. This is reasonable for small payloads since we are reusing the scratch buffer
scratch [MAX_CONTROL_LINE_SIZE]byte
. But for large payloads a new buffer area sized as that of the actual message payload has already been created by invokingmake([]byte)
Here I think we may avoid this unnecessary copy for large message payloads within
processMsg(data []byte)
by introducting a bool flag, saylargeMsgBuf
, indicating that we don't need to copy the message payload a second timeI've done some preliminary benchmark tests locally. The result showed that this fix could gain approximately 5%~6% performance boost for large message mayloads sized at 5K.
How does this small fix look here?
The text was updated successfully, but these errors were encountered: