diff mbox series

[FFmpeg-devel] avformat/matroskaenc: Remove unnecessary function calls

Message ID HE1PR0301MB215465E05E5D12F58E2262438F4E9@HE1PR0301MB2154.eurprd03.prod.outlook.com
State Accepted
Commit 5627da3555ecbf6fbfd98d045abbc7b63bc98fce
Headers show
Series [FFmpeg-devel] avformat/matroskaenc: Remove unnecessary function calls
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt April 14, 2021, 9:22 p.m. UTC
ffio_fill() is used when initially writing unknown length elements;
yet it can happen that the amount of bytes written by it is zero in
which case it is of course unnecessary to ever call it. Whether it is
possible to know this during compiletime depends upon how aggressively
the compiler inlines function calls (i.e. if it inlines calls to
start_ebml_master() where the upper bound for the size of the element
implies that the size will be written on one byte) and this depends upon
optimization settings. It is not the aim of this patch to inline all
calls where it is known that ffio_fill() will be unnecessary, but merely
to make compilers that inline such calls aware of the fact that writing
zero bytes with ffio_fill() is unnecessary. To this end
av_builtin_constant_p() is used to check whether the size is a
compiletime constant.

For GCC 10 this made a difference at -O3 only: The size of .text
decreased from 0x747F (with 29 calls to ffio_fill(), eight of which
use size zero) to 0x7337 (with 21 calls to ffio_fill(), zero of which
use size zero).
For Clang 11 it made a difference at -O2 and -O3: At -O2, the size of
.text decreased from 0x879C to 0x871C (with eight calls to ffio_fill()
eliminated); at -O3 the size of .text decreased from 0xAF2F to 0xAEBF.
Once again, eight calls to ffio_fill() with size zero have been
eliminated.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/matroskaenc.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Andreas Rheinhardt April 16, 2021, 11:08 a.m. UTC | #1
Andreas Rheinhardt:
> ffio_fill() is used when initially writing unknown length elements;
> yet it can happen that the amount of bytes written by it is zero in
> which case it is of course unnecessary to ever call it. Whether it is
> possible to know this during compiletime depends upon how aggressively
> the compiler inlines function calls (i.e. if it inlines calls to
> start_ebml_master() where the upper bound for the size of the element
> implies that the size will be written on one byte) and this depends upon
> optimization settings. It is not the aim of this patch to inline all
> calls where it is known that ffio_fill() will be unnecessary, but merely
> to make compilers that inline such calls aware of the fact that writing
> zero bytes with ffio_fill() is unnecessary. To this end
> av_builtin_constant_p() is used to check whether the size is a
> compiletime constant.
> 
> For GCC 10 this made a difference at -O3 only: The size of .text
> decreased from 0x747F (with 29 calls to ffio_fill(), eight of which
> use size zero) to 0x7337 (with 21 calls to ffio_fill(), zero of which
> use size zero).
> For Clang 11 it made a difference at -O2 and -O3: At -O2, the size of
> .text decreased from 0x879C to 0x871C (with eight calls to ffio_fill()
> eliminated); at -O3 the size of .text decreased from 0xAF2F to 0xAEBF.
> Once again, eight calls to ffio_fill() with size zero have been
> eliminated.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavformat/matroskaenc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 3649ac25a2..0141fb0b8d 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -195,6 +195,8 @@ static void put_ebml_size_unknown(AVIOContext *pb, int bytes)
>  {
>      av_assert0(bytes <= 8);
>      avio_w8(pb, 0x1ff >> bytes);
> +    if (av_builtin_constant_p(bytes) && bytes == 1)
> +        return;
>      ffio_fill(pb, 0xff, bytes - 1);
>  }
>  
> 
Will apply tomorrow unless there are  objections.

- Andreas
diff mbox series

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 3649ac25a2..0141fb0b8d 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -195,6 +195,8 @@  static void put_ebml_size_unknown(AVIOContext *pb, int bytes)
 {
     av_assert0(bytes <= 8);
     avio_w8(pb, 0x1ff >> bytes);
+    if (av_builtin_constant_p(bytes) && bytes == 1)
+        return;
     ffio_fill(pb, 0xff, bytes - 1);
 }