[WIP] NetworkDB performance improvements #2046
Conversation
fcrisciani
force-pushed
the
performance
branch
from
213939e
to
3ca0c60
Compare
Codecov Report
@@ Coverage Diff @@
## master #2046 +/- ##
=========================================
Coverage ? 40.05%
=========================================
Files ? 138
Lines ? 22108
Branches ? 0
=========================================
Hits ? 8856
Misses ? 11954
Partials ? 1298
Continue to review full report at Codecov.
|
CPU profile showed how the mRandomNodes was taking ~30% of the CPU time of the gossip cycle. Changing the data structure from a []string to a map allowed to improve the performance of all the functions that were using it. Following the comparison of the benchmarks before and after the change AddNodeNetwork: benchmark old ns/op new ns/op delta BenchmarkAddNetworkNode-4 1859 181 -90.26% benchmark old allocs new allocs delta BenchmarkAddNetworkNode-4 1 1 +0.00% benchmark old bytes new bytes delta BenchmarkAddNetworkNode-4 15 15 +0.00% DelNodeNetwork: benchmark old ns/op new ns/op delta BenchmarkDeleteNetworkNode-4 71.0 75.8 +6.76% benchmark old allocs new allocs delta BenchmarkDeleteNetworkNode-4 0 0 +0.00% benchmark old bytes new bytes delta BenchmarkDeleteNetworkNode-4 3 7 +133.33% RandomNode: benchmark old ns/op new ns/op delta BenchmarkRandomNodes-4 1830 172 -90.60% benchmark old allocs new allocs delta BenchmarkRandomNodes-4 16 1 -93.75% benchmark old bytes new bytes delta BenchmarkRandomNodes-4 535 48 -91.03% Signed-off-by: Flavio Crisciani <[email protected]>
fcrisciani
force-pushed
the
performance
branch
from
3ca0c60
to
6ba12b1
Compare
|
|
||
| var i int | ||
| for node := range nodes { |
There was a problem hiding this comment.
I think go does not guarantee the range is random as a spec. Will there be any side effects if a certain implementation of Go returns predictable range? Also is the impact of calling randomOffet rather significant?
There was a problem hiding this comment.
I have to check on that, I read that since go 1.0 the keys were randomized, but have to check is that is a common assumption for all the architectures, or at least the one that we support.
Yes the randomOffset is the bottlenek, that can be seen from the detailed flame graph, the rand.Int has 90% and 10% is the big.NewInt
There was a problem hiding this comment.
Ah I see. This thread has some details: https://groups.google.com/forum/#!topic/golang-nuts/zBcqMsDNt7Q . I guess it is safe to assume the randomness if there is a major perf gain and just have a comment about the assumption.
There was a problem hiding this comment.
It's not cryptographically random and also the test highlighted some unbalance in the results, but the test also makes sure that there is no node that never comes up in the selection (min == 0 condition)
Another approach that I was thinking about is to have the original string slice and an index that is saved with the network and loop on the slice in a circular buffer fashion. Considering that nodes not change every second, that should guarantee fairness in the selection and the randomness is guaranteed at insertion time (on the base of when the node join). The problem of the slice still remains the linear loop for each insertion and deletion that is pretty lame to pay.
| @@ -64,8 +64,8 @@ type NetworkDB struct { | |||
| networks map[string]map[string]*network | |||
|
|
|||
| // A map of nodes which are participating in a given | |||
| // network. The key is a network ID. | |||
| networkNodes map[string][]string | |||
| // network. The first key is a network ID. The second key is | |||
There was a problem hiding this comment.
Is the comment "second key is" incomplete?
fcrisciani
changed the title
|
@fcrisciani will this change potentially solve the limitation on the |
|
@KCrawley actually this is still pretty experimental and I think I will do other changes before having this one ready. |
CPU profile showed how the mRandomNodes was taking ~30% of the CPU time
of the gossip cycle.
Changing the data structure from a []string to a map allowed to improve
the performance of all the functions that were using it.
Following the comparison of the benchmarks before and after the change
Benchmark on:
AddNodeNetwork (90% faster):
DelNodeNetwork (8% faster):
RandomNode (90% faster and 93% less allocations):
Full profile:
Detail:
Signed-off-by: Flavio Crisciani [email protected]