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

[Laurent Pinchart]

On Tue, May 10, 2022 at 04:22:51PM +0300, Laurent Pinchart wrote:
> 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 ?

It would also be useful to make the same measurements using the C++20
std::format, as that would be the long term goal.

> > 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