concurrent-map
concurrent-map copied to clipboard
Add non locking versions of Get, Set and Has
I have personally had a requirement to do multiple consecutive operations using the same key with the requirement that nothing can change from start to end. To enable this, I propose explicitly named versions of the Get, Set and Has functions which do not take out locks on the shard. Instead, it is up to the user to use the GetShard method and lock/unlock it correctly.
The three functions would look like
func (m ConcurrentMap) UnlockedSet(key string, value interface{})
func (m ConcurrentMap) UnlockedGet(key string) (interface{}, bool)
func (m ConcurrentMap) UnlockedHas(key string) bool
I'm not sure about the naming. Maybe UnsafeGet, UnsafeSet and UnsafeHas is better?
Here's an example of how I think this could be useful
shard := conMap.GetShard(key)
shard.RLock()
pointerToMyStruct, ok := conMap.UnlockedGet(key)
// Some other thread might be trying to write to this same key but, we still have a lock!
if ok {
pointerToMyStruct.ReadSomeCoolInfo()
}
shard.RUnlock()
// Now the write on the other thread will go-ahead
In my case, I'm developing a cache where entries expire. I want to ensure that if the value has expired when the Get call is made, it's still expired when when ReadSomeCoolInfo call is made. If another thread were allowed to refresh the value between the two functions, it would cause me to return the wrong information.
The alternative to these functions would be to expose items on the shards to other packages and let the developer dig in and do it all manually.
Hi! First of all, I like your username. It's like a hardcore scotch distillery ❤️
Your proposal makes a lot of sense. My only concern is that I feel that 99% of users that end up using this repo are just looking for a way to set and get stuff. So any additional public APIs will make the library less user-friendly.
This is why I think exposing items on the shards to other packages makes more sense.
WDYT?
Thank you!
Keeping the public API as simple as possible does make sense. Exposing the underlying members can sometimes be just as confusing as a complicated API to inexperienced developers.
In this case, however, the items member is protected to some degree by the fact that users have to go out of their way to get the shard first. This is outside of the get/set basic usage enough that the average user probably won't go down this path and access items mistakenly.
Just exposing items makes it a very small PR too 😁
Maybe if we called it InternalItems // here be dragons or something like that?
Yeah, I think that's good! Shall I make the change and open a PR?
@orcaman @Lochlanna Guys, I have another simple idea, we can add Get Callback to Get method like that:
// GetCb is a callback executed in a map.Get() call, while RLock is held
type GetCb[V any] func(v V, exists bool)
// Get retrieves an element from map under given key.
func (m ConcurrentMap[V]) Get(key string, cbs ...GetCb[V]) (V, bool) {
// Get shard
shard := m.GetShard(key)
shard.RLock()
// Get item from shard.
val, ok := shard.items[key]
for _, cb := range cbs {
cb(val, ok)
}
shard.RUnlock()
return val, ok
}
The most important thing is No Extra public methods, signature compatibility