Skip to content

Commit ef60e8e

Browse files
committed
portfwd: Optimize errors.Is() checks
On the read data path, we had many checks like: if errors.Is(err, io.EOF) { return nil } There are 2 issues with these checks: 1. io.Copy() is documented to never return io.EOF, and current code matches the docs. 2. errors.Is() is expensive and should not be used in the fast path This change remove the checks when they are not needed, and it the case of handling Recv() which is documented to return io.EOF, or PacketCon.ReadFrom() which does not mention io.EOF semantics, move the check into a err != nil block. I did not make any measurements with lima, but dolthub.com did synthetic benchmarks: https://www.dolthub.com/blog/2024-05-31-benchmarking-go-error-handling/ Based on the benchmarks the difference in the case when err == nil is not big, but the code is more clear when we gather all error handling under a err != nil check. Signed-off-by: Nir Soffer <[email protected]>
1 parent c9285ae commit ef60e8e

File tree

1 file changed

+10
-15
lines changed

1 file changed

+10
-15
lines changed

pkg/portfwd/client.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,10 @@ func HandleTCPConnection(ctx context.Context, client *guestagentclient.GuestAgen
2929
rw := &GrpcClientRW{stream: stream, id: id, addr: guestAddr}
3030
g.Go(func() error {
3131
_, err := io.Copy(rw, conn)
32-
if errors.Is(err, io.EOF) {
33-
return nil
34-
}
3532
return err
3633
})
3734
g.Go(func() error {
3835
_, err := io.Copy(conn, rw)
39-
if errors.Is(err, io.EOF) {
40-
return nil
41-
}
4236
return err
4337
})
4438

@@ -64,10 +58,11 @@ func HandleUDPConnection(ctx context.Context, client *guestagentclient.GuestAgen
6458
buf := make([]byte, 65507)
6559
for {
6660
n, addr, err := conn.ReadFrom(buf)
67-
if errors.Is(err, io.EOF) {
68-
return nil
69-
}
7061
if err != nil {
62+
// https://pkg.go.dev/net#PacketConn does not mention io.EOF semantics.
63+
if errors.Is(err, io.EOF) {
64+
return nil
65+
}
7166
return err
7267
}
7368
msg := &api.TunnelMessage{
@@ -86,10 +81,10 @@ func HandleUDPConnection(ctx context.Context, client *guestagentclient.GuestAgen
8681
g.Go(func() error {
8782
for {
8883
in, err := stream.Recv()
89-
if errors.Is(err, io.EOF) {
90-
return nil
91-
}
9284
if err != nil {
85+
if errors.Is(err, io.EOF) {
86+
return nil
87+
}
9388
return err
9489
}
9590
addr, err := net.ResolveUDPAddr("udp", in.UdpTargetAddr)
@@ -134,10 +129,10 @@ func (g GrpcClientRW) Write(p []byte) (n int, err error) {
134129

135130
func (g GrpcClientRW) Read(p []byte) (n int, err error) {
136131
in, err := g.stream.Recv()
137-
if errors.Is(err, io.EOF) {
138-
return 0, nil
139-
}
140132
if err != nil {
133+
if errors.Is(err, io.EOF) {
134+
return 0, nil
135+
}
141136
return 0, err
142137
}
143138
if len(in.Data) == 0 {

0 commit comments

Comments
 (0)