=str Make use of ConcurrentHashMap instead of List to reduce contention.

[He-Pin]

Was: akka/akka#31262
refs: akka/akka#29557

Use an ConcurrentMap as set to trace the contention in onFeedbackDispatched

[mdedetrich]

Is it possible to provide jmh benchmarks to show the difference?

[He-Pin]

@mdedetrich I think this is a bug fix not a performance optimization, I can switch to ConcurrentHashMap i you like.

[mdedetrich]

Ah I see, I will get @jrudolph to review this since he made the original issue.

[He-Pin]

After read the paper of TrieMap, I think I need to change it back to ConcurrentHashMap.

However, we should point out that
the nonblocking hash table does not perform compression – once
the underlying table is resized to a certain size, the memory is
used regardless of whether the elements are removed. This can be
a problem for long running applications and applications using a
greater number of concurrent data structures

[mdedetrich]

lgtm

[pjfanning]

I'm a bit confused as to the use of AtomicReference[mutable.Map] instead of just plain mutable.Map.

[pjfanning]

lgtm

[jrudolph]

Yes, this was originally a performance improvement. Regardless of that fact, let's not just merge changes in performance critical code without running benchmarks or discussing arguments why they can be omitted.

It has been a long time since I worked on this / proposed the change (almost 3 years... oh my). I guess in this case, it's fine, it seems after all they ran some benchmarks in Akka to validate it does not introduce obvious performance regressions.

I'm a bit confused as to the use of AtomicReference[mutable.Map] instead of just plain mutable.Map.

The comment at its declaration explains it. There's a race condition between scheduling an async callback (i.e. an incoming external event into the stream) and shutting down a stream. The atomic reference makes sure, that we will report feedback to the async callback reliably even if it raced against shutdown (this is basically what this whole feature is about).