Skip to content
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

Impl cow_trie based on trie #94

Closed
wants to merge 1 commit into from
Closed

Impl cow_trie based on trie #94

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 18, 2015

This is incomplete, documentation needs to be updated. I just wanted to see if this was something other people would be interested in adding to this library.

This is the TrieMap with all the Boxed pointers replaced with Arc references. This lets people modify a small number of nodes in a CowTrieMap without having to clone the entire CowTrieMap. Nodes are then shared between the old copy of the old copy and the new one.

When modifying the CowTrieMap at a slow rate (a few keys per generation) I have benched marked this as about ~250 times faster then a the standard TrieMap.

@Gankra
Copy link
Owner

Gankra commented Jan 23, 2015

Oh shit I saw this on my phone and promptly forgot. If I don't look at this tomorrow someone poke me.

@ghost
Copy link
Author

ghost commented Jan 29, 2015

@gankro poke

@Gankra
Copy link
Owner

Gankra commented Jan 29, 2015

Ah sorry, got super sick and have been high on cough medecine for a while. So is this basically just "toss and Arc in TrieMap", or is it more complex than that?

@Gankra
Copy link
Owner

Gankra commented Jan 29, 2015

CC @michaelsproul

@Gankra
Copy link
Owner

Gankra commented Jan 29, 2015

One concern is that I think there's a general agreement that TrieMap is kinda weird (only works with uint, doesn't have path compression), so building out other stuff that duplicates its design is a bit nasty.

@michaelsproul
Copy link

I am of the personal opinion that the TrieMap code should be respectfully abandoned. I think our efforts would be better concentrated on a Patricia Trie, and I have started work to that end here:

https://github.com/michaelsproul/rust_patricia_trie

So far I just have an insert method, but it seems to work and I am hopeful about the chosen representation.

If anyone would like to help, I'd greatly appreciate it! I'm devoting my time to a whole lot of projects for uni and won't really be able to work on the Trie again until April.

@ghost
Copy link
Author

ghost commented Jan 30, 2015

@gankro It's pretty straight forward. There is some changes needed to support mutation using make_unique but other then that it is just a TrieMap with Arc pointers. I have been using it in one of my projects as the primary way to store generational data-sets with good success. Since I am mapping uints to values this is very close to an ideal data-structure.

I could also maintain this library outside of collect-rs. I just felt it fell into the description of collect-rs

Ideas that are too niche, crazy, or experimental to land in libcollections

That being said, I do agree that TrieMap could be better. I'm just would rather have a good enough data structure now, rather then questing for the perfect data structure; at the cost of working on the problems I want to actually invest time in.

@ghost ghost closed this Jan 30, 2015
@Gankra
Copy link
Owner

Gankra commented Jan 30, 2015

:(

I was just typing up that I was reasonably convinced that we could land this.

@Gankra
Copy link
Owner

Gankra commented Jan 30, 2015

In particular I'm interested in exploring the space of persistent collections.

Although we're getting kinda bloaty, I think I need to do what @huonw suggested with adding a feature-per-collection.

@ghost
Copy link
Author

ghost commented Jan 30, 2015

Sorry, I might have been premature in closing this. I should probably rebase these changes ontop of of the current TrieMap as there has been a number of changes done in the last two weeks since I hacked this together.

This pull request was closed.
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.

None yet

3 participants