atlas-engineer / nyxt

Nyxt - the hacker's browser.
https://nyxt-browser.com/
9.91k stars 416 forks source link

Overhaul concurrency #2381

Open Ambrevar opened 2 years ago

Ambrevar commented 2 years ago

In https://github.com/atlas-engineer/nyxt/pull/2348#issuecomment-1149960674 I pointed out that our concurrency usage is deeply flawed: when a condition arises in a thread started by run-thread, if the caller waits on a result channel it will hang forever.

Poor, poor design indeed... :'(

I started working on a solution: basically pass an error-channel, write to it on error and have the caller run calispel:fair-alt on the result channel. Not too hard, but then we need to deal with the GTK thread and designing a consistent API is tricky here.

And then, I started looking at Lparallel again (https://lparallel.org/). The initial reason why I didn't want to go with it is because it didn't have any fair-alt equivalent (also know as SELECT statement in CSP), which, in my understanding, is critical for CSP. In the vast majority of cases, SELECT is used to interrupt threads or handle errors.

But! This was due to my poor understanding, back then, of how CL conditions work. Actually, the crazy thing is that we don't need SELECT for interruptions: we can just use the condition system instead! And indeed, this is exactly how Lparallel is intended to work: https://lparallel.org/handling/

For instance, when we interrupt the prompt buffer we can raise a condition from the prompt buffer thread. On the caller side, we used to have this:

(defun wait-on-prompt-buffer (prompt-buffer)
  "Block and return PROMPT-BUFFER results."
  (when (prompt-buffer-p prompt-buffer)
    (show-prompt-buffer prompt-buffer)
    (calispel:fair-alt
      ((calispel:? (prompter:result-channel prompt-buffer) results)
       (hide-prompt-buffer prompt-buffer)
       results)
      ((calispel:? (prompter:interrupt-channel prompt-buffer))
       (hide-prompt-buffer prompt-buffer)
       (error 'nyxt-prompt-buffer-canceled)))))

Instead, with Lparallel it would look like this (untested):

(defun wait-on-prompt-buffer (prompt-buffer)
  "Block and return PROMPT-BUFFER results."
  (when (prompt-buffer-p prompt-buffer)
    (lpara:task-handler-bind ((condition (lambda (c)
                                           (hide-prompt-buffer prompt-buffer)
                                           (error 'nyxt-prompt-buffer-canceled)))))
    (lpara:submit-task (prompter:result-channel prompt-buffer)
                       #'show-prompt-buffer prompt-buffer)
    (prog1 (lparallel:receive-result (prompter:result-channel prompt-buffer))
      (hide-prompt-buffer prompt-buffer))))

It's even shorter than the Calispel code and saves us from declaring 1 channel (the interrupt channel).

Cool, nah?

Other benefits of Lparallel:

Benchmark

Lparallel vs. Calispel:

(time (let ((ch (lparallel:make-channel))
            (limit 100000))
        (dotimes (i limit) (lparallel:submit-task ch #'fibo 5))
        (dotimes (i limit) (lparallel:receive-result ch))))
; Evaluation took:
;  0.048 seconds of real time

(time (let ((ch (make-instance 'calispel:channel
                               :buffer (make-instance 'jpl-queues:unbounded-fifo-queue)))
            (limit 100000))
        (dotimes (i limit) (bt:make-thread (lambda () (calispel:! ch (fibo 5)))))
        (dotimes (i limit) (calispel:? ch))))
; Evaluation took:
;  1.716 seconds of real time

Of course it's easy to extend the Calispel example to only create 2 threads that receive "tasks", thus re-implementing what Lparallel does. But that's precisely my point: Lparallel uses an efficient approach, so why reinvent the wheel?

Future work

I need to see how Lparallel deals with the GTK loop. This part is always tricky.

EDIT: Fix typos.

aadcg commented 2 years ago

This sounds exciting!

aadcg commented 2 years ago

I'll be studying lparallel these days since I'll need it for recursive-search.

Ambrevar commented 2 years ago

Another concern I had with Lparallel is that the documentation does not show how to ask a specific thread to write to a channel. It's a lower-level use of channels, but something that could be useful when one does not want to rely on the kernel (say, because it's busy doing other things and our thread is critical and has highest priority).

Turns out it's possible with some less documented part of Lparallel:

lparallel.queue:make-queue
lparallel.queue:push-queue
lparallel.queue:pop-queue

should do the job.

So compared to Calispel, the only thing that seems to be missing is the SELECT statement (calispel:fair-alt).