[vlc-devel] [RFC 0/2] New executor API

[Alexandre Janniaux]

Hi,

On Fri, Aug 28, 2020 at 11:16:21AM +0200, Romain Vimont wrote:
> On Fri, Aug 28, 2020 at 10:25:16AM +0200, Alexandre Janniaux wrote:
> > Hi,
> >
> > On Thu, Aug 27, 2020 at 10:28:58PM +0300, Rémi Denis-Courmont wrote:
> > > Le torstaina 27. elokuuta 2020, 22.13.37 EEST Romain Vimont a écrit :
> > > > > So the runnable will be using poll(), vlc_cond_t, vlc_sem_t or some other
> > > > > wait method that supports timing-out. It might as well use that time-out
> > > > > capability instead of relying on a thread for the sole purpose of maybe
> > > > > (ab)using the interrupt callback on time-out.
> > > >
> > > > Yes, if you only want timeout, but not "unexpected" cancelation, that
> > > > works (and is better).
> > > >
> > > > > Besides, timing out is an error that deserves logging. Interrupt is not.
> > > > > So using the same callback for both purpose looks dubious to me.
> > > >
> > > > The goal is simplicity.
> > >
> > > By making things more complicated. Yeah right...
> >
> > I agree with Remi, if you want simplicity, the best thing
> > would be to limit the asynchronous framework to the
> > asynchronous execution and signalling like almost every other
> > asynchronous framework.
>
> For example, the Java concurrent framework provides a method cancel() on
> futures:
> https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/Future.html#cancel(boolean)
>
> Of course, this is a higher level language, and thread interruption is
> something managed at the Thread level, so there is no custom interrupt
> callback to provide.
>
> > Especially since youǘe talked about some `future` object, and
> > thus a pro-actor model which doesn't have ways to do
> > signalling from the outside of the asynchronous framework to
> > the task by itself in almost all implementation, and instead
> > have each layer of promise being able to return either a result
> > or an error that the future will catch, and an internal
> > composition rule for each layer's result.
> >
> > In the case the vlc_executor is killed, the preparser client
> > needs to be killed first since it is using the executor, and
> > thus can cancel its tasks internally directly, which might be
> > at the middle of the preprocessing, at the beginning or right
> > before the end. It's much less convoluted than having the
> > preparser client use the vlc_executor to cancel the taks, which
> > will just really call the preparser specific cancellation code
> > anyway.
>
> (The executor would still need a Cancel() function to remove the tasks
> not started yet.)
>
> This would greatly simplify the executor itself.
>
> But then, to get the same features, each user (like the preparser) will
> have to:
>  - still implement the interrupt mechanism for cancelation (it will just
>    not pass the callback to the executor)

We can remove this point since it's need for both.

>  - implement the timeout manually in addition (which came "for free" if
>    interruption was implemented)

It can use the already implemented vlc_timer for this case
and in the best case we're starting one thread less for timer
queries within VLC.

>  - manage the list of its tasks submitted to the executor

IMHO, it's a lie to put it only here, you still need to
manage the list of tasks if you want to get a result somehow,
and manage the lifecycle of the resources shared between the
task and the task creator.

>  - interrupt all remaining tasks before deleting the executor

You need this in both case anyway, and like I wrote, before
the vlc_executor is destroyed anyway since the task creator
has access to it.

> Instead of just providing:
>  - the "interrupt" callback (that must be written for cancelation
>    anyway);
>  - the timeout parameter to vlc_executor_Submit().
>
> In fact, there are 3 levels of features for the executor:
>  1. submit tasks (and cancel tasks not started yet)
>  2. cancel tasks with interruption (to be called on explicit cancel or
>     executor deletion)
>  3. timeout (calling the interrupt() callback)
>
> Possibilities are basically either 1, 1+2 or 1+2+3.
>
> IIUC, what you (Rémi & Alex) suggest is 1.
>
> I don't really like the principle of an additional thread to trigger
> "interrupt" calls either, but I see that as a trade-off between
> simplicity for the executor impl VS simplicity for the users of the
> executor.
>
> I initially implemented 1+2+3. The current version is:
> https://code.videolan.org/rom1v/vlc/commits/executor.16
>
> I just added two commits to only get 1+2 (I removed the timeout):
> https://code.videolan.org/rom1v/vlc/commits/executor.17
>
> As you can see, it greatly simplifies the executor:
> https://code.videolan.org/rom1v/vlc/-/commit/e25283cf4dbb03a0c145a4f4aea6da1ae9382126
>
> But the users have to handle timeout manually (in addition to the
> interrupt mechanism):
> https://code.videolan.org/rom1v/vlc/-/commit/d838204802842e44fff23d7a6b46b69c338c617e
>
> Regards
>
> >
> > Regards,
> > --
> > Alexandre Janniaux
> > Videolabs
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel