Need help with weave as background process

[olliNiinivaara]

I have refactored Guildenstern so that http event listener (selector) and Weave background process are now in different threads. When event comes in, it is submitted to Weave and then instantly spawned. Everything works as long as there's a sleep(10) command after submitting, otherwise Weave crashes as if I'm submitting events faster that it can process them.

The sleep line and the error message without it are here:

https://github.com/olliNiinivaara/GuildenStern/blob/1b1dfab9b3529eb45cce0e35df16c891b1a394f2/src/guildenstern/dispatcher.nim#L133

Clearly there's something wrong, but what?

[mratsim]

That's interesting. I'll looked into it.

[mratsim]

I've merged a fix in #157

You also need to update your code according to https://github.com/mratsim/GuildenStern/pull/1/commits/a1e22d4798a7475c25d3c0a6051c4aca43d3c647

1. With submit there is no need for spawnProcess.
2. processAllandTryPark is when your own eventLoop is on the same thread as Nim main thread, but when you call runInBackground it's unnecessary, Weave does it already:
   

   weave/weave/executor.nim

   Lines 133 to 157 in 97b8f55

   	proc runUntil*(_: typedesc[Weave], signal: ptr Atomic[bool]) =
   	## Start a Weave event loop until signal is true on the current thread.
   	## It wakes-up on job submission, handles multithreaded load balancing,
   	## help process tasks
   	## and spin down when there is no work anymore.
   	preCondition: not signal.isNil
   	while not signal[].load(moRelaxed):
   	processAllandTryPark(Weave)
   	syncRoot(Weave)
   	proc runInBackground*(
   	thr: var Thread[ptr Atomic[bool]],
   	_: typedesc[Weave],
   	signalShutdown: ptr Atomic[bool]
   	) =
   	## Start the Weave runtime on a background thread.
   	## It wakes-up on job submissions, handles multithreaded load balancing,
   	## help process tasks
   	## and spin down when there is no work anymore.
   	proc eventLoop(shutdown: ptr Atomic[bool]) {.thread.} =
   	init(Weave)
   	Weave.runUntil(shutdown)
   	exit(Weave)
   	thr.createThread(eventLoop, signalShutdown)

Besides the fix, I think I'll need to design some specific multithreading scheme for IO needs like HTTP servers. Does Guildenstern reaches the same perf as HttpBeast when they are both single-threaded?

[olliNiinivaara]

Thank you!, I've implemented the fix.
The remaining problem is that it without -d:danger it fails with (multiple):

fail: /home/olli/.nimble/pkgs/weave-#master/weave/instrumentation/contracts.nim(86, 15)

localThreadKind == WorkerThread`
    Contract violated for pre-condition at runtime.nim:142
        
localThreadKind == WorkerThread
    The following values are contrary to expectations:
        
localThreadKind == WorkerThread  [Worker N/A]

It is too much to require -d:danger switch to be always on; the code must work in other modes, too.

---

On designing an I/O specific mutlithreading scheme, see my comment at:
#132 (comment)

"HTTP server" is too broadly defined I/O problem, especially because all Nim servers (AsyncHTTPServer, Guildenstern, HTTPBeast...) are written with the expectation that they are run behind a true HTTP (reverse proxy) server such as Nginx or Caddyserver which takes care of the real I/O problems of open Internet containing malicious adversarial I/O devices.

---

However, here are some benchmark results. As I see it, minimizing worst-case latency is more important than throughput ;-)

Guildenstern with Weave, 4 threads:
Max Latency: 1.37ms
Requests/sec: 78291.63

Guildenstern, single-threaded:
Max Latency: 4.05ms
Requests/sec: 94011.72

httpbeast, single-threaded:
Max Latency: 9.88ms
Requests/sec: 86211.69

httpbeast, 4 threads:
Max Latency: 11.88ms
Requests/sec: 113153.31

[mratsim]

The assert shouldn't be triggered unless there is are spawn or parallel loop left, I'll checkout your fixes.

Mmmh it's good to know that the base single-threaded server is fast enough.

I'm reopening this #22 for IO research.

I fear that while playing well with async is relatively easy (and done), having multithreading optimized for IO requires a completely different design, one of those would be more akin to Tokio in Rust. I.e. I need to have distributing waiting at the selector level (not familiar with low-level async IO yet so may be inaccurate).