Skip to content
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

Optimize is_bogon by 10-100x #50

Merged
merged 1 commit into from
Jan 3, 2024
Merged

Conversation

Alextopher
Copy link
Contributor

This PR improves the preformance of is_bogon by

  1. Avoiding a lot of duplicate work by pre-parsing IP networks in a lazy_static!.
  2. Treating IPv4 and IPv6 addresses seperately. A v4 address can never be part of a v6 subnet.

Some rough benchmarks puts this change at a >100x preformance improvement for is_bogon for ipv4 addresses.

I expiremented with using a radix tree. I found it preformed ~45% faster on random ipv6 addresses but ~33% worse on random ipv4 addresses. I don't think it's worth the additional complexity to use a trie, yet.

While making this change I thought it would also be valuable to add:

pub fn is_bogon_addr(ip_address: IpAddr) -> bool;

This allows users to avoid some memory allocations in some use cases. We might also find some uses within this library later.

# 10_000_000 random ipv4 addresses
new: 386.25305ms
old: 39.090128764s
Speedup: 101.203415

# 10_000_000 random ipv6 addresses
new: 4.575585554s
old: 48.682978251s
Speedup: 10.639727

@Alextopher
Copy link
Contributor Author

Alextopher commented Jan 2, 2024

This change should be semver minor change

Copy link
Contributor

@emberian emberian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great win!

@talhahwahla talhahwahla merged commit bb42047 into ipinfo:master Jan 3, 2024
1 check failed
@Alextopher Alextopher mentioned this pull request Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants