Skip to content

Nodes store complete routes as uint-based types #35

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

Open
wants to merge 7 commits into
base: pcronin/performance-feature-branch
Choose a base branch
from

Conversation

PatrickCronin
Copy link
Owner

We think a uint-based type is possibly more suitable for internally representing a rstrie than the bitslice type (which is currently implemented as []byte). Since we'll need to use uint32 for IPv4 route parts and uint128 (a custom type) for IPv6 route parts, each node will necessary contain a uint32 or a uint128, and thus we think having each node store its complete representation (conceptually, the route that the node represents) instead of just the bits unique to the node without consideration of the nodes and thus bits of the node's parents.

@coveralls
Copy link

coveralls commented Oct 6, 2022

Pull Request Test Coverage Report for Build 3808664512

  • 246 of 281 (87.54%) changed or added relevant lines in 6 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-2.8%) to 80.876%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/routesum/main.go 4 6 66.67%
pkg/routesum/routesum.go 26 29 89.66%
pkg/routesum/uint128/uint128.go 19 25 76.0%
pkg/routesum/routetype/v4.go 58 70 82.86%
pkg/routesum/routetype/v6.go 83 95 87.37%
Files with Coverage Reduction New Missed Lines %
pkg/routesum/routesum.go 1 91.11%
Totals Coverage Status
Change from base Build 3807508815: -2.8%
Covered Lines: 351
Relevant Lines: 434

💛 - Coveralls

@PatrickCronin
Copy link
Owner Author

Interesting, there's a few issues on the Internet that seem to suggest there was issues with profiling on OS X (I've been using an M1 MacBook Pro). For interest's sake I ran the profiles on an other Xeon-based server running a flavor of linux.

The bitslice program ran faster again, although this time it was closer.

bitslice-based radix nodes uint-based complete route nodes
real 43.576s 44.714s
user 59.703s 1m9.716s
sys 1.591s 1.518s

The profiles now contain boxes from the program, so at least I've got something to look at that makes sense.

profile-bitslice
profile-uint128

@PatrickCronin PatrickCronin force-pushed the pcronin/store-complete-routes-as-uints branch from ab8dd10 to 413ca9a Compare October 6, 2022 14:08
@PatrickCronin PatrickCronin changed the base branch from main to pcronin/performance-feature-branch October 6, 2022 15:18
@PatrickCronin PatrickCronin force-pushed the pcronin/performance-feature-branch branch from f6dc114 to b8782e3 Compare October 6, 2022 15:29
@PatrickCronin PatrickCronin force-pushed the pcronin/store-complete-routes-as-uints branch from 413ca9a to 4bcfa32 Compare October 6, 2022 18:27
@PatrickCronin PatrickCronin force-pushed the pcronin/store-complete-routes-as-uints branch from 4bcfa32 to 0df0069 Compare October 20, 2022 15:17
@PatrickCronin PatrickCronin force-pushed the pcronin/performance-feature-branch branch from 3191ad4 to 919d6ed Compare December 30, 2022 14:55
@PatrickCronin PatrickCronin force-pushed the pcronin/store-complete-routes-as-uints branch from 0df0069 to 165a329 Compare December 30, 2022 19:19
@PatrickCronin PatrickCronin force-pushed the pcronin/performance-feature-branch branch from 919d6ed to 1bb8216 Compare December 31, 2022 02:59
@PatrickCronin PatrickCronin force-pushed the pcronin/store-complete-routes-as-uints branch from 165a329 to 4ee17a1 Compare December 31, 2022 03:01
@PatrickCronin PatrickCronin force-pushed the pcronin/performance-feature-branch branch from ac52555 to 8f16126 Compare January 15, 2023 01:57
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.

2 participants