mratsim / weave

A state-of-the-art multithreading runtime: message-passing based, fast, scalable, ultra-low overhead
Other
537 stars 22 forks source link

Need help with weave as background process #155

Closed olliNiinivaara closed 4 years ago

olliNiinivaara commented 4 years ago

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 commented 4 years ago

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

mratsim commented 4 years ago

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: https://github.com/mratsim/weave/blob/97b8f5541bb399312d86e2e915caa6c82e970941/weave/executor.nim#L133-L157

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 commented 4 years ago

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: https://github.com/mratsim/weave/issues/132#issuecomment-651647277

"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 commented 4 years ago

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 https://github.com/mratsim/weave/issues/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).