Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Generics Support #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

adrianosela
Copy link

@adrianosela adrianosela commented Jan 20, 2025

Generics Support

  • Helps users of this package avoid panics due to wrong type-assertions.
  • Simplifies code for users of this package (avoids type assertions at runtime).
  • Adds interfaces for Trie and TrieLoader (making the previous Trie and TrieLoader structs the default implementation of those interfaces, renamed to private types)
  • Made lookup functions e.g. Find() more idiomatic by returning true/false for whether item was found
  • Split up Trie, TrieLoader, and utils into different files
  • Updates README for usage e.g. now iptrie.NewTrie[string]() vs before iptrie.NewTrie()

Benchmark results below: TL;DR; the same as before... theres a lot of variability...

Benchmark Before Changes

|   *(OPs/Sec)*   |       IPTrie        |     Infoblox      |       NRadix       |       Ranger       |
|-----------------|---------------------|-------------------|--------------------|--------------------|
| LoadNets Random | 350,489 (48.1%)     | 253,830 (34.9%)   | 728,095 (100.0%)   | 23,186 (3.2%)      |
| LoadNets Sorted | 1,044,655 (92.5%)   | 389,975 (34.5%)   | 1,129,496 (100.0%) | 33,403 (3.0%)      |
| Read Check      | 16,781,931 (100.0%) | 4,634,126 (27.6%) | 9,227,059 (55.0%)  | 11,067,407 (65.9%) |
| Read Lookup     | 5,246,513 (100.0%)  | 4,515,453 (86.1%) | N/A                | 876,058 (16.7%)    |

Benchmark After Changes

|   *(OPs/Sec)*   |       IPTrie        |     Infoblox      |       NRadix       |       Ranger       |
|-----------------|---------------------|-------------------|--------------------|--------------------|
| LoadNets Random | 345,581 (46.1%)     | 244,978 (32.7%)   | 749,788 (100.0%)   | 23,504 (3.1%)      |
| LoadNets Sorted | 1,040,587 (97.4%)   | 369,672 (34.6%)   | 1,067,840 (100.0%) | 33,596 (3.1%)      |
| Read Check      | 16,702,326 (100.0%) | 2,969,610 (17.8%) | 8,459,722 (50.6%)  | 10,335,672 (61.9%) |
| Read Lookup     | 4,972,233 (100.0%)  | 4,512,782 (90.8%) | N/A                | 725,826 (14.6%)    |

@adrianosela adrianosela force-pushed the master branch 4 times, most recently from 14dab3b to 16f6b4b Compare January 20, 2025 22:19
@adrianosela
Copy link
Author

Hey @phemmer! Thanks for this repo :) Is there interest in this change?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant