Open kim opened 6 years ago
Parent: #123
An acceptable solution might also be stm-containers
>= 1, which has a O(1)
size
operation, with random element selection through UnfoldlM
.
Unfortunately, it will be a while until that version appears in lts, as it presumably breaks a lot of packages. In our codebase it breaks spock.
@kim Just realised that now we do have stm-containers >= 1
-- do you think we can crack on with this one, then?
@adinapoli-mndc I am wondering if what we come up with for #6 could influence the choice of datastructure here. E.g. some sort of priority queue / heap might make sense, so if we move to stm-containers
now, we would have to retrofit that on top.
That's a valid concern -- better tackle #6 first, and then let it guide the choice of the data structure here.
106 introduced
HashSet
instead ofSet
as the datastructure holding the “views” of the network from the POV of one node. The motivation was mainly the node identity typen
needs to carry information about the (last known) network address of a peer, in order to be able to reconnect to it. Imposing anOrd
constraint onn
seemed at least contrived.One issue now is that
HashSet
s don’t track their size, makingsize
O(n)
(as opposed toO(1)
forSet
); see tibbe/unordered-containers#138. It‘s not a big problem, since the number of elements stored is fairly small, but less than optimal becausesize
is used a lot.Another not-so-nicety is that
HashSet
doesn’t have an indexing function (a laelemAt
), requiring conversion to a list in order to select a random element (again occurs often in the algorithm).Two approaches come to mind:
HashSet
such that it’s size is tracked (could be tricky with updates), and a separateVector
or some such is maintained for the purpose of drawing random elements.IntMap
underneath and explicitly convert fromHashable
toInt
(Achtung: collisions)