Message ID | tencent_FF44F3E5B36C75DAC90DEEA7255073030805@qq.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,1/2] avutil/executor: Allowing thread_count be zero | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
On Mon, Jun 24, 2024 at 12:48 PM Zhao Zhili <quinkblack@foxmail.com> wrote: > From: Zhao Zhili <zhilizhao@tencent.com> > > Before the patch, disable threads support at configure/build time > was the only method to force zero thread in executor. However, > it's common practice for libavcodec to run on caller's thread when > user specify thread number to one. And for WASM environment, whether > threads are supported needs to be detected at runtime. So executor > should support zero thread at runtime. > > A single thread executor can be useful, e.g., to handle network > protocol. So we can't take thread_count one as zero thread, which > disabled a valid usercase. > > Other libraries take -threads 0 to mean auto. Executor as a low > level utils doesn't do cpu detect. So take thread_count zero as > zero thread, literally. > > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> > Hi Zhili, Thank you for the v2 I will merge it in 3 days if there are no objections.
On Tue, Jun 25, 2024 at 8:36 PM Nuo Mi <nuomi2021@gmail.com> wrote: > > > On Mon, Jun 24, 2024 at 12:48 PM Zhao Zhili <quinkblack@foxmail.com> > wrote: > >> From: Zhao Zhili <zhilizhao@tencent.com> >> >> Before the patch, disable threads support at configure/build time >> was the only method to force zero thread in executor. However, >> it's common practice for libavcodec to run on caller's thread when >> user specify thread number to one. And for WASM environment, whether >> threads are supported needs to be detected at runtime. So executor >> should support zero thread at runtime. >> >> A single thread executor can be useful, e.g., to handle network >> protocol. So we can't take thread_count one as zero thread, which >> disabled a valid usercase. >> >> Other libraries take -threads 0 to mean auto. Executor as a low >> level utils doesn't do cpu detect. So take thread_count zero as >> zero thread, literally. >> >> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> >> > Hi Zhili, > Thank you for the v2 > I will merge it in 3 days if there are no objections. > Applied > >
diff --git a/libavutil/executor.c b/libavutil/executor.c index 26691fe157..fb20104b58 100644 --- a/libavutil/executor.c +++ b/libavutil/executor.c @@ -82,9 +82,11 @@ static int run_one_task(AVExecutor *e, void *lc) /* nothing */; if (*prev) { AVTask *t = remove_task(prev, *prev); - ff_mutex_unlock(&e->lock); + if (e->thread_count > 0) + ff_mutex_unlock(&e->lock); cb->run(t, lc, cb->user_data); - ff_mutex_lock(&e->lock); + if (e->thread_count > 0) + ff_mutex_lock(&e->lock); return 1; } return 0; @@ -146,14 +148,17 @@ AVExecutor* av_executor_alloc(const AVTaskCallbacks *cb, int thread_count) return NULL; e->cb = *cb; - e->local_contexts = av_calloc(thread_count, e->cb.local_context_size); + e->local_contexts = av_calloc(FFMAX(thread_count, 1), e->cb.local_context_size); if (!e->local_contexts) goto free_executor; - e->threads = av_calloc(thread_count, sizeof(*e->threads)); + e->threads = av_calloc(FFMAX(thread_count, 1), sizeof(*e->threads)); if (!e->threads) goto free_executor; + if (!thread_count) + return e; + has_lock = !ff_mutex_init(&e->lock, NULL); has_cond = !ff_cond_init(&e->cond, NULL); @@ -175,9 +180,12 @@ free_executor: void av_executor_free(AVExecutor **executor) { + int thread_count; + if (!executor || !*executor) return; - executor_free(*executor, 1, 1); + thread_count = (*executor)->thread_count; + executor_free(*executor, thread_count, thread_count); *executor = NULL; } @@ -195,9 +203,9 @@ void av_executor_execute(AVExecutor *e, AVTask *t) ff_cond_signal(&e->cond); ff_mutex_unlock(&e->lock); -#if !HAVE_THREADS - // We are running in a single-threaded environment, so we must handle all tasks ourselves - while (run_one_task(e, e->local_contexts)) - /* nothing */; -#endif + if (!e->thread_count || !HAVE_THREADS) { + // We are running in a single-threaded environment, so we must handle all tasks ourselves + while (run_one_task(e, e->local_contexts)) + /* nothing */; + } } diff --git a/libavutil/executor.h b/libavutil/executor.h index c602bcb613..0eb21c10c8 100644 --- a/libavutil/executor.h +++ b/libavutil/executor.h @@ -46,7 +46,7 @@ typedef struct AVTaskCallbacks { /** * Alloc executor * @param callbacks callback structure for executor - * @param thread_count worker thread number + * @param thread_count worker thread number, 0 for run on caller's thread directly * @return return the executor */ AVExecutor* av_executor_alloc(const AVTaskCallbacks *callbacks, int thread_count);