SafeMap Len() is Linear Time Instead of Constant and ForEach() is Unsafe

[nrhvyc]

While reading through the safemap package I noticed that Len() is being calculated with the sync.Map function s.data.Range(). It looks like this was previously constant time prior to this PR: https://github.com/anthdm/hollywood/pull/63/files because len() is constant for a traditional map.

It's worth noting that (sync.Map).Range() isn't safe since it's non blocking meaning (SafeMap[K,V]).ForEach() isn't either. Therefore it's not guaranteed to get an accurate length or an accurate slice of []*PID for children in actor.Context.

The (sync.Map).Range() function comments state:
// Range does not necessarily correspond to any consistent snapshot of the Map's contents: no key will be visited more than once, but if the value for any key is stored or deleted concurrently (including by f), Range may reflect any mapping for that key from any point during the Range call. Range does not block other methods on the receiver; even f itself may call any method on m.
So this would lead to a common concurrency bug: https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use.

Since safemap is used by children in actor.Context, if the number of children is sufficiently large, then calculating length will be slow and likely inaccurate if children are changing.

The performance gains of #63 traded off safety for speed. Depending on your thoughts, it might be worth rolling this back to have a lock in SafeMap.

Is the use case for children aligned with the optimization of sync.Map? It seems like it isn't. Note that according to the docs the sync.Map "type is optimized for two common use cases: (1) when the entry for a given key is only ever written once but read many times, as in caches that only grow, or (2) when multiple goroutines read, write, and overwrite entries for disjoint sets of keys. In these two cases, use of a Map may significantly reduce lock contention compared to a Go map paired with a separate Mutex or RWMutex."

[perbu]

the change was mine and to be honest I did it only based on the benchmarks I did at the time, not really taking the safety and semantics into account. The case for reverting it is good.

[anthdm]

@nrhvyc Thanks for your detailed write up. I think reverting is the right call.