Keep track of number of set elements in map

[jonhoo]

This allows much more efficient versions of len() and is_empty(), and also enables ExactSizeIterator to be implemented for VecMap's iterators.

Fixes #32

[Gankra]

Could you add some more test cases to make sure all your changes are right? One thing off the top of my head: read some elements from the iterators and check that size_hint updates accordingly.

[jonhoo]

@gankro isn't that what https://github.com/contain-rs/vec-map/pull/33/files#diff-b4aea3e418ccdb71239b96952d9cddb6R1127 does?

[Gankra]

Oh wow, haha great! That's what I get for reviewing while falling asleep!

Lemme just double check with some people before hitting merge.

[Gankra]

hey @kbknapp you (clap-rs) seem to be the biggest user of VecMap; any thoughts on whether this would improve your use case, or just add overhead?

[kbknapp]

@gankro I recently switched to a bare Vec in my particular use case that would have benefited the most from this change. However, I would be interested to some benches as to whether the added branches affect the common cases in any meaningful way.

I have some worst case scenarios where len() could be called several times per arg (assuming LLVM doesn't optimize it down to one), and when going through thousands, or tens of thousands of args (such as shell glob expansions) this change could have net positive on this particular use case should I go back to VecMap (which is entirely possible).

Of course keeping in mind in the grand scheme of things, this is probably a very, very small portion of the total execution time.

---

That's kind of a non-answer of, "This change doesn't affect my use case right now at all." with the caveat of I'd like to see some benches for potential future use 😜

[jonhoo]

To me, this was mostly about the large (and potentially unexpected) slowdown for len() and is_empty() being more severe than the performance penalty of the on extra branch in insert and remove (which I suspect the branch predictor will also be really good at predicting). I'm currently working towards a major deadline, so unfortunately don't have time to run performance experiments at the moment, but I suspect that they'll show the impact to be minimal.

@kbknapp what kind of benchmark results is it you'd like to see? In particular, what's the workload you have in mind where this would arise (closed-loop add/remove from the map?)

[ofek]

Let's keep this alive.

[jonhoo]

@gankro @kbknapp bump

[Gankra]

Let's just go ahead with this. Should be trivial overhead.......

[jonhoo]

Nice! @gankro can you also do a (minor?) version bump?
EDIT: Actually, I guess it should maybe be major given the serde bump.

[Gankra]

0.8.0 published