-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: add benchmark for peerQueue #204
base: main
Are you sure you want to change the base?
Conversation
b.ResetTimer() | ||
b.ReportAllocs() | ||
for i := 0; i < b.N; i++ { | ||
queue = newPeerQueue(ctx, peerStats) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I thought it's a bad func to bench but it's getting interesting.
We create a new peerQueue
for every session
which is created per every GetRangeByHeight
request. Here is the thing: in newPeerQueue
we init priority queue via heap.Init
function which creates queue in an optimal way (O(N)
instead of O(N*logN)
to be precise). BUT! we aren't using it correctly, we init heap with an empty slice and pushing elements one by one, which will result in O(N*logN)
work.
if we will change newPeerStats
to something like:
func newPeerStats(peers []*peerStat) peerStats {
ps := make(peerStats, len(peers))
copy(ps, peers)
heap.Init(&ps)
return ps
}
We can get the following results:
% go-perftuner bstat acopy.txt bcopy.txt
args: [acopy.txt bcopy.txt]name old time/op new time/op delta
PeerQueue/push_for_10-10 784ns ± 0% 546ns ± 1% -30.31% (p=0.004 n=5+6)
PeerQueue/push_for_100-10 7.86µs ± 1% 5.95µs ± 0% -24.29% (p=0.004 n=6+5)
PeerQueue/push_for_1000-10 84.5µs ± 0% 59.3µs ± 0% -29.80% (p=0.002 n=6+6)
PeerQueue/push_for_1000000-10 116ms ± 2% 134ms ± 0% +14.66% (p=0.004 n=6+5)
name old alloc/op new alloc/op delta
PeerQueue/push_for_10-10 448B ± 0% 280B ± 0% -37.50% (p=0.002 n=6+6)
PeerQueue/push_for_100-10 2.37kB ± 0% 1.10kB ± 0% -53.72% (p=0.002 n=6+6)
PeerQueue/push_for_1000-10 17.7kB ± 0% 8.4kB ± 0% -52.66% (p=0.002 n=6+6)
PeerQueue/push_for_1000000-10 44.9MB ± 0% 8.0MB ± 0% -82.19% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
PeerQueue/push_for_10-10 8.00 ± 0% 4.00 ± 0% -50.00% (p=0.002 n=6+6)
PeerQueue/push_for_100-10 11.0 ± 0% 4.0 ± 0% -63.64% (p=0.002 n=6+6)
PeerQueue/push_for_1000-10 14.0 ± 0% 4.0 ± 0% -71.43% (p=0.002 n=6+6)
PeerQueue/push_for_1000000-10 41.0 ± 0% 4.0 ± 0% -90.24% (p=0.002 n=6+6)
(no idea why pushing 1M peers resulted in +15% time, yet).
} | ||
|
||
for _, peerStats := range peers { | ||
var queue *peerQueue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think keeping queue
inside every subtest will be better to misuse it in the future (assume moving 1 subtest before another or so).
Overview
Added a benchmark for push/pop operations in peerQueue