Skip to content

Speed operations by memory for prefix within node rather than dynamically #39

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 3 commits into
base: master
Choose a base branch
from

Conversation

copelaje
Copy link

@copelaje copelaje commented Jun 8, 2025

First off, love the code and approach. I'd work the problem nearly identically so I'm starting with the code you've graciously shared and making the necessary mods which should benefit all pytricia users.

This first MR makes no interface changes, but provides further speed improvement by eliminating unnecessary dynamic memory allocation for the prefix. This is particularly beneficial because nearly all access operations previously performed dynamic allocation of a prefix, and then nearly always throwing it away. Now it can be done stack allocation which is more efficient for this case, yielding ~20% speed improvements from prior baseline in my testing.

It looked like the existing code had this use in mind with the implementation of New_Prefix2 however it was not actually used that way. With this update I adjusted New_Prefix2 and directly replaced New_Prefix itself.

prefix is used in loads of places so the updates are extensive, but not complicated. All tests pass. Only warning is that while I attempted code updates for the py2 ifdef sections I don't have a good ability to test they are correct. At the point in the game it may be reasonable to just drop py2 support for newer versions anyways.

My next MR will be geared at addressing #20 to allow quick save/restore functionality for already build pytricia objects.

Thanks again!

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.

1 participant