[libcamera-devel] [PATCH] cam: convert to libfmt

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, May 10, 2022 at 10:16:17AM +0300, Tomi Valkeinen wrote:
> This is just a conversation starter, not for merging. I really like
> libfmt, and I really hate the C++ stream-based string formatting. libfmt
> is the first library I add to any new C++ project I make.

You've got it wrong. The first library you should add to any new C++
project you make is libcamera :-)

> You can find more information about libfmt from:
> 
> https://github.com/fmtlib/fmt
> https://fmt.dev/latest/index.html
> 
> This patch is just a crude conversion with ugly macros to showcase what
> the formatting code might look like.
> 
> libfmt pages suggest that libfmt would be faster and smaller (exe size)
> than iostreams, but for the size it didn't seem to be true in cam's case
> as the tests below show. However, simple prints did reduce the exe size,
> but the few more complex ones increased the size.
> 
> Size tests with gcc 11.2.0-19ubuntu1
> 
> - Without libfmt
> 
> debug		3523400
> debug lto	3269368
> release		223056
> release lto	172280
> 
> - With libfmt
> 
> debug		4424256
> debug lto	4143840
> release		303952
> release lto	252640
> 
> Above shows that cam's size clearly increases with libfmt. However, the
> increase really comes only from one case, the use of fmt::memory_buffer
> and std::back_inserter.

Is this because libfmt is a header-only library ? What if it is built as
a shared object, what's the impact on code size in cam ?

> Converting that code to use fmt::format() and
> naive string append gives:
> 
> release with string append	233680
> release lto with string append	186936

What's the impact of switching to string concatenation on runtime ?

Would you consider the option of keeping std::stringstream ?

> Also, if I add another use of fmt::memory_buffer and std::back_inserter
> to another file, I see much less increase in the size:
> 
> release lto with two uses of memory_buffer, back_inserter	256736
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> ---
>  src/cam/camera_session.cpp | 105 ++++++++++++++++---------------------
>  src/cam/drm.cpp            |  68 ++++++++----------------
>  src/cam/event_loop.cpp     |   9 ++--
>  src/cam/file_sink.cpp      |  31 +++++------
>  src/cam/image.cpp          |  15 +++---
>  src/cam/kms_sink.cpp       |  38 ++++++--------
>  src/cam/main.cpp           |  28 +++++-----
>  src/cam/meson.build        |   5 +-
>  src/cam/options.cpp        |  66 ++++++++++-------------
>  src/cam/stream_options.cpp |  18 +++----
>  10 files changed, 165 insertions(+), 218 deletions(-)

[snip]

-- 
Regards,

Laurent Pinchart