[RFC,v2,6/9] libcamera: base: log: Pass dynamic prefix through
diff mbox series

Message ID 20250130195811.1230581-7-pobrn@protonmail.com
State New
Headers show
Series
  • libcamera: base: log: Misc. changes
Related show

Commit Message

Barnabás Pőcze Jan. 30, 2025, 7:58 p.m. UTC
Use move construction to essentially pass through the string
returned by `Loggable::logPrefix()` to avoid an unnecessary copy.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/base/log.h | 2 +-
 src/libcamera/base/log.cpp   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Jacopo Mondi Feb. 3, 2025, 4:48 p.m. UTC | #1
Hi Barnabás

On Thu, Jan 30, 2025 at 07:58:47PM +0000, Barnabás Pőcze wrote:
> Use move construction to essentially pass through the string
> returned by `Loggable::logPrefix()` to avoid an unnecessary copy.
>
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/base/log.h | 2 +-
>  src/libcamera/base/log.cpp   | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h
> index acef24203..1fb92603f 100644
> --- a/include/libcamera/base/log.h
> +++ b/include/libcamera/base/log.h
> @@ -64,7 +64,7 @@ class LogMessage
>  public:
>  	LogMessage(const char *fileName, unsigned int line,
>  		   const LogCategory &category, LogSeverity severity,
> -		   const std::string &prefix = std::string());
> +		   std::string prefix = {});
>  	~LogMessage();
>
>  	std::ostream &stream() { return msgStream_; }
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 9415b6937..0dfdb0e9b 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -872,9 +872,9 @@ const LogCategory &LogCategory::defaultCategory()
>   */
>  LogMessage::LogMessage(const char *fileName, unsigned int line,
>  		       const LogCategory &category, LogSeverity severity,
> -		       const std::string &prefix)
> +		       std::string prefix)

Aren't we creating an instance here (using the std::string copy-constructor)

>  	: category_(category), severity_(severity),
> -	  timestamp_(utils::clock::now()), prefix_(prefix)
> +	  timestamp_(utils::clock::now()), prefix_(std::move(prefix))

Just to move it here ?

>  {
>  	std::ostringstream ossFileInfo;
>  	ossFileInfo << utils::basename(fileName) << ":" << line;
> --
> 2.48.1
>
>
Barnabás Pőcze Feb. 3, 2025, 6:25 p.m. UTC | #2
2025. február 3., hétfő 17:48 keltezéssel, Jacopo Mondi <jacopo.mondi@ideasonboard.com> írta:

> Hi Barnabás
> 
> On Thu, Jan 30, 2025 at 07:58:47PM +0000, Barnabás Pőcze wrote:
> > Use move construction to essentially pass through the string
> > returned by `Loggable::logPrefix()` to avoid an unnecessary copy.
> >
> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  include/libcamera/base/log.h | 2 +-
> >  src/libcamera/base/log.cpp   | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h
> > index acef24203..1fb92603f 100644
> > --- a/include/libcamera/base/log.h
> > +++ b/include/libcamera/base/log.h
> > @@ -64,7 +64,7 @@ class LogMessage
> >  public:
> >  	LogMessage(const char *fileName, unsigned int line,
> >  		   const LogCategory &category, LogSeverity severity,
> > -		   const std::string &prefix = std::string());
> > +		   std::string prefix = {});
> >  	~LogMessage();
> >
> >  	std::ostream &stream() { return msgStream_; }
> > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> > index 9415b6937..0dfdb0e9b 100644
> > --- a/src/libcamera/base/log.cpp
> > +++ b/src/libcamera/base/log.cpp
> > @@ -872,9 +872,9 @@ const LogCategory &LogCategory::defaultCategory()
> >   */
> >  LogMessage::LogMessage(const char *fileName, unsigned int line,
> >  		       const LogCategory &category, LogSeverity severity,
> > -		       const std::string &prefix)
> > +		       std::string prefix)
> 
> Aren't we creating an instance here (using the std::string copy-constructor)

Yes, but inside the function there is no copying anymore because this "local"
instance is moved into the member variable.


> 
> >  	: category_(category), severity_(severity),
> > -	  timestamp_(utils::clock::now()), prefix_(prefix)
> > +	  timestamp_(utils::clock::now()), prefix_(std::move(prefix))
> 
> Just to move it here ?
> 
> >  {
> >  	std::ostringstream ossFileInfo;
> >  	ossFileInfo << utils::basename(fileName) << ":" << line;
> > --
> > 2.48.1
> >
> >
>

Patch
diff mbox series

diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h
index acef24203..1fb92603f 100644
--- a/include/libcamera/base/log.h
+++ b/include/libcamera/base/log.h
@@ -64,7 +64,7 @@  class LogMessage
 public:
 	LogMessage(const char *fileName, unsigned int line,
 		   const LogCategory &category, LogSeverity severity,
-		   const std::string &prefix = std::string());
+		   std::string prefix = {});
 	~LogMessage();
 
 	std::ostream &stream() { return msgStream_; }
diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
index 9415b6937..0dfdb0e9b 100644
--- a/src/libcamera/base/log.cpp
+++ b/src/libcamera/base/log.cpp
@@ -872,9 +872,9 @@  const LogCategory &LogCategory::defaultCategory()
  */
 LogMessage::LogMessage(const char *fileName, unsigned int line,
 		       const LogCategory &category, LogSeverity severity,
-		       const std::string &prefix)
+		       std::string prefix)
 	: category_(category), severity_(severity),
-	  timestamp_(utils::clock::now()), prefix_(prefix)
+	  timestamp_(utils::clock::now()), prefix_(std::move(prefix))
 {
 	std::ostringstream ossFileInfo;
 	ossFileInfo << utils::basename(fileName) << ":" << line;