Re: [FFmpeg-devel] [PATCH 15/23] dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Andreas Rheinhardt
> Sent: 2021年3月12日 15:59
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 15/23]
> dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
>
> Guo, Yejun:
> >
> >
> >> -Original Message-
> >>>> -Original Message-
> >>>> From: ffmpeg-devel On Behalf Of
> >>>> Andreas Rheinhardt
> >>>> Sent: 2021年3月11日 5:55
> >>>> To: ffmpeg-devel@ffmpeg.org
> >>>> Cc: Andreas Rheinhardt
> >>>> Subject: [FFmpeg-devel] [PATCH 15/23]
> >>>> dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
> >>>>
> >>>> Fixes Coverity issue #1473507.
> >>>>
> >>>> Signed-off-by: Andreas Rheinhardt
> >>>> ---
> >>>> libavfilter/dnn/dnn_backend_native_layer_conv2d.c | 10 +-
> >>>> 1 file changed, 5 insertions(+), 5 deletions(-)
> >>>> }
> >>>>
> >>>> //release memory
> >>>> -av_freep(_id);
> >>>> -
> >>>> for (int i = 0; i < thread_num; i++){
> >>>> av_freep(_param[i]);
> >>>> }
> >>>
> >>> LGMT, and just one question: shall we reduce the allocation as less as
> >> possible? thanks.
> >>>
> >> If it is doable without too much effort as is here, then the answer is
> >> yes. Actually, one could go even further than what this patchset did:
> >> One could keep the thread_param array instead of constantly allocating
> >> and freeing it. With much more effort, one could even keep the threads.
> >
> > thanks, and is the reason that: we need to check the return value for
> dynamic
> > allocation, and also need some effort to make sure to free it for all
> > possible
> > paths in the code followed?
> >
> Yes, reducing the amount of allocations also reduces the amount of
> checks and frees one has to perform. You can see the latter in this
> patchset, namely in this patch and in the one that stopped allocating
> thread_param separately (which enabled to remove the loop for freeing them).
got it, thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/23] dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
Guo, Yejun:
>
>
>> -Original Message-
>>>> -Original Message-
>>>> From: ffmpeg-devel On Behalf Of
>>>> Andreas Rheinhardt
>>>> Sent: 2021年3月11日 5:55
>>>> To: ffmpeg-devel@ffmpeg.org
>>>> Cc: Andreas Rheinhardt
>>>> Subject: [FFmpeg-devel] [PATCH 15/23]
>>>> dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
>>>>
>>>> Fixes Coverity issue #1473507.
>>>>
>>>> Signed-off-by: Andreas Rheinhardt
>>>> ---
>>>> libavfilter/dnn/dnn_backend_native_layer_conv2d.c | 10 +-
>>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>>> }
>>>>
>>>> //release memory
>>>> -av_freep(_id);
>>>> -
>>>> for (int i = 0; i < thread_num; i++){
>>>> av_freep(_param[i]);
>>>> }
>>>
>>> LGMT, and just one question: shall we reduce the allocation as less as
>> possible? thanks.
>>>
>> If it is doable without too much effort as is here, then the answer is
>> yes. Actually, one could go even further than what this patchset did:
>> One could keep the thread_param array instead of constantly allocating
>> and freeing it. With much more effort, one could even keep the threads.
>
> thanks, and is the reason that: we need to check the return value for dynamic
> allocation, and also need some effort to make sure to free it for all possible
> paths in the code followed?
>
Yes, reducing the amount of allocations also reduces the amount of
checks and frees one has to perform. You can see the latter in this
patchset, namely in this patch and in the one that stopped allocating
thread_param separately (which enabled to remove the loop for freeing them).
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/23] dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
> -Original Message-
> >> -Original Message-
> >> From: ffmpeg-devel On Behalf Of
> >> Andreas Rheinhardt
> >> Sent: 2021年3月11日 5:55
> >> To: ffmpeg-devel@ffmpeg.org
> >> Cc: Andreas Rheinhardt
> >> Subject: [FFmpeg-devel] [PATCH 15/23]
> >> dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
> >>
> >> Fixes Coverity issue #1473507.
> >>
> >> Signed-off-by: Andreas Rheinhardt
> >> ---
> >> libavfilter/dnn/dnn_backend_native_layer_conv2d.c | 10 +-
> >> 1 file changed, 5 insertions(+), 5 deletions(-)
> >> }
> >>
> >> //release memory
> >> -av_freep(_id);
> >> -
> >> for (int i = 0; i < thread_num; i++){
> >> av_freep(_param[i]);
> >> }
> >
> > LGMT, and just one question: shall we reduce the allocation as less as
> possible? thanks.
> >
> If it is doable without too much effort as is here, then the answer is
> yes. Actually, one could go even further than what this patchset did:
> One could keep the thread_param array instead of constantly allocating
> and freeing it. With much more effort, one could even keep the threads.
thanks, and is the reason that: we need to check the return value for dynamic
allocation, and also need some effort to make sure to free it for all possible
paths in the code followed?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/23] dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
Guo, Yejun:
>
>
>> -Original Message-
>> From: ffmpeg-devel On Behalf Of
>> Andreas Rheinhardt
>> Sent: 2021年3月11日 5:55
>> To: ffmpeg-devel@ffmpeg.org
>> Cc: Andreas Rheinhardt
>> Subject: [FFmpeg-devel] [PATCH 15/23]
>> dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
>>
>> Fixes Coverity issue #1473507.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavfilter/dnn/dnn_backend_native_layer_conv2d.c | 10 +-
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
>> b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
>> index 2b83896da9..2e5aacdc2b 100644
>> --- a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
>> +++ b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
>> @@ -38,6 +38,9 @@ typedef struct ThreadCommonParam{ typedef struct
>> ThreadParam{
>> ThreadCommonParam *thread_common_param;
>> int thread_start, thread_end;
>> +#if HAVE_PTHREAD_CANCEL
>> +pthread_t thread;
>> +#endif
>> } ThreadParam;
>>
>> int ff_dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context,
>> int file_size, int operands_num) @@ -187,7 +190,6 @@ int
>> ff_dnn_execute_layer_conv2d(DnnOperand *operands, const int32_t
>> *input_opera
>> int thread_num = (ctx->options.conv2d_threads <= 0 ||
>> ctx->options.conv2d_threads > av_cpu_count())
>> ? (av_cpu_count() + 1) : (ctx->options.conv2d_threads); #if
>> HAVE_PTHREAD_CANCEL
>> -pthread_t *thread_id = av_malloc_array(thread_num, sizeof(*thread_id));
>> int thread_stride;
>> #endif
>> ThreadParam **thread_param = av_malloc_array(thread_num,
>> sizeof(*thread_param)); @@ -230,17 +232,15 @@ int
>> ff_dnn_execute_layer_conv2d(DnnOperand *operands, const int32_t
>> *input_opera
>> thread_param[i]->thread_common_param =
>> _common_param;
>> thread_param[i]->thread_start = thread_stride * i + pad_size;
>> thread_param[i]->thread_end = (i == thread_num - 1) ? (height -
>> pad_size) : (thread_param[i]->thread_start + thread_stride);
>> -pthread_create(_id[i], NULL,
>> dnn_execute_layer_conv2d_thread, (void *)thread_param[i]);
>> +pthread_create(_param[i]->thread, NULL,
>> + dnn_execute_layer_conv2d_thread, (void *)thread_param[i]);
>> }
>>
>> //join threads, res gets function return
>> for (int i = 0; i < thread_num; i++){
>> -pthread_join(thread_id[i], NULL);
>> +pthread_join(thread_param[i]->thread, NULL);
>> }
>>
>> //release memory
>> -av_freep(_id);
>> -
>> for (int i = 0; i < thread_num; i++){
>> av_freep(_param[i]);
>> }
>
> LGMT, and just one question: shall we reduce the allocation as less as
> possible? thanks.
>
If it is doable without too much effort as is here, then the answer is
yes. Actually, one could go even further than what this patchset did:
One could keep the thread_param array instead of constantly allocating
and freeing it. With much more effort, one could even keep the threads.
Anyway, applied this patchset with the changes you requested.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/23] dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Andreas Rheinhardt
> Sent: 2021年3月11日 5:55
> To: ffmpeg-devel@ffmpeg.org
> Cc: Andreas Rheinhardt
> Subject: [FFmpeg-devel] [PATCH 15/23]
> dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
>
> Fixes Coverity issue #1473507.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavfilter/dnn/dnn_backend_native_layer_conv2d.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
> b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
> index 2b83896da9..2e5aacdc2b 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
> +++ b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
> @@ -38,6 +38,9 @@ typedef struct ThreadCommonParam{ typedef struct
> ThreadParam{
> ThreadCommonParam *thread_common_param;
> int thread_start, thread_end;
> +#if HAVE_PTHREAD_CANCEL
> +pthread_t thread;
> +#endif
> } ThreadParam;
>
> int ff_dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context,
> int file_size, int operands_num) @@ -187,7 +190,6 @@ int
> ff_dnn_execute_layer_conv2d(DnnOperand *operands, const int32_t
> *input_opera
> int thread_num = (ctx->options.conv2d_threads <= 0 ||
> ctx->options.conv2d_threads > av_cpu_count())
> ? (av_cpu_count() + 1) : (ctx->options.conv2d_threads); #if
> HAVE_PTHREAD_CANCEL
> -pthread_t *thread_id = av_malloc_array(thread_num, sizeof(*thread_id));
> int thread_stride;
> #endif
> ThreadParam **thread_param = av_malloc_array(thread_num,
> sizeof(*thread_param)); @@ -230,17 +232,15 @@ int
> ff_dnn_execute_layer_conv2d(DnnOperand *operands, const int32_t
> *input_opera
> thread_param[i]->thread_common_param =
> _common_param;
> thread_param[i]->thread_start = thread_stride * i + pad_size;
> thread_param[i]->thread_end = (i == thread_num - 1) ? (height -
> pad_size) : (thread_param[i]->thread_start + thread_stride);
> -pthread_create(_id[i], NULL,
> dnn_execute_layer_conv2d_thread, (void *)thread_param[i]);
> +pthread_create(_param[i]->thread, NULL,
> + dnn_execute_layer_conv2d_thread, (void *)thread_param[i]);
> }
>
> //join threads, res gets function return
> for (int i = 0; i < thread_num; i++){
> -pthread_join(thread_id[i], NULL);
> +pthread_join(thread_param[i]->thread, NULL);
> }
>
> //release memory
> -av_freep(_id);
> -
> for (int i = 0; i < thread_num; i++){
> av_freep(_param[i]);
> }
LGMT, and just one question: shall we reduce the allocation as less as
possible? thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 15/23] dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
Fixes Coverity issue #1473507.
Signed-off-by: Andreas Rheinhardt
---
libavfilter/dnn/dnn_backend_native_layer_conv2d.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
index 2b83896da9..2e5aacdc2b 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
+++ b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
@@ -38,6 +38,9 @@ typedef struct ThreadCommonParam{
typedef struct ThreadParam{
ThreadCommonParam *thread_common_param;
int thread_start, thread_end;
+#if HAVE_PTHREAD_CANCEL
+pthread_t thread;
+#endif
} ThreadParam;
int ff_dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context,
int file_size, int operands_num)
@@ -187,7 +190,6 @@ int ff_dnn_execute_layer_conv2d(DnnOperand *operands, const
int32_t *input_opera
int thread_num = (ctx->options.conv2d_threads <= 0 ||
ctx->options.conv2d_threads > av_cpu_count())
? (av_cpu_count() + 1) : (ctx->options.conv2d_threads);
#if HAVE_PTHREAD_CANCEL
-pthread_t *thread_id = av_malloc_array(thread_num, sizeof(*thread_id));
int thread_stride;
#endif
ThreadParam **thread_param = av_malloc_array(thread_num,
sizeof(*thread_param));
@@ -230,17 +232,15 @@ int ff_dnn_execute_layer_conv2d(DnnOperand *operands,
const int32_t *input_opera
thread_param[i]->thread_common_param = _common_param;
thread_param[i]->thread_start = thread_stride * i + pad_size;
thread_param[i]->thread_end = (i == thread_num - 1) ? (height -
pad_size) : (thread_param[i]->thread_start + thread_stride);
-pthread_create(_id[i], NULL, dnn_execute_layer_conv2d_thread,
(void *)thread_param[i]);
+pthread_create(_param[i]->thread, NULL,
dnn_execute_layer_conv2d_thread, (void *)thread_param[i]);
}
//join threads, res gets function return
for (int i = 0; i < thread_num; i++){
-pthread_join(thread_id[i], NULL);
+pthread_join(thread_param[i]->thread, NULL);
}
//release memory
-av_freep(_id);
-
for (int i = 0; i < thread_num; i++){
av_freep(_param[i]);
}
--
2.27.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
