Wayland fd watching

[Maksim Sisov]

On 2022-01-13 15:46, Pekka Paalanen wrote:
> On Thu, 13 Jan 2022 14:05:47 +0200
> Maksim Sisov <msisov at igalia.com> wrote:
> 
>> On 2022-01-13 11:42, Pekka Paalanen wrote:
>> > On Thu, 13 Jan 2022 10:05:56 +0200
>> > Maksim Sisov <msisov at igalia.com> wrote:
>> >
> 
> ...
> 
>> >> I see. The only only problem here is that we don't use poll, but rather
>> >> libevent, which is wrapped around Chromium's MessagePumpLibevent. It
>> >> constantly notifies us that a fd is ready to be read.
>> >
>> > You say "constantly", do you mean it never waits?
>> >
>> >> Here is a diagram that shows the flow without a dedicated thread -
>> >> https://drive.google.com/file/d/1Jq8aX4cWszwL5fleE8ayGOeond70xChS/view?usp=sharing
>> >> .
>> >>
>> >> As you can see, it returns from OnCanRead and waits until libevent
>> >> notifies us the next time that we can actually read events (if prepared
>> >> to read previously). And if the main thread blocks because of the above
>> >> mentioned reason (libevent didn't have a chance to notify us), the
>> >> events will never be read.
>> >
>> > Are you describing the old approach or the new approach with the
>> > separate polling thread?
>>
>> I'm describing the old one as having a dedicated thread doesn't really
>> bring much benefits as I said in my first message.
>>
>> >
>> > Doing prepare_read from OnCanRead handler seems a bit backwards. The
>> > prepare_read dance is to make sure that when you are going to wait for
>> > new events (block), you have processed all incoming events so far and
>> > flushed out all requests that might have resulted from those, so that
>> > if you are waiting for replies to those requests, they will actually
>> > arrive. So the prepare_read/dispatch/flush dance should be done from a
>> > "the event loop is going to sleep now" hook, not from "the fd has bytes
>> > to read" hook.
>>
>> I checked libevent - they added prepare/check watchers in
>> https://github.com/libevent/libevent/pull/793 in Spring, 2019. Chromium
>> still uses libevent 1.4.15
>> (https://source.chromium.org/chromium/chromium/src/+/main:base/third_party/libevent/README.chromium),
>> which looks to be old. It seems like we need to make an update that
>> hasn't been done for ages. Moreover, it seems to be patched :\ I'll need
>> to check if we can upgrade.
> 
> For your reference, SardemFF7 (in OFTC IRC) just pointed me to
> 
> https://github.com/sardemff7/libgwater/blob/master/wayland/libgwater-wayland.c
> 
> as an example of hooking libwayland-client into glib event loop.
> 
> An important detail is that "check" (read) and "dispatch" are separate
> steps as well. Those should help to avoid the below I think.

Thanks. gtk/gdk also implements event loop the same way using glib.
However, Chromium can either use libevent or libglib based on the
configuration and I'm afraid we have to use support both. Even though,
glib is default on Desktop Linux, we need to make it work correctly with
libevent as well. I'm not aware about the hook that libevent does before
going into sleep except the watcher callbacks I mentioned before that
are not part of stable versions yet.

> 
>> > Hmm, yeah... I suppose when your event loop decides to do something
>> > else than actually read the Wayland socket after it was prepared for
>> > reading, and that something else is a blocking thing, you would have to
>> > cancel the read first just in case, and then prepare_read again to be
>> > able to read afterwards.
> 
> If check only reads, and dispatch is the step that might e.g. tear down
> other threads synchronously, you shouldn't need to cancel_read in order
> to do synchronous things between threads.
> 
> Also the time it takes from Wayland socket becoming readable to
> actually calling read_events should be minimized, because other threads
> might have done prepare_read, have been woken up, and are now waiting
> for the last reader to enter the read_events. Not doing long operations
> in the check/read step should help with that too.
> 
> 
> Thanks,
> pq

-- 
Best Regards,
Maksim Sisov
* Usual work time - 08:00-16:00 (EEST).