Display server main control loop

[Thomas Voß]

On 04.04.2013 10:41, Alexandros Frantzis wrote:
> On Thu, Apr 04, 2013 at 08:32:05AM +0200, Thomas Voß wrote:
>> On 03.04.2013 17:51, Alexandros Frantzis wrote:
>>>
>>> To get a better feeling of what's involved, I created a prototype
>>> abstraction for a main loop and an implementation using boost asio.
>>> Get it and use it with:
>>>
>>> $ bzr branch lp:~afrantzis/+junk/mainloop
>>> $ cd mainloop && make
>>> $ ./mainloop
>>>
>>> The main loop abstraction was designed so that event creation and
>>> handling are completely dissociated. The display server (or other
>>> consumer) can handle an event without caring if it is coming from a
>>> signal, a read from a fd, or another mechanism.  So, we could have
>>> something like this in the mir main loop:
>>>
>>
>> This looks pretty good to me, some thoughts on the separation of
>> concerns, though:
>>> auto main_loop = main_loop_factory->create_main_loop();
>>>
>>> auto pause_event = platform->create_pause_event(main_loop_factory);
>>
>> I would rather prefer:
>>
>> PauseEvent pause_event(main_loop);
>> pause_event.async_wait(pause_handler);
>>
>> QuitEvent quit_event(main_loop);
>> quit_event.async_wait(quit_handler);
>>
>> DisplayConfigurationEvent disp_config_event(main_loop);
>> disp_config_event.async_wait(disp_conf_handler);
> 
> The problem is that the way many events are triggered is platform
> dependent. For example, a display configuration event may be triggered
> by data being available on an FD on the desktop (from udev), and
> something completely different on Android.  That's the reason I opted
> for asking components to give us event object, since they have all the
> information to set this up.
> 

Okay, the idea here is: "Client" code expresses an interest in a certain
event, say "DisplayReconfigurationEvent" and it's not a concern of the
code expressing interest in the event to know where this event might
originate from. That is, I would expect the respective Event classes
itself to know how to set themselves up and talk to the
platform-specific components. With that, we would achieve a strong
separation of concerns, while keeping platform-specific tasks local to
the Event classes.

> In the scheme you propose, would different triggers be handled by having
> a different version of EventService for each platform?
> 
>> See boost::asio::signal_set as an example: Different parties interested
>> in a certain set of signals just setup their respective signal set and
>> execute an (async) wait on the respective set. Behind the scenes, the
>> signal_set class sets up all of the handlers, and associates all of the
>> operation with one io_service, which forces handler execution to the
>> thread that the io_service.run method is executed on.
>>
>> To this end, a mir::EventService would need to be created, and the
>> respective events, including custom handler types (not only void()), are
>> known to the EventService. On startup, the service is registered with
>> the io_service representing the main event loop with
>> boost::asio::add_service<mir::EventService>(io_service).
>>
>> A class QuitEvent would then do:
>>
>> class QuitEvent
>> {
>> public:
>> 	QuitEvent(const std::shared_ptr<mir::MainLoop>& main_loop) :
>> io_service(main_loop->io_service()) {}
>>
>> 	void async_wait(QuitEventHandler handler)
>> 	{
>> 	
>> boost::asio::use_service<mir::EventService>(io_service).enqueue_handler_for_quit_event(handler);
>> 	}
>> private:
>> 	boost::asio::io_service& io_service;
>> };
>>
>> From my perspective, keeping the handler definition local, without
>> encoding everything within the main loop avoids tight coupling, and the
>> main loop is stripped down to only the synchronization concern.
> 
> What you are proposing is more versatile, but I see tighter coupling as
> something desirable in this case. The main control loop is not supposed
> to be a generic event loop, it's only about events that need to change
> the state of the main components of the display server in a coordinated
> manner. As such, the handlers need to defined at a level where all the
> information is available, and that is at the control loop level. I don't
> want to allow event handler registration away from the main loop, since
> this is not useful for our use cases, and will make the code more
> difficult to reason about.
> 

>From my perspective, there is a difference between the concern of
synchronization and the concern of communicating specific events to
components interested in them. In this respect, I do agree with your
intention to avoid a generic event loop. What I'm proposing is
basically: We have a generic reactor (boost::asio::io_service) that is
concerned with synchronization and we can add specific services to
specific instances of the reactor, for example a
mir::display::ConfigurationEventService.

> The model I am proposing is that components offer interesting events
> which are independent of any particular main loop, and at the main
> control loop level we decide if/how to handle them. So, e.g., the
> platforms tells us: "I am producing display configuration events, use
> this event object if you want to handle them".
> 
> I would like to support events that provide more information (i.e. have
> a custom handler) and I am currently thinking about how to go about
> this, while still elegantly allowing different triggers depending on the
> platform.

I think my proposal addresses your requirement quite elegantly in that
the Event carries the platform-independent information but knows
internally how to wire itself up to the platform-specific pieces.
Components interested in the event then are able to say: I'm interested
in this event and would like to wait for it, not knowing anything about
something platform-specific.

> 
> Thanks,
> Alexandros
>