[vlc-devel] [RFC 1/2] executor: introduce new executor API

[Steve Lhomme]

On 2020-08-27 10:48, Romain Vimont wrote:
> On Thu, Aug 27, 2020 at 07:59:42AM +0200, Steve Lhomme wrote:
>> On 2020-08-26 19:19, Romain Vimont wrote:
>>> Introduce a new API to execute "runnables". The execution can be
>>> automatically interrupted on timeout or via an explicit cancelation
>>> request.
>>> ---
>>>    include/vlc_executor.h | 194 +++++++++++++++
>>>    src/Makefile.am        |   1 +
>>>    src/libvlccore.sym     |   4 +
>>>    src/misc/executor.c    | 545 +++++++++++++++++++++++++++++++++++++++++
>>>    4 files changed, 744 insertions(+)
>>>    create mode 100644 include/vlc_executor.h
>>>    create mode 100644 src/misc/executor.c
>>>
>>> diff --git a/include/vlc_executor.h b/include/vlc_executor.h
>>> new file mode 100644
>>> index 0000000000..4dd8b93628
>>> --- /dev/null
>>> +++ b/include/vlc_executor.h
>>> @@ -0,0 +1,194 @@
>>> +/*****************************************************************************
>>> + * vlc_executor.h
>>> + *****************************************************************************
>>> + * Copyright (C) 2020 Videolabs, VLC authors and VideoLAN
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU Lesser General Public License as published by
>>> + * the Free Software Foundation; either version 2.1 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public License
>>> + * along with this program; if not, write to the Free Software Foundation,
>>> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
>>> + *****************************************************************************/
>>> +
>>> +#ifndef VLC_EXECUTOR_H
>>> +#define VLC_EXECUTOR_H
>>> +
>>> +#include <vlc_common.h>
>>> +#include <vlc_tick.h>
>>> +
>>> +# ifdef __cplusplus
>>> +extern "C" {
>>> +# endif
>>> +
>>> +/** Executor type (opaque) */
>>> +typedef struct vlc_executor vlc_executor_t;
>>> +
>>> +/**
>>> + * A Runnable is intended to be run from another thread by an executor.
>>> + */
>>> +struct vlc_runnable {
>>> +
>>> +    /**
>>> +     * This function is to be executed by a vlc_executor_t.
>>> +     *
>>> +     * It must implement the actions (arbitrarily long) to execute from an
>>> +     * executor thread, synchronously. As soon as run() returns, the execution
>>> +     * of this runnable is complete.
>>> +     *
>>> +     * After the runnable is submitted to an executor via
>>> +     * vlc_executor_Submit(), the run() function is executed at most once (zero
>>> +     * if the execution is canceled before it was started).
>>> +     *
>>> +     * It may not be NULL.
>>> +     *
>>> +     * \param userdata the userdata provided to vlc_executor_Submit()
>>> +     */
>>> +    void
>>> +    (*run)(void *userdata);
>>> +
>>> +    /**
>>> +     * This function attempts to interrupt the execution of run().
>>> +     *
>>> +     * If not NULL, it may be called on vlc_executor_Cancel() or on timeout.
>>> +     *
>>> +     * It is called from a thread different from the one executing run(). It is
>>> +     * free to do any actions to "interrupt" the execution of run() (set a
>>> +     * flag, close a file descriptor, etc.).
>>> +     *
>>> +     * The runnable will be considered "finished" once run() actually
>>> +     * terminates.
>>> +     *
>>> +     * It should be quick, not to block the interruption of other runnables.
>>> +     *
>>> +     * \param userdata the userdata provided to vlc_executor_Submit()
>>> +     */
>>> +    void
>>> +    (*interrupt)(void *userdata);
>>> +
>>> +    /**
>>> +     * This function notifies the end of the execution.
>>> +     *
>>> +     * If not NULL, it is called either:
>>> +     *  - when run() terminates;
>>> +     *  - when the task is canceled before run() is called.
>>> +     *
>>> +     * In other words, it is always called in the end if vlc_executor_Submit()
>>> +     * returns VLC_SUCCESS.
>>
>> It should also be called when the runnable is canceled.
> 
> It is called everytime (if the runnable is canceled while run() is
> running, then run() terminates, so it's case 1).
> 
>> You should add a return value to tell if it was sucessful or not.
> 
> I initially added a bool parameter to report the "cancel" status, but
> after using it in the preparser, I realized it was useless and
> confusing.
> 
> Useless: the user provides both the interrupt() and runnable()
> callbacks, so they can know the exact status.

In the few executor APIs I've used in the past, it was always giving the 
result of the task. Maybe you don't need it now but later we're going to 
need it. IMO it's part of the executor design pattern.

When you run 1000's of task you don't keep track of which ones you run 
and if they might have succeeded or not. Each task takes care of itself 
and is standalone.

> Confusing: on timeout (or if the user calls vlc_executor_Cancel()), the
> run() function could still finish successfully on cancelation (for
> example if the last part was not "interruptible"). Only the user knows
> if the action is successful or not, and they typically already store a
> state.

You can have 3 states: success, failure, timeout. A timeout could allow 
the task to be restarted (a few times).