deprecate/remove Builders
laher opened this issue · 2 comments
I don't think there's any need for the builders if/once this is merged: #35 . Adding multiple items at once can be achieved without using them.
So, I don't know for sure, but they seem redundant to me now.
- We could just delete them from the codebase.
- Alternatively, just mark them depracted and/or update the Example tests & README guidance
Maybe I'm missing something, IDK
Unfortunately the SetMany implementations are incorrect (#39, #32 (comment)).
I can think of ways to implement SetMany such that it may perform fewer copies in some cases.
While the SetMany function is executing, you have to keep track of which nodes are explicitly owned by the receiver map. These nodes are the ones that have been copied/allocated during SetMany, as these cannot be referenced by other maps.
For such nodes you can treat the mutable flag as true (but not recursively!).
However, this is not simple to implement under the current API, and I think the values that are to be bulk-inserted would have to have some special properties for this to be worthwhile (i.e. they should fall into a small number of hash buckets, otherwise we don't save any allocations at all).
After some recent discussions, I still think it's valid to recommend the creation via varargs but I don't know about fully deprecating the builders now. Closing. Ta