Wayland fd watching

[Pekka Paalanen]

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.

> > 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20220113/2d84070b/attachment.sig>