Skip to content

Commit dbfd15c

Browse files
committed
core/connection: remove unnecessary defers
There are a bunch of unnecessary defers that incur a speed cost as defers are slower by ~2X than a plain function call. Noticed while instrumenting this driver with OpenCensus. For those that need numbers to motivate the change: $ benchstat before.txt after.txt name old time/op new time/op delta ConnectionDeficientSize-4 1.15ms ±77% 1.02ms ±55% ~ (p=0.280 n=10+10) ConnectionDeficientMessageBody-4 2.23ms ±20% 1.94ms ±24% -12.75% (p=0.035 n=10+10) ConnectionDeficientHeader-4 1.86ms ±41% 1.97ms ±27% ~ (p=0.423 n=9+8) ConnectionDeficientOpReply-4 2.06ms ±19% 2.11ms ±20% ~ (p=0.931 n=9+9) Given these benchmarks ```go package main import ( "context" "net" "testing" "github.com/mongodb/mongo-go-driver/core/address" "github.com/mongodb/mongo-go-driver/core/connection" ) type branch int const ( deficientSize branch = iota deficientMessageBody deficientHeader deficientOpReply ) func BenchmarkConnectionDeficientSize(b *testing.B) { benchmarkAndTakeBranches(b, deficientSize) } func BenchmarkConnectionDeficientMessageBody(b *testing.B) { benchmarkAndTakeBranches(b, deficientMessageBody) } func BenchmarkConnectionDeficientHeader(b *testing.B) { benchmarkAndTakeBranches(b, deficientHeader) } func BenchmarkConnectionDeficientOpReply(b *testing.B) { benchmarkAndTakeBranches(b, deficientOpReply) } func benchmarkAndTakeBranches(b *testing.B, behavior branch) { ln, err := net.Listen("tcp", ":0") if err != nil { b.Fatalf("Failed to get address: %v", err) } defer ln.Close() b.ReportAllocs() // Server routine go func() { id := uint64(0) for { conn, err := ln.Accept() id += 1 if err != nil { return } // Expecting: // 1. 4 bytes for Size // 2. 16 bytes for MessageHeader // 3. At least 36 bytes for the message body // 3. 16 bytes: reply.MessageHeader // 4. 4 bytes: CursorID // 5. 4 bytes: StartingFrom // 6. 4 bytes: Number returned // buf := new(bytes.Buffer) switch behavior { case deficientSize: // Expecting >= 4 bytes containing the size of the message but //we'll give them just 3 to trigger "unable to decode message length" conn.Write([]byte{0x01, 0x02, 0x03}) conn.Close() case deficientMessageBody: // Expecting >= 20 bytes of body, but we'll give // them 5 trigger "unable to read full message" bz := make([]byte, 5) bz[0] = 0x10 bz[1] = 0x00 bz[2] = 0x00 bz[3] = 0x00 conn.Write(bz) conn.Close() case deficientHeader: // Expecting >= 20 bytes of body so we'll give them 20 // with no opcode set trigger "opcode not implemented" bz := make([]byte, 20) bz[0] = 0x10 bz[1] = 0x00 bz[2] = 0x00 bz[3] = 0x00 conn.Write(bz) conn.Close() case deficientOpReply: // Expecting >= 20 bytes of body so we'll give them 20 // with an opcode set to "OpReply" i.e. 0x01 to trigger // "unable to decode OP_REPLY" bz := make([]byte, 20) bz[0] = 0x10 bz[1] = 0x00 bz[2] = 0x00 bz[3] = 0x00 // set the OpCode to "OpReply" i.e. 0x01 bz[12] = 0x01 conn.Write(bz) conn.Close() } } }() ctx := context.Background() addr := address.Address(ln.Addr().String()) for i := 0; i < b.N; i++ { conn, _, err := connection.New(ctx, addr) if err != nil { b.Errorf("Failed to create connection: %v", err) continue } _, err = conn.ReadWireMessage(ctx) _ = conn.Close() } } ```
1 parent de03a35 commit dbfd15c

File tree

1 file changed

+5
-5
lines changed

1 file changed

+5
-5
lines changed

core/connection/connection.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ func (c *connection) ReadWireMessage(ctx context.Context) (wiremessage.WireMessa
299299
var sizeBuf [4]byte
300300
_, err := io.ReadFull(c.conn, sizeBuf[:])
301301
if err != nil {
302-
defer c.Close()
302+
c.Close()
303303
return nil, Error{
304304
ConnectionID: c.id,
305305
Wrapped: err,
@@ -321,7 +321,7 @@ func (c *connection) ReadWireMessage(ctx context.Context) (wiremessage.WireMessa
321321

322322
_, err = io.ReadFull(c.conn, c.readBuf[4:])
323323
if err != nil {
324-
defer c.Close()
324+
c.Close()
325325
return nil, Error{
326326
ConnectionID: c.id,
327327
Wrapped: err,
@@ -331,7 +331,7 @@ func (c *connection) ReadWireMessage(ctx context.Context) (wiremessage.WireMessa
331331

332332
hdr, err := wiremessage.ReadHeader(c.readBuf, 0)
333333
if err != nil {
334-
defer c.Close()
334+
c.Close()
335335
return nil, Error{
336336
ConnectionID: c.id,
337337
Wrapped: err,
@@ -345,7 +345,7 @@ func (c *connection) ReadWireMessage(ctx context.Context) (wiremessage.WireMessa
345345
var r wiremessage.Reply
346346
err := r.UnmarshalWireMessage(c.readBuf)
347347
if err != nil {
348-
defer c.Close()
348+
c.Close()
349349
return nil, Error{
350350
ConnectionID: c.id,
351351
Wrapped: err,
@@ -354,7 +354,7 @@ func (c *connection) ReadWireMessage(ctx context.Context) (wiremessage.WireMessa
354354
}
355355
wm = r
356356
default:
357-
defer c.Close()
357+
c.Close()
358358
return nil, Error{
359359
ConnectionID: c.id,
360360
message: fmt.Sprintf("opcode %s not implemented", hdr.OpCode),

0 commit comments

Comments
 (0)