-
Notifications
You must be signed in to change notification settings - Fork 46
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
Merge luikore/hat-trie? #20
Comments
luikore's fork has some nice features, but when I last checked it I didn't like the API: instead of being based on callbacks it should have been based on iterator objects (which hold a state of the iteration). Callback-based C API is slower, and it limits what can be done in a wrapper. With a callback-based interface you can't implement |
It looks to me like his hattrie_walk is implemented with a callback, while hattrie_iter_with_prefix is done with an iterator. I'm not a C expert, however, so I'd defer to you if you told me different :) If I have that right, perhaps you could include hattrie_iter_with_prefix? And then we could work on an iterator-based hattrie_walk? |
I'm not a C expert either, and I'm not an author of this repo - let's wait for @dcjones :) |
See also: #10 |
Ah! I see, you implemented https://github.com/kmike/hat-trie ? Very cool. It seems like there would be a lot of benefit in making sure dcjones' repository works for all derivatives then (Python, Ruby, etc.). Thanks! |
I'll have a go at merging @luikore's changes. At a glance, it looks like it just adds features without breaking backwards compatibility, so it should be possible to have one version that works for everyone. The use of callbacks does make it harder to write language bindings, but it seems limited to @luikore Would you be open to this? I think it would be great if we pooled our efforts, unless there is a compelling reason to maintain two version. I'd be happy to give you commit access to this repo as well. |
@dcjones Thanks! I'd like to make a PR with Though, the On the callback interface: it is very simple to implement, and easier to lock, and not always slower (function call is very fast on some modern CPUs, and heap iterator may make a page fault ...), and (mainly) I was too lazy to extract the loop context into a new iterator. But I like the unified API too 😄 |
I'm really happy to have found hat-trie and want to give back. However, my use case is one that uses luikore's ruby gem, backed by a modified hat-trie. Rather than give back to just one repository, I wonder if it would be possible to merge his changes back and then discuss possible improvements in documentation and/or readability in code? Related: luikore/triez#4
The text was updated successfully, but these errors were encountered: