This looks concurrency safe to me too. Here's style suggestions:
Instead of
type memoryCache struct {
Data map[string]string
mux sync.Mutex
}
You could have
type memoryCache struct {
Data map[string]string
*sync.RWMutex
}
func (c memoryCache) Set(key string, val string) {
defer c.Unlock()
c.Lock()
c.Data[key] = val
}
func (c memoryCache) Get(key string) (string, bool) {
defer c.RUnlock()
c.RLock()
val, ok := c.Data[key]
return val, ok
}
A map is already pointing to a backing data structure, so having it behind
a pointer adds an unnecessary dereference. sync mutexes work well with
struct embedding. RWMutex is good for cases where there may be multiple
concurrent reads and only a few writes, although profiling is the right way
to make performance choices like that.
Matt
On Monday, January 8, 2018 at 12:10:07 PM UTC-6, Jake Montgomery wrote:
> Your use of the sync.Mutex seems sound. At first glance, I see no
> concurrency problems.
>
> I would suggest running the program with the -race flag. If you make
> memoryCache a package, then perhaps a concurrent test that can be run
> with -race would also be good. In that case you may also want to consider
> renaming `Data` to `data` to prevent others from directly accessing it
> without mutex.
>
> - Jake
>
> On Monday, January 8, 2018 at 12:38:35 PM UTC-5, Yigit Oztemel wrote:
>>
>> I have a small question about golang concurrency. I'm using go for an API
>> project and in background, goroutines create/update caches and web requests
>> always get cached data. We are using redis for cache and I want to store a
>> copy of the cache in go variables so we can decrease local network traffic.
>> Because of that I created the following example:
>>
>> https://github.com/ygto/go-memory-cache/blob/master/main.go
>>
>> func main() {
>>
>> c := NewCache()
>> wg := sync.WaitGroup{}
>> wg.Add(100)
>> for i := 0; i < 100; i++ {
>> go func(i int) {
>> key := fmt.Sprintf("name_%d", i)
>> val := fmt.Sprintf("%d", i)
>> *c.Set(key, val)*
>> *wg.Done()*
>> }(i)
>> }
>> wg.Wait()
>> wg = sync.WaitGroup{}
>>
>> wg.Add(100)
>> for i := 0; i < 100; i++ {
>> go func(i int) {
>> key := fmt.Sprintf("name_%d", i)
>> if data, ok := c.Get(key); ok {
>> fmt.Println(data)
>> }
>> wg.Done()
>> }(i)
>> }
>> wg.Wait()
>> }
>> I highlighted the lines. In my real case, I'm using a framework and it
>> doesn't allow me to send variables c and wg as parameters to func
>>
>> My question is, Is the above code memory safe? Can it give "concurrent
>> map" error?
>>
>> Maybe it's not good but is it a pragmatic solution?
>>
>> Thanks
>>
>
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
