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

btreesets #65

Open
konsumlamm opened this issue Jan 3, 2021 · 2 comments
Open

btreesets #65

konsumlamm opened this issue Jan 3, 2021 · 2 comments

Comments

@konsumlamm
Copy link

Fusion has a btreetables module, which mimics the tables module from the Nim standard library. Are there any plans to also add a btreesets module (which would possibly mimic the sets module from the standard library)? An implementation could probably use btreetables internally or just copy the implementation.

@timotheecour
Copy link
Member

An implementation could probably use btreetables internally or just copy the implementation.

same comment as nim-lang/RFCs#200 (comment)

wrapper is not re-implementation
My main problem with nim's Table/HashSet/Intests/etc apis is not the number of unique table types, but the fact that these are more or less full re-implementations (with some partial code reuse). MultiTable as proposed here is just a thin wrapper, and in fact has a Table as a field. So all the complex aspects (optimizing performance for collisions, handling tombstones etc) is already taken care of. IMO other table types should be thin wrappers, eg HashSet is a prime candidate; and I believe performance would be at least equal to current HashSet performance; there are other cases.
The only justification for re-implementation is a benchmark showing performance of a wrapper is not as good as a re-implementation in at least some meaningful cases; I haven't seen such benchmark.

@konsumlamm
Copy link
Author

I also like the API of std/critbits, which allows to set the value type to void, making the mapping/table behave like a set. This way, we can avoid code duplication at a very low cost, we'd just need a type alias and maybe some set-specific wrapper procs.

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

No branches or pull requests

2 participants