yl2chen / cidranger

Fast IP to CIDR lookup in Golang
MIT License
900 stars 106 forks source link

performance improvement on certain operation #39

Closed ldkingvivi closed 3 years ago

ldkingvivi commented 4 years ago

include #37 but remove unnecessary changes, add the benchmark on this as well. before

BenchmarkNetworkEqualIPv4-16             5917344           198 ns/op          64 B/op          6 allocs/op
BenchmarkNetworkEqualIPv6-16             4486786           273 ns/op          42 B/op          6 allocs/op

after

BenchmarkNetworkEqualIPv4-16            100000000           10.6 ns/op         0 B/op          0 allocs/op
BenchmarkNetworkEqualIPv6-16            100000000           10.9 ns/op         0 B/op          0 allocs/op

change newPathprefixTrie to not call newPrefixTree, save time on extra parseCIDR and NewNetwork before

BenchmarkNewPathprefixTriev4-16                                      2731194           440 ns/op         288 B/op         12 allocs/op
BenchmarkNewPathprefixTriev6-16                                      1610536           743 ns/op         456 B/op         16 allocs/op

after

BenchmarkNewPathprefixTriev4-16                                     13315774            88.3 ns/op        16 B/op          4 allocs/op
BenchmarkNewPathprefixTriev6-16                                      7967464           153 ns/op          64 B/op          4 allocs/op
coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 91.281% when pulling 9adafe1ea7c93728894840481caffc81717e6c20 on ldkingvivi:improve_performance into 7ff5a0e84593dad6fbd50551343618d7956b3c71 on yl2chen:master.

dotwaffle commented 3 years ago

LGTM! All the tests in my code pass when replacing with this modified version, looks like an easy win for double the performance here!

panga commented 3 years ago

This PR improves insert performance by 60% for my use case and also reduces heap memory usage by 25%. Can we get this merged and released?

yl2chen commented 3 years ago

I am sorry guys for the neglect here, been pretty swamped and missed the update here. Looks good and merged!