Use of Concurrent Hashmap instead of Mutex<HashMap>

[starship-jr]

Hello everyone, First of all, thank you all for this incredible project and all your efforts !

I have a question about the use of the HashMap behind the Mutex. Could this be replaced by some concurrent HashMap, like dashMap for example ?

I am fairly new to rust, not to mention rust async and executor like tokio, so surely my thoughts are naive.

If the use of a Mutex HahsMap is only usefull for controlling multi threading access to the Hashmap, from the tokio's tasks executor. Can we gain some speed with the use of a concurrent hashmap, with less coordination/contention on the tokio runtime ?

What is your opinion on this subject ?

[k0nserv]

Yes it might be a good idea to consider something like that, but the golden rule with performance is always to measure first and make changes later. Maybe the lock contention around various Mutex<HashMap> aren't actually a problem.

If you are curious about this and want to help, measuring performance is the best place to start

[starship-jr]

Thank you for the response. a might is enough for me right now.

Totally agree, without measuring, words or thoughts are of little use. I would love to contribute, I will need some time to understand the code base and think of proper scenarios to test.

I will be back if I have something interesting to report !

Wish you all the best

[k0nserv]

Thank you for the response. a might is enough for me right now.

I typically agree if it has no negative impacts otherwise, in this instance we'd be adding a new dependency so I think it warrants further scrutiny.

I would love to contribute, I will need some time to understand the code base and think of proper scenarios to test.

I think a fruitful way to progress this is to add lock tracking to our Mutex wrapper in util in the same way as quinn does it. Our wrappers currently only support the sync variants of locks but we could wrap the tokio::sync variants in a similar way.

Running one of the more taxing exmaples, e.g. broadcast, with lock tracking might gain us some insight

[starship-jr]

I typically agree if it has no negative impacts otherwise, in this instance we'd be adding a new dependency so I think it warrants further scrutiny.

Yes, should be extra carefull with it, some users reported issues with dashmap using tokio https://github.com/xacrimon/dashmap/issues/79 It was in an old release, but there is still an open issue in current version https://github.com/xacrimon/dashmap/issues/150
Dashmap/concurrent HashMap is just something I am curious about, seeing the Mutex<HashMap> in various sections of the code base. I will look further into it.

I think a fruitful way to progress this is to add lock tracking to our Mutex wrapper in util in the same way as quinn does it. Our wrappers currently only support the sync variants of locks but we could wrap the tokio::sync variants in a similar way.

Sounds great for std sync Mutex !

For the tokio Mutex, I see they optionally use the tracing feature in it, maybe we could already use it and wrap the std Mutex in the same way ? If relevant, I am not sure how to propely expose it to other crates, maybe using a feature flag in the util crate ?