Concurrent map writes issues with using RWLock

Getting Help Code Review
1

I am trying to have a map that can store prices and their amount i want map values must be thread safe i am using sync.RWLock but i get concurrent map write error 6 or 8th time if i run again and again [Code]: https://play.golang.org/p/PuRiyrvqoQI

package main

import (
“fmt”
“time”
“sync”
)

type quantity struct {
sync.RWMutex
amount float64
}

type Entries map[float64]*quantity

func main() {
vals:=Entries{}
go vals.Adder(100,1)
go vals.Adder(100,1)
go vals.Adder(100,1)
go vals.Adder(100,1)
go vals.Adder(100,1)
go vals.Adder(100,1)
time.Sleep(time.Millisecond*1)
for i,val:= range vals{

fmt.Println(i,": ",val)
}

}

func (m Entries)Adder(price,amnt float64) {
_,ok:=m[price]
if !ok {
m[price]=&quantity{}
m[price].Lock()
m[price].amount+=amnt
m[price].Unlock()
return
}
m[price].Lock()
m[price].amount+=amnt
m[price].Unlock()
}

2

You use locks in a very strange way. I suggest you to use a global mux variable:

var mu = &sync.Mutex{}

and lock/unlock access to map as you need

mu.Lock()
...map operation (read/write)
mu.Unock()
3

but this approach will block other go routine to perform operations which can case me a longer delay

4

Even though you lock and unlock the values seperately, you still have concurrent access to the map that holds those locks.

You need to lock the map as a whole. There is no other way, fullstop.

If you are careful with Lock()ing only when actually writing and RLock() most of the time, it won’t be that bad.

5

and what about sync.map does this locks whole map or just entries of keys ?

6

It does some juggling around with a cached copy of the map and a dirty flag as far as I can tell from a very quick glance at the sources, but if it is more performant than simply locking everything, I’m not sure. Also please be aware of the fact, that it uses a regular Mutex if it locks.

7

then which one should i use sync.map or regular map ? i have developed a huge things using maps but concurrent write panics is a huge problem

8

Do a propper benchmar under real life load and sizes.

9

In this case you can implement your own dynamic structure with semaphores. But this is a solution only if you have really big structures. If not use maps with confidence because are dynamic structures and are pretty fast.

[le] Instead using a big map for something try to use many small maps grouped in an array of maps or whatever and lock the small maps as you need.

10

actually i am working on exchange project here i need In memory storage for orders to process order matching fast (these order are also stored in database ) currently i am using maps for prices as keys and arrays or slice of orders as values associated to keys
structure is described below
price : orders
1.02 : [ ]orders
1.03 : [ ] orders

i need to read from maps and change [ ] order array according to incoming order
problem is that map is not thread safe so concurrent writes panics occur
If i use locks then it locks whole map which casts me delays of milliseconds so what do you suggest for that kind of data structure

11

If it deals with real money if real people, please start by getting rid of floats, Google will tell why you shouldn’t use floats for monetary values.

And then just use the naïve approach and lock everything until this really becomes a bottleneck, do not try to optimise issues away you don’t have in production code.

2 Likes
12

i get your point “lock everything until this really becomes a bottleneck” and thanks for informing about not to use floats for real money

13

The book GO Programming language, Chapter 9.7, Concurrent non-blocking cache has some nice examples how to lock on a map entry instead of the whole map :wink: Check https://github.com/adonovan/gopl.io/tree/master/ch9

1 Like
14

thanks man