I have a Go server that takes input from a number of TCP clients that stream in data. The format is a custom format and the end delimiter could appear within the byte stream so it uses bytes stuffing to get around this issue.
I am looking for hotspots in my code and this throws up a HUGE one and I'm sure it could be made more efficient but I'm not quite sure how at the moment given the provided Go functions.
The code is below and pprof shows the hotspot to be popPacketFromBuffer
command. This looks at the current buffer, after each byte has been received and looks for the endDelimiter on it's own. If there is 2 of them in a row then it is within the packet itself.
I did look at using ReadBytes()
instead of ReadByte()
but it looks like I need to specify a delimiter and I'm fearful that this will cut off a packet mid stream? And also in any case would this be more efficient than what I am doing anyway?
Within the popPacketFromBuffer
function it is the for loop that is the hotspot.
Any ideas?
// Read client data from channel
func (c *Client) listen() {
reader := bufio.NewReader(c.conn)
clientBuffer := new(bytes.Buffer)
for {
c.conn.SetDeadline(time.Now().Add(c.timeoutDuration))
byte, err := reader.ReadByte()
if err != nil {
c.conn.Close()
c.server.onClientConnectionClosed(c, err)
return
}
wrErr := clientBuffer.WriteByte(byte)
if wrErr != nil {
log.Println("Write Error:", wrErr)
}
packet := popPacketFromBuffer(clientBuffer)
if packet != nil {
c.receiveMutex.Lock()
packetSize := uint64(len(packet))
c.bytesReceived += packetSize
c.receiveMutex.Unlock()
packetBuffer := bytes.NewBuffer(packet)
b, err := uncompress(packetBuffer.Bytes())
if err != nil {
log.Println("Unzip Error:", err)
} else {
c.server.onNewMessage(c, b)
}
}
}
}
func popPacketFromBuffer(buffer *bytes.Buffer) []byte {
bufferLength := buffer.Len()
if bufferLength >= 125000 { // 1MB in bytes is roughly this
log.Println("Buffer is too large ", bufferLength)
buffer.Reset()
return nil
}
tempBuffer := buffer.Bytes()
length := len(tempBuffer)
// Return on zero length buffer submission
if length == 0 {
return nil
}
endOfPacket := -1
// Determine the endOfPacket position by looking for an instance of our delimiter
for i := 0; i < length-1; i++ {
if tempBuffer[i] == endDelimiter {
if tempBuffer[i+1] == endDelimiter {
i++
} else {
// We found a single delimiter, so consider this the end of a packet
endOfPacket = i - 2
break
}
}
}
if endOfPacket != -1 {
// Grab the contents of the provided packet
extractedPacket := buffer.Bytes()
// Extract the last byte as we were super greedy with the read operation to check for stuffing
carryByte := extractedPacket[len(extractedPacket)-1]
// Clear the main buffer now we have extracted a packet from it
buffer.Reset()
// Add the carryByte over to our new buffer
buffer.WriteByte(carryByte)
// Ensure packet begins with a valid startDelimiter
if extractedPacket[0] != startDelimiter {
log.Println("Popped a packet without a valid start delimiter")
return nil
}
// Remove the start and end caps
slice := extractedPacket[1 : len(extractedPacket)-2]
return deStuffPacket(slice)
}
return nil
}
Looks like you call popPacketFromBuffer()
each time every single byte received from connection. However popPacketFromBuffer()
copy hole buffer and inspect for delimeters every byte each tyme. Maybe this is overwhelming. For me you don't need loop
for i := 0; i < length-1; i++ {
if tempBuffer[i] == endDelimiter {
if tempBuffer[i+1] == endDelimiter {
i++
} else {
// We found a single delimiter, so consider this the end of a packet
endOfPacket = i - 2
break
}
}
}
in popPacketFromBuffer()
Maybe instead of loop just testing last two bytes
if (buffer[len(buffer)-2] == endDelimiter) && (buffer[len(buffer)-1] != endDelimiter){
//It's a packet
}
would be enough for purpose.