Hello, I'm new to Nim and I need help understanding why the following code does not persist the table data when the write operation is run concurrently:
# nim c -r --threads:on -d:debug example.nim
import locks, threadpool, tables
from os import sleep
type
Foo = ref object
lock: Lock
data {.guard: lock.}: Table[string, int]
proc init(f: Foo): Foo =
f.lock.initLock()
f.lock.withLock:
f.data = initTable[string, int]()
return f
proc read(f: Foo, k: string): int =
f.lock.withLock:
return (if f.data.hasKey(k): f.data[k] else: -1)
proc write(f: Foo, k: string, v: int): void =
f.lock.withLock:
f.data[k] = v
let f = Foo().init()
for i in 0..1000:
spawn f.write("foo", i)
discard spawn f.read("foo")
sync()
sleep(500)
echo f.read("foo")
Upon its completion the above should print 1000 but it doesn't. With a very similar code in Go I'm able to have it work as expected:
package main
import (
"fmt"
"sync"
"time"
)
type Foo struct {
sync.RWMutex
data map[string]int
}
func (f *Foo) read(k string) int {
f.RLock()
defer f.RUnlock()
return f.data[k]
}
func (f *Foo) write(k string, v int) {
// This prevents `fatal error: concurrent map writes`
f.Lock()
f.data[k] = v
f.Unlock()
}
func main() {
foo := Foo{data: make(map[string]int)}
for i := 1; i < 1000; i++ {
go func() {
foo.write("foo", i)
foo.read("foo")
}()
}
time.Sleep(500 * time.Millisecond)
fmt.Println(foo.read("foo"))
}
Thanks in advance for your help and please let me know if the Nim code should be written differently in order to achieve the same result as compared to the Golang example.
Your program is non-deterministic. There is no enforced sequencing between the writers, so the behavior you get is "last writer wins", as it's commonly called.
All the spawned writer tasks are "racing" to make their respective changes, but there is no guarantee as to which order they'll proceed. So you'll effectively get a "random" output everytime.
even though the writes are enclosed in a critical section to avoid corrupting the data structure.
Thanks for the answer, please see my replies below:
All the spawned writer tasks are "racing" to make their respective changes, but there is no guarantee as to which order they'll proceed. So you'll effectively get a "random" output everytime.
This is not completely true. In comparison with the Go example which always prints 1000 upon completion, the Nim example always prints -1. There's no randomness in the output in either examples.
I'd appreciate If you could point me in the direction that would help me solve the problem, perhaps with examples or updated code. Thanks
This:
spawn f.write("foo", i)
discard spawn f.read("foo")
is not equivalent to this:
go func() {
foo.write("foo", i)
foo.read("foo")
}()
That said, I still have no idea why it prints -1.
The problem in this code is that nim has isolated threads, so Foo instance in writer and Foo instance in reader are DIFFERENT instances. Upon spawning the thread the f is deeply copied to it's thread local storage.
This code needs the shared lock and shared Foo instances using ptr and allocshared0
Thank you @QMaster! That was it, here's the updated example for reference:
# nim c -r --threads:on -d:debug example.nim
import tables, locks, threadpool
type
Foo = object of RootObj
lock: Lock
data {.guard: lock.}: Table[string, int]
FooPtr = ptr Foo
proc init(f: Foo): FooPtr =
var fp = cast[FooPtr](allocShared0(sizeof(Foo)))
fp.lock.initLock()
fp.lock.withLock:
fp.data = initTable[string, int]()
return fp
proc read(fp: FooPtr, k: string): int {.thread.} =
var f = cast[FooPtr](fp)
f.lock.withLock:
return (if f.data.hasKey(k): f.data[k] else: -1)
proc write(fp: FooPtr, k: string, v: int): void {.thread.} =
var f = cast[FooPtr](fp)
f.lock.withLock:
f.data[k] = v
proc main(): void =
let foo = Foo().init()
defer: deallocShared(foo)
for i in 0..1000:
spawn foo.write("foo", i)
discard spawn foo.read("foo")
echo foo.read("foo")
main()
Being new to Nim, which I must say is quite an aswesome language, is the code above idiomatic or is there a safer/simpler way to achieve what I'm trying to do?
@Recruit_main707
IIRC arc's/orc's heap is shared, so your original code should (?) work with it? You should try it, ptrs are not bad, but you can avoid the casting and stuff.
Ah you meant avoid casting in both read and write procs, yep, those seem to be redundant :)
As QMaster points out Nim spawn will deeply copy the element. That being said it's not a great idea to have a table with strings in a shared structure. Those strings will be owned by whichever thread created them, and the thread might free that memory if it finds it is no longer in use (in that thread).
That being said if you compile with --gc:arc your example works as you expect. With the new ARC GC we are allowed to share memory between threads like this and as such those spawn calls doesn't deep copy the memory. There's also no issue of sharing the string keys under ARC.
However as @boia01 pointed out you're not guaranteed to always get 1000, and I don't think you are guaranteed that in the Go version either. Since your threads to so little work it is probably most likely to happen that way, but it's not guaranteed.
BTW, you can replace this:
(if f.data.hasKey(k): f.data[k] else: -1)
with this:
f.data.getOrDefault(k, -1)
Thanks @PMunch, that makes sense. I'll play around with the different gc options in addition to enhancing this sample to have more guarantees.
Thanks for the tip @xigoi, that's a miss on me for not reading the docs in detail... :P