From 3c76eca6b6b393b7c66a22d382d1e763da1c658a Mon Sep 17 00:00:00 2001 From: Antonis Geralis Date: Mon, 12 Aug 2024 23:15:44 +0300 Subject: [PATCH 1/5] port to std/atomics --- threading/channels.nim | 25 +++++++++++++------------ threading/smartptrs.nim | 11 +++++++---- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/threading/channels.nim b/threading/channels.nim index b4d694c..eab2f32 100644 --- a/threading/channels.nim +++ b/threading/channels.nim @@ -100,8 +100,7 @@ runnableExamples("--threads:on --gc:orc"): when not (defined(gcArc) or defined(gcOrc) or defined(gcAtomicArc) or defined(nimdoc)): {.error: "This module requires one of --mm:arc / --mm:atomicArc / --mm:orc compilation flags".} -import std/[locks, isolation] -import ./atomics +import std/[locks, isolation, atomics] import system/ansi_c # Channel @@ -120,19 +119,19 @@ type # ------------------------------------------------------------------------------ -proc getTail(chan: ChannelRaw, order: Ordering = Relaxed): int {.inline.} = +proc getTail(chan: ChannelRaw, order: MemoryOrder = moRelaxed): int {.inline.} = chan.tail.load(order) -proc getHead(chan: ChannelRaw, order: Ordering = Relaxed): int {.inline.} = +proc getHead(chan: ChannelRaw, order: MemoryOrder = moRelaxed): int {.inline.} = chan.head.load(order) -proc setTail(chan: ChannelRaw, value: int, order: Ordering = Relaxed) {.inline.} = +proc setTail(chan: ChannelRaw, value: int, order: MemoryOrder = moRelaxed) {.inline.} = chan.tail.store(value, order) -proc setHead(chan: ChannelRaw, value: int, order: Ordering = Relaxed) {.inline.} = +proc setHead(chan: ChannelRaw, value: int, order: MemoryOrder = moRelaxed) {.inline.} = chan.head.store(value, order) -proc setAtomicCounter(chan: ChannelRaw, value: int, order: Ordering = Relaxed) {.inline.} = +proc setAtomicCounter(chan: ChannelRaw, value: int, order: MemoryOrder = moRelaxed) {.inline.} = chan.atomicCounter.store(value, order) proc numItems(chan: ChannelRaw): int {.inline.} = @@ -262,9 +261,8 @@ template frees(c) = if c.d != nil: # this `fetchSub` returns current val then subs # so count == 0 means we're the last - if c.d.atomicCounter.fetchSub(1, AcqRel) == 0: - if c.d.buffer != nil: - freeChannel(c.d) + if c.d.atomicCounter.fetchSub(1, moAcquireRelease) == 0: + freeChannel(c.d) when defined(nimAllowNonVarDestructor): proc `=destroy`*[T](c: Chan[T]) = @@ -273,15 +271,18 @@ else: proc `=destroy`*[T](c: var Chan[T]) = frees(c) +proc `=wasMoved`*[T](x: var Chan[T]) = + x.d = nil + proc `=dup`*[T](src: Chan[T]): Chan[T] = if src.d != nil: - discard fetchAdd(src.d.atomicCounter, 1, Relaxed) + discard fetchAdd(src.d.atomicCounter, 1, moRelaxed) result.d = src.d proc `=copy`*[T](dest: var Chan[T], src: Chan[T]) = ## Shares `Channel` by reference counting. if src.d != nil: - discard fetchAdd(src.d.atomicCounter, 1, Relaxed) + discard fetchAdd(src.d.atomicCounter, 1, moRelaxed) `=destroy`(dest) dest.d = src.d diff --git a/threading/smartptrs.nim b/threading/smartptrs.nim index 36a7b12..05286a8 100644 --- a/threading/smartptrs.nim +++ b/threading/smartptrs.nim @@ -7,7 +7,7 @@ # distribution, for details about the copyright. ## C++11 like smart pointers. They always use the shared allocator. -import std/isolation, ./atomics +import std/[isolation, atomics] from typetraits import supportsCopyMem proc raiseNilAccess() {.noinline.} = @@ -93,7 +93,7 @@ template frees(p) = if p.val != nil: # this `fetchSub` returns current val then subs # so count == 0 means we're the last - if p.val.counter.fetchSub(1, AcqRel) == 0: + if p.val.counter.fetchSub(1, moAcquireRelease) == 0: `=destroy`(p.val.value) deallocShared(p.val) @@ -104,14 +104,17 @@ else: proc `=destroy`*[T](p: var SharedPtr[T]) = frees(p) +proc `=wasMoved`*[T](p: var SharedPtr[T]) = + p.val = nil + proc `=dup`*[T](src: SharedPtr[T]): SharedPtr[T] = if src.val != nil: - discard fetchAdd(src.val.counter, 1, Relaxed) + discard fetchAdd(src.val.counter, 1, moRelaxed) result.val = src.val proc `=copy`*[T](dest: var SharedPtr[T], src: SharedPtr[T]) = if src.val != nil: - discard fetchAdd(src.val.counter, 1, Relaxed) + discard fetchAdd(src.val.counter, 1, moRelaxed) `=destroy`(dest) dest.val = src.val From ac631fcf7cc89db65b6bc395f5026a58afd1c24d Mon Sep 17 00:00:00 2001 From: Antonis Geralis Date: Mon, 12 Aug 2024 23:16:43 +0300 Subject: [PATCH 2/5] deprecate atomics --- threading/atomics.nim | 2 ++ 1 file changed, 2 insertions(+) diff --git a/threading/atomics.nim b/threading/atomics.nim index e4f922e..a218e6b 100644 --- a/threading/atomics.nim +++ b/threading/atomics.nim @@ -39,6 +39,8 @@ runnableExamples("--threads:on"): loc.atomicInc(1) assert loc.load == 6 +{.deprecated: "use `std/atomics` instead".} + when not compileOption("threads"): {.error: "This module requires --threads:on compilation flag".} From 7652bb15fb000634204c770ac43160166afe8e4f Mon Sep 17 00:00:00 2001 From: Antonis Geralis Date: Mon, 12 Aug 2024 23:20:29 +0300 Subject: [PATCH 3/5] fixed smartpointers --- threading/smartptrs.nim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/threading/smartptrs.nim b/threading/smartptrs.nim index 05286a8..de76d5a 100644 --- a/threading/smartptrs.nim +++ b/threading/smartptrs.nim @@ -122,7 +122,7 @@ proc newSharedPtr*[T](val: sink Isolated[T]): SharedPtr[T] {.nodestroy.} = ## Returns a shared pointer which shares ## ownership of the object by reference counting. result.val = cast[typeof(result.val)](allocShared(sizeof(result.val[]))) - result.val.counter.store(0) + result.val.counter.store(0, moRelaxed) result.val.value = extract val template newSharedPtr*[T](val: T): SharedPtr[T] = @@ -135,7 +135,7 @@ proc newSharedPtr*[T](t: typedesc[T]): SharedPtr[T] = result.val = cast[typeof(result.val)](allocShared0(sizeof(result.val[]))) else: result.val = cast[typeof(result.val)](allocShared(sizeof(result.val[]))) - int(result.val.counter) = 0 + result.val.counter.store(0, moRelaxed) proc isNil*[T](p: SharedPtr[T]): bool {.inline.} = p.val == nil From 32f87812866c39e7a107307855d6a05055f972fc Mon Sep 17 00:00:00 2001 From: Antonis Geralis Date: Tue, 13 Aug 2024 00:53:46 +0300 Subject: [PATCH 4/5] add test --- tests/tchannels_destroy.nim | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/tchannels_destroy.nim diff --git a/tests/tchannels_destroy.nim b/tests/tchannels_destroy.nim new file mode 100644 index 0000000..78d374b --- /dev/null +++ b/tests/tchannels_destroy.nim @@ -0,0 +1,31 @@ +discard """ + matrix: "--threads:on --gc:orc; --threads:on --gc:arc" + disabled: "freebsd" +""" + +import threading/channels, std/os + +type + WorkRequest = ref object + id: int + +var + chanIn: Chan[WorkRequest] + thread: Thread[Chan[WorkRequest]] + +proc workThread(chanIn: Chan[WorkRequest]) {.thread.} = + echo "Started work thread" + var req: WorkRequest + chanIn.recv(req) + echo "Got work ", req.id + +proc main = + chanIn = newChan[WorkRequest]() + createThread(thread, workThread, chanIn) + + chanIn.send(WorkRequest(id: 1)) + + sleep(100) # Give thread time to run + # joinThread(thread) + +main() From 56ad66b1f09344ac07da7198f9bd8110ccaa9188 Mon Sep 17 00:00:00 2001 From: Antonis Geralis Date: Tue, 13 Aug 2024 01:04:57 +0300 Subject: [PATCH 5/5] remove test --- tests/tchannels_destroy.nim | 31 ------------------------------- 1 file changed, 31 deletions(-) delete mode 100644 tests/tchannels_destroy.nim diff --git a/tests/tchannels_destroy.nim b/tests/tchannels_destroy.nim deleted file mode 100644 index 78d374b..0000000 --- a/tests/tchannels_destroy.nim +++ /dev/null @@ -1,31 +0,0 @@ -discard """ - matrix: "--threads:on --gc:orc; --threads:on --gc:arc" - disabled: "freebsd" -""" - -import threading/channels, std/os - -type - WorkRequest = ref object - id: int - -var - chanIn: Chan[WorkRequest] - thread: Thread[Chan[WorkRequest]] - -proc workThread(chanIn: Chan[WorkRequest]) {.thread.} = - echo "Started work thread" - var req: WorkRequest - chanIn.recv(req) - echo "Got work ", req.id - -proc main = - chanIn = newChan[WorkRequest]() - createThread(thread, workThread, chanIn) - - chanIn.send(WorkRequest(id: 1)) - - sleep(100) # Give thread time to run - # joinThread(thread) - -main()