status-im / nim-taskpools

Lightweight, energy-efficient, easily auditable threadpool
Other
101 stars 6 forks source link

Unable to use strings with --gc:orc #12

Open Benjamin-Lee opened 2 years ago

Benjamin-Lee commented 2 years ago

Hi @mratsim,

This library looks sweet! Most of my work deals with strings and seqs, so I was excited to see that this library works with them using ARC and ORC GCs. However, when I tried using it with them, I'm getting segfaults. I might be misunderstanding the limitations of the library though.

For reference, here's the code I used to test it. Just the example code but the term function returns a stringified float that is then parsed back:

import taskpools
import std/[strutils, math, cpuinfo]

# From https://github.com/nim-lang/Nim/blob/v1.6.2/tests/parallel/tpi.nim
# Leibniz Formula https://en.wikipedia.org/wiki/Leibniz_formula_for_%CF%80
proc term(k: int): string =
  if k mod 2 == 1:
    $(-4'f / float(2*k + 1))
  else:
    $(4'f / float(2*k + 1))

proc piApprox(tp: Taskpool, n: int): float =
  var pendingFuts = newSeq[FlowVar[string]](n)
  for k in 0 ..< pendingFuts.len:
    pendingFuts[k] = tp.spawn term(k) # Schedule a task on the threadpool a return a handle to retrieve the result.
  for k in 0 ..< pendingFuts.len:
    result += parseFloat(sync pendingFuts[k])     # Block until the result is available.

proc main() =
  var n = 1_000_000
  var nthreads = countProcessors()

  var tp = Taskpool.new(num_threads = nthreads) # Default to the number of hardware threads.

  echo formatFloat(tp.piApprox(n))

  tp.syncAll()                                  # Block until all pending tasks are processed (implied in tp.shutdown())
  tp.shutdown()

# Compile with nim c -r -d:release --threads:on --outdir:build example.nim
main()
quantimnot commented 2 years ago

I've run into intermittent segfaults with the same stack trace as the above example.

Here is the stack trace:

Traceback (most recent call last)
taskpools/taskpools.nim(171) workerEntryFn
taskpools/taskpools.nim(242) eventLoop
nim/lib/std/tasks.nim(79) invoke
nim/lib/std/tasks.nim(121) taskpool_term
taskpools/taskpools.nim(491) taskpool_term
taskpools/flowvars.nim(54) readyWith
taskpools/channels_spsc_single.nim(78) trySend
SIGSEGV: Illegal storage access. (Attempt to read from nil?)

I have found the above example to be intermittent as well.

I've tried Nim={1.7.1, 1.6.4, 1.4.8} and mm={orc, arc, boehm}.

They all fail except for Nim=1.4.8, so maybe there is a regression or incompatible change after that version.

@Benjamin-Lee Does this library work for you on Nim 1.4.8? I guess that is a workaround for now.

Edit: I also confirmed that my program works if I don't return a string or seq.