Try to fix the "deadlock" in batch receive (#48)

* Try to fix the "deadlock" in batch receive, somehow it's also 5% faster (to be checked in Travis, deadlock not reproduced locally)

* Ignore nimble during runs nim-lang/nimble#696

* don't auto fail due to getTicks on ARM

* Negative count is non-blocking, can be investigated later (#49)

* When using C++ we need C++11

* typo

* Make travis Arch visible

* fix arch

Author: mratsim

.travis.yml

@@ -12,21 +12,32 @@ matrix:
     # Build and test using both gcc and clang
     # Build and test on both x86-64 and ARM64
     - os: linux
-      env: CHANNEL=devel
+      arch: amd64
+      env:
+        - ARCH=amd64
+        - CHANNEL=devel
       compiler: gcc
 
     - os: linux
       arch: arm64
-      env: CHANNEL=devel
+      env:
+        - ARCH=arm64
+        - CHANNEL=devel
       compiler: gcc
 
     - os: linux
-      env: CHANNEL=devel
+      arch: amd64
+      env:
+        - ARCH=amd64
+        - CHANNEL=devel
       compiler: clang
 
     # On OSX we only test against clang (gcc is mapped to clang by default)
     - os: osx
-      env: CHANNEL=devel
+      arch: amd64
+      env:
+        - ARCH=amd64
+        - CHANNEL=devel
       compiler: clang
   fast_finish: true
 
@@ -66,7 +77,7 @@ before_script:
     - export PATH="nim-${CHANNEL}/bin${PATH:+:$PATH}"
 script:
     - nimble refresh
-    - nimble install cligen
+    - nimble install cligen || true
     - nimble test
 branches:
   except:

azure-pipelines.yml

@@ -147,7 +147,7 @@ steps:
 
   - bash: |
       nimble refresh
-      nimble install cligen
+      nimble install cligen || true
     displayName: 'Building the package dependencies'
 
   - bash: |

weave/channels/channels_mpsc_unbounded_batch.nim

@@ -147,6 +147,7 @@ proc tryRecvBatch*[T](chan: var ChannelMpscUnboundedBatch[T], bFirst, bLast: var
   ## Returns the number of items received
   ##
   ## If no items are returned bFirst and bLast are undefined
+  ## and should not be used.
   ##
   ## ⚠️ This leaks the next item
   ##   nil or overwrite it for further use in linked lists
@@ -172,9 +173,10 @@ proc tryRecvBatch*[T](chan: var ChannelMpscUnboundedBatch[T], bFirst, bLast: var
     # We lose the competition, bail out
     chan.front.next.store(front, moRelaxed)
     discard chan.count.fetchSub(result, moRelaxed)
-    postCondition: chan.count.load(moRelaxed) >= 0
+    postCondition: chan.count.load(moRelaxed) >= 0 # TODO: somehow it can be negative
     return
 
+  # front == last
   chan.front.next.store(nil, moRelaxed)
   if compareExchange(chan.back, last, chan.front.addr, moAcquireRelease):
     # We won and replaced the last node with the channel front
@@ -190,19 +192,22 @@ proc tryRecvBatch*[T](chan: var ChannelMpscUnboundedBatch[T], bFirst, bLast: var
   # we assume that consumer has plenty of work to do with the
   # already retrived batch
   next = cast[T](front.next.load(moRelaxed))
-  if not next.isNil:
-    # Extra node after this one, no competition with producers
-    prefetch(front)
-    result += 1
-    discard chan.count.fetchSub(result, moRelaxed)
-    chan.front.next.store(next, moRelaxed)
-    fence(moAcquire)
-    bLast = front
-    postCondition: chan.count.load(moRelaxed) >= 0
-    return
+  while next.isNil:
+    # We spinlock, unfortunately there seems to be a livelock potential
+    # or contention issue if we don't use cpuRelax
+    # at least twice or just bail out if next is nil.
+    # Replace this spinlock by "if not next.isNil" and run the "memory pool" bench
+    # or fibonacci and the program will get stuck.
+    # The queue should probably be model checked and/or run through Relacy
+    cpuRelax()
+    next = cast[T](front.next.load(moRelaxed))
 
-  # The last item wasn't linked to the list yet, bail out
+  prefetch(front)
+  result += 1
   discard chan.count.fetchSub(result, moRelaxed)
+  chan.front.next.store(next, moRelaxed)
+  fence(moAcquire)
+  bLast = front
   postCondition: chan.count.load(moRelaxed) >= 0
 
 func peek*(chan: var ChannelMpscUnboundedBatch): int32 {.inline.} =

weave/config.nim

@@ -7,6 +7,12 @@
 
 import strutils
 
+# With C++ we need C++11
+# ----------------------------------------------------------------------------------
+
+when defined(cpp):
+  {.passC:"-std=c++11".}
+
 # Static configuration & compile-time options
 # ----------------------------------------------------------------------------------
 

weave/instrumentation/timers.nim

@@ -71,7 +71,8 @@ when defined(i386) or defined(amd64):
       lfence()
       return rdtsc()
 else:
-  {.error: "getticks is not supported on this CPU architecture".}
+  when defined(WV_profile):
+    {.error: "getticks is not supported on this CPU architecture".}
 
 template timer_new*(timer: var Timer, ghzClock: float64) =
   timer.elapsed = 0