Add benchmark test case for TURN/UDP profiling

[rg0now]

A new benchmark test is added that allows to stress the TURN/UDP client and the server in a simple setup. This allows to get an insight into the running times of the main TURN/UDP client-server routines with real I/O.

Usage:

• run: go test -cpuprofile cpu.prof -bench=. -run=XXX -benchtime=10s
• view profile: go tool pprof -http=":8001" ./cpu.prof &

[rg0now]

Overall, it seems pion/turn is doing fairly well in the benchmarks: judging from the flame graph (click View/Flame-graph in the GUI), roughly 65% of the running time is spent in I/O (UDPConn.WriteTo/ReadFrom), 20% is taken by the Go runtime and only the remaining 15% is spent on TURN client/server packet forging and business logics. I was particularly happy to not see excess packet-buffer/byte-slice copies all around the place, which is a frequent source of overhead in networking code.

One minor outlier is UDPAddr.String(): we spend almost 7% of the total running time stringifying UDP addresses. The major user is the allocation manager's GetAllocation() routine (>4% total running time) that calls Fivetuple.FingerPrint() per each received packet to find the allocation in the allocation map, effectively calling UDPAddr.Stringify() per each packet multiple times (see attached screenshot). I'm wondering whether we could (and should) try to optimize this.

Apart from this, however, pion/turn seems to be doing nicely and there do not seem to be major low-hanging optimization targets.

[stv0g]

One minor outlier is UDPAddr.String()

I think we should modernize the code and use the netip package here. We could use its AddrPort type to make the FiveTuple comparable and hence get rid of the stringification completely :)

[codecov]

Codecov Report

Base: 68.44% // Head: 68.92% // Increases project coverage by +0.48% 🎉

Coverage data is based on head (6715d4a) compared to base (a60ed1d).
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #298      +/-   ##
==========================================
+ Coverage   68.44%   68.92%   +0.48%     
==========================================
  Files          38       38              
  Lines        2475     2475              
==========================================
+ Hits         1694     1706      +12     
+ Misses        644      636       -8     
+ Partials      137      133       -4     

Flag	Coverage Δ
go	68.92% <ø> (+0.48%)	⬆️
wasm	45.55% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
client.go	71.72% <0.00%> (+0.77%)	⬆️
internal/client/transaction.go	94.68% <0.00%> (+9.57%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[stv0g]

Thanks :) Looks good to me!

I only have made a couple of small code-style fixes:

• err.Error() should not be necessary to print the error. Just use err. The %s format will call String() implicity on the error
• Error messages usually start with "Failed to ..."
• Both comments and log messages (excluding errors created by fmt.Errorf()) should start with a capital letter.

My changes are here abbc38b

Could you apply them to your branch, so we can merge them via this PR?
Unfortunately, I dont have the permission to push to your branch myself directly.

[stv0g]

And, I fixed some of the linter warnings which you silcenced via //nolint

[rg0now]

Thanks! I wish you didn't have to do all the extra work with my PRs, I'll try to stick the coding style.

As per netip.AddrPort: I guess the definition of FiveTuple would change as follows:

type FiveTuple struct {
	Protocol
	SrcAddr, DstAddr netip.AddrPort
}

Now I see that netip.AddrPort is safe for comparison, but is the struct it is embedded in also comparable? I mean, with the new definition can we write this?

// Equal asserts if two FiveTuples are equal
func (f *FiveTuple) Equal(b *FiveTuple) bool {
	return *f == *b
}

And then, can we also use the FiveTuple directly as a map key?

// Manager is used to hold active allocations
type Manager struct {
...
	allocations  map[FiveTuple]*Allocation
...
}

// GetAllocation fetches the allocation matching the passed FiveTuple
func (m *Manager) GetAllocation(fiveTuple *FiveTuple) *Allocation {
	return m.allocations[*fiveTuple]
}

My understanding is that this all would be OK, but I've never used netip nor struct-as-map-key myself.

And if this is all OK, are you willing to take a PR that makes this change? Is it OK to add a new package dependency?

[stv0g]

Thanks! I wish you didn't have to do all the extra work with my PRs, I'll try to stick the coding style.

Please do not worry about that. I am really happy to have the chance to do this review work.
I started to like it, as it allows me to learn about the Pion code-base step-by-step :)