cats-effect icon indicating copy to clipboard operation
cats-effect copied to clipboard

Default constructor for `MapRef`

Open armanbilge opened this issue 1 year ago • 4 comments
trafficstars

We currently have many MapRef implementations, based on Ref[Map], sharding, ConcurrentHashMap, etc. We should have an apply constructor that chooses a reasonable default implementation, probably ConcurrentHashMap.

armanbilge avatar Sep 13 '24 22:09 armanbilge

@armanbilge I'd like to pick this up

yisraelU avatar Sep 16 '24 17:09 yisraelU

@BalmungSan raised a good point that this default constructor should only require a Concurrent constraint. We can pattern-match for Async to use ConcurrentHashMap implementation, otherwise maybe we can fallback to the sharded Ref[Map] with the number of shards determined by the number of processors.

armanbilge avatar Sep 16 '24 22:09 armanbilge

While we're at it, a nicer API for defaultedMapRef would also be welcome. E.g. a constructor that directly takes a default value, or an instance method on trait MapRef. MapRefOptionOps is probably a good place for it too.

Jasper-M avatar Sep 17 '24 09:09 Jasper-M

MapRefOptionOps is probably a good place for it too.

Yes, I think this would be my vote. I think should be easy to chain with the constructor as well so we won't need a separate constructor?

armanbilge avatar Sep 17 '24 19:09 armanbilge