Make similar changes to those made in sets.nim, including hcode, rightSize
rawGet/rawGetKnownHC result protocol, nextTry probe sequence to be the cache
friendlier h=h+1 which in turn allows supporting changing deletion to fix the
infinite loop bug with local rehashing which in turn has desirable properties
of graceful table aging when deletes do happen and also making insert-only
usage patterns no longer pay any time/space cost to check deleted status.
Unlike collections.sets, this module has add() for duplicate key inserts and
a 3rd type of table, CountTable. The first wrinkle is handled by introducing
a rawGetDeep for unconditionally adding entries along collision chains. This
point of CountTable seems to be space efficiency at 2 items per slot. These
changes retain that by keeping the val==0 => EMPTY rule and not caching hash
codes. putImpl is expanded in-place for CountTable since the new putImpl() is
too different. { Depending on table size relative to caches & key expense,
regular Table[A,B] may become faster than CountTable, especially if the basic
count update could be something like inc(mGetOrPut(t, key, 0)). }
Unit tests pass, but in this module those are much more of just a demo than
probing for bugs. Should exercise/test this a little more before merging.
The estimation of the initialSize as simply array len + 10 was too small for
for all but the smallest sets. It would not elide/skip one final enlarge().
That last one is actually always the most expensive enlarge(). Indeed, in a
series where one to start from tiny and build up the table..that last one is
about 50% of all the enlarging time in general. So, this simple and reasonable
optimization (compared to just starting at 64) was only helping about half as
much as it could.
Introduce a rightSize() proc to be the inverse to mustRehash(). Export it
to clients since pre-sizing is externally useful in set construction and the
current mustRehash rules are opaque and beyond the control of clients.
Also add test module logic to check that rightSize() and mustRehash() are
inverses in the appropriate sense..not really in a block/assertion throwing
unit test since this is a peformance nice-to-have issue rather than about
basic correctness. (Also, fix a too vs. two typo in doc comment.)
Replace state enum with a cached hash code which has the same memory overhead
and locality as the enum, but can really speed things up with non-integer-like
keys (keys for which either hash() or == take more than couple cycles, or where
the key data is "indirect" and might incur another cache miss). To function as
both empty/filled state and a hash code cache, it only needs to be ensured that
hash codes are non-zero for any real key. That is done at the one place in the
whole file hash() is called. Keep convention clear via isFilled() & isEmpty().
An isDeleted state will no longer be necessary as per below excl/inf loop fix.
Since some use sites know hc and some do not, re-factor rawGet into two forms -
one with known hash code and one with an unknown HC that returns it. Both forms
still return <0 on missing, but returns the much more informative "-1 - index".
That return can be quickly inverted by -1 - result to recover the index where
insert should happen, provided no modifications are made to the table in the
meantime. This protocol retains the prior <0 interface and also makes it easy
to avoid unnecessary duplicate search work in procs like containsOrInclImpl
(which formerly searched in the initial get and AGAIN in rawInsert). Strip the
searching part out of rawInsert to "make it even more raw". swap(s.data, n) a
bit earlier so rawGet and rawGetKnownHC can have similar parameter lists and
integrate well with rawInsert/code sharing between Set and OrderedSet impls.
This PR also fixes infinite looping upon too many deletes. [ The deleted state
(aka "tombstone") approach is vulnerable to the table filling up with deleted
items which forces giant scans for missing keys which could be anywhere. In
the version prior to this PR, table wraparound wasn't even detected yielding
infinite loops. ] This PR changes excl() from marking slots as deleted to Knuth
algo 6.4R, "local/incremental moveback rehashing" - adapted from Knuth's h->h-1
to the cache-friendlier h->h+1 probe sequence and adapted from "gotos" to a new
doWhile template. This method restores the table to a state that would have
resulted from pure inserts (in some order). Update nextTry accordingly. Since
linear probing can degrade a little faster, 50% rather than 66% may be a better
default growth threshold, but users should be able to adjust threshold anyway.
Old unit tests all pass. More extensive testing in this module is probably
warranted before taking similar enhancements over to collections.tables.
- SinglyLinkedRing's prepend was broken
- needed a tail so that prepend can work properly
- now append works as well, so I added it too
- simple testcase added as well
