Skip to content

Commit

Permalink
reduce Conn.mu hold times (apache#1105)
Browse files Browse the repository at this point in the history
The streamPool is already internally serialized by the Go runtime and
designed for high scalability. But currently retrieval from that pool
is unnecessarily serialized by the Conn.mu instead.

Additionally when the pool is empty, the hold times are even higher,
also this channel allocation for the timeout channel eas in that lock scope.

All of this amounted to enough overhead to make the lock appear in
internal profiles.

Technically the only thing required to be serialized are the accesses to
the Conn.calls map, so limit lock scope to exactly these operations.
  • Loading branch information
nightlyone authored and Zariel committed May 5, 2018
1 parent ef2238f commit 0b2eb41
Showing 1 changed file with 11 additions and 10 deletions.
21 changes: 11 additions & 10 deletions conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -624,22 +624,23 @@ func (c *Conn) exec(ctx context.Context, req frameWriter, tracer Tracer) (*frame
// resp is basically a waiting semaphore protecting the framer
framer := newFramer(c, c, c.compressor, c.version)

c.mu.Lock()
call := c.calls[stream]
if call != nil {
c.mu.Unlock()
return nil, fmt.Errorf("attempting to use stream already in use: %d -> %d", stream, call.streamID)
} else {
call = streamPool.Get().(*callReq)
}
c.calls[stream] = call

call := streamPool.Get().(*callReq)
call.framer = framer
call.timeout = make(chan struct{})
call.streamID = stream
call.req = req

c.mu.Lock()
existingCall := c.calls[stream]
if existingCall == nil {
c.calls[stream] = call
}
c.mu.Unlock()

if existingCall != nil {
return nil, fmt.Errorf("attempting to use stream already in use: %d -> %d", stream, existingCall.streamID)
}

if tracer != nil {
framer.trace()
}
Expand Down

0 comments on commit 0b2eb41

Please sign in to comment.