Skip to content

Implement an O(1) stack treemap move iterator #10214

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

Conversation

alexcrichton
Copy link
Member

Because the move iterator contains ownership of the tree, we can destructure it
as we go (and modify the internal structure). This also allows us to make the
move iterator for a treemap invertable and exactly sized (for an enumerable
variant). Sadly, I know of no way to extend this to a normal iterator, so iter()
will still require a stack allocation for now.

Additionally, this implements the clear() method in terms of move_iter() to
force O(1) stack usage when clearing a tree. I tried to implement this in a Drop
implementation for TreeMap as well, but I ran into ICE problems about some
missing vtables, so I'll leave that implementation for a later date. The idea
would be that dropping a TreeMap requires O(1) stack space instead of O(depth)
stack space.

Because the move iterator contains ownership of the tree, we can destructure it
as we go (and modify the internal structure). This also allows us to make the
move iterator for a treemap invertable and exactly sized (for an enumerable
variant). Sadly, I know of no way to extend this to a normal iterator, so iter()
will still require a stack allocation for now.

Additionally, this implements the clear() method in terms of move_iter() to
force O(1) stack usage when clearing a tree. I tried to implement this in a Drop
implementation for `TreeMap` as well, but I ran into ICE problems about some
missing vtables, so I'll leave that implementation for a later date. The idea
would be that dropping a `TreeMap` requires O(1) stack space instead of O(depth)
stack space.
@huonw
Copy link
Member

huonw commented Nov 1, 2013

Does this work when alternating next and next_back? (If so, it's probably worth a test.)

@alexcrichton
Copy link
Member Author

Ah, I should probably add a test yes. It's incredibly inefficient (because you're rotating everything one way and then the other), but it does work.

@huonw
Copy link
Member

huonw commented Nov 1, 2013

Ah cool! Might be worth mentioning the inefficiency in the docs?

@thestinger
Copy link
Contributor

The maximum depth is log2(n) so on 32-bit it will never be above approximately 32. Is this actually faster than the previous method?

@alexcrichton
Copy link
Member Author

Hm, I didn't think too hard about this. A log2(n) depth is nothing to worry about at all.

@alexcrichton alexcrichton deleted the treemap-move-iter-no-stack branch November 1, 2013 02:47
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 27, 2023
…h,giraffate

Changelog for Rust 1.67 🐞

Roses are red,
violets are blue,
if changelogs could talk,
what would we do?

---

The usual disclaimer: This PR is written, as if the version was already released. It should be merged with the coming release on 2023-01-26. So, please provide feedback and approve it, if everything looks good, but let's wait with the r+ until the release :)

---

changelog: none
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