Go: context: optimize Err by removing defer

Created on 23 Oct 2017  ·  7Comments  ·  Source: golang/go

https://github.com/golang/go/blob/eca45997dfd6cd14a59fbdea2385f6648a0dc786/src/context/context.go#L335-L339

Defers currently result in lower performance vs. inline calls (#14939). See the following benchmarks for reference:

https://github.com/grpc/grpc-go/blob/master/benchmark/primitives/primitives_test.go#L140
https://github.com/grpc/grpc-go/blob/master/benchmark/primitives/primitives_test.go#L174

BenchmarkMutexWithDefer-12                  20000000            51.2 ns/op
BenchmarkMutexWithoutDefer-12               100000000           16.5 ns/op

Also note the following benchmarks:
https://github.com/grpc/grpc-go/blob/master/benchmark/primitives/primitives_test.go#L29,L62

BenchmarkSelectClosed-12                    50000000            20.3 ns/op
BenchmarkSelectOpen-12                      300000000            5.74 ns/op

This ultimately means that the following code is more optimal in the case where an error is rare:

select {
case <-ctx.Done():
  return ctx.Err()
default:
}

vs.

if err := ctx.Err(); err != nil {
  return err
}

Unless there are plans to fix #14939 soon (which would be ideal), a trivial refactor to remove the defer will result in a ~3x speedup. If this is done, it might also be interesting to take a one-time pass through everything in the standard packages and remove other trivial defers encountered.

FrozenDueToAge NeedsDecision Performance

Most helpful comment

So would I, but #14939 has been open for 1.5 years, and this change would only take a couple minutes to make, so it may be worth doing as a temporary workaround.

All 7 comments

I would frankly rather fix #14939, at least for the unconditional single defer case, than complicate code across the standard library.

So would I, but #14939 has been open for 1.5 years, and this change would only take a couple minutes to make, so it may be worth doing as a temporary workaround.

Fixed with CL 107137.

Thanks for the fix @bontibon!

@dfawley despite #14939 not having been fixed, your original request has been implemented by @bontibon's CL https://go-review.googlesource.com/c/go/+/107137 and commit https://github.com/golang/go/commit/0b9c1ad20d4d4120e30f05129987f44de57032be.

I'll close this issue, thank you @dfawley for the report/request, it'll def make grpc-go faster! Please reopen if needed.

As not to be a credit thief, it was actually @keegancsmith's CL, not mine.

Oops, thank you @keegancsmith for the CL :)

np

Was this page helpful?
0 / 5 - 0 ratings

Related issues

longzhizhi picture longzhizhi  ·  3Comments

OneOfOne picture OneOfOne  ·  3Comments

stub42 picture stub42  ·  3Comments

bradfitz picture bradfitz  ·  3Comments

myitcv picture myitcv  ·  3Comments