Skip to content

replace treemap with a balanced tree #4487

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

Closed
wants to merge 22 commits into from
Closed

replace treemap with a balanced tree #4487

wants to merge 22 commits into from

Conversation

thestinger
Copy link
Contributor

This is a full replacement for TreeMap as a balanced tree (AA Tree). I started from scratch so there aren't many similarities with the old implementation.

Improvements over the old implementation:

  • neither the key or value type needs to implement Copy or Clone (only uses moves) and there's no usage of @
  • insert, find and remove are O(log n) in the worst case instead of O(n) (the old TreeMap didn't actually have remove)
  • O(n) implementation of Eq
  • TreeSet wrapper, which will include O(n + m) implementations of is_disjoint, is_subset, is_superset, difference, symmetric_difference, intersection and union

In the future someone might want to replace this with a full red-black tree, but I think the performance will be pretty competitive as-is (and the asymptotic complexity would be the same). The constant factor for insertion is likely higher than a full rbtree implementation, but searching will be a bit quicker since the tree will usually be a bit more balanced.

Closes #2010, #2808.

@brson
Copy link
Contributor

brson commented Jan 14, 2013

This rewrite looks good to me. Can you run it through make tidy and fix the reported errors?

@thestinger
Copy link
Contributor Author

@brson: tidy warnings are fixed now (I just fixed all the things that had FIXMEs). It's all working well so it should be good to merge.

@thestinger thestinger closed this Jan 18, 2013
@thestinger thestinger deleted the tree branch January 18, 2013 18:06
@graydon
Copy link
Contributor

graydon commented Jan 18, 2013

landed in c7abdd3

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