[RFC PATCH v1 5/7] libcamera: base: log: Use `std::string_view` to avoid some copies

[Laurent Pinchart]

Hi Barnabás,

Thank you for the patch.

On Tue, Jan 21, 2025 at 06:56:07PM +0000, Barnabás Pőcze wrote:
> Use `std::string_view` to avoid some largely unnecessary copies, and
> to make string comparisong potentially faster by eliminating repeated
> `strlen()` calls.
> 
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  include/libcamera/base/log.h |  5 +++--
>  src/libcamera/base/log.cpp   | 25 +++++++++++++------------
>  2 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h
> index 89f707e54..9c7775660 100644
> --- a/include/libcamera/base/log.h
> +++ b/include/libcamera/base/log.h
> @@ -9,6 +9,7 @@
>  
>  #include <atomic>
>  #include <sstream>
> +#include <string_view>
>  
>  #include <libcamera/base/private.h>
>  
> @@ -29,7 +30,7 @@ enum LogSeverity {
>  class LogCategory
>  {
>  public:
> -	static LogCategory *create(const char *name);
> +	static LogCategory *create(std::string_view name);
>  
>  	const std::string &name() const { return name_; }
>  	LogSeverity severity() const { return severity_.load(std::memory_order_relaxed); }
> @@ -38,7 +39,7 @@ public:
>  	static const LogCategory &defaultCategory();
>  
>  private:
> -	explicit LogCategory(const char *name);
> +	explicit LogCategory(std::string_view name);
>  
>  	const std::string name_;
>  
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 795260b0a..78db4990c 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -15,6 +15,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <string_view>
>  #include <syslog.h>
>  #include <time.h>
>  #include <unordered_set>
> @@ -313,11 +314,11 @@ private:
>  
>  	void parseLogFile();
>  	void parseLogLevels();
> -	static LogSeverity parseLogLevel(const std::string &level);
> +	static LogSeverity parseLogLevel(std::string_view level);
>  
>  	friend LogCategory;
>  	void registerCategory(LogCategory *category);
> -	LogCategory *findCategory(const char *name) const;
> +	LogCategory *findCategory(std::string_view name) const;
>  
>  	static bool destroyed_;
>  
> @@ -642,17 +643,17 @@ void Logger::parseLogLevels()
>  		if (!len)
>  			continue;
>  
> -		std::string category;
> -		std::string level;
> +		std::string_view category;
> +		std::string_view level;
>  
>  		const char *colon = static_cast<const char *>(memchr(pair, ':', len));
>  		if (!colon) {
>  			/* 'x' is a shortcut for '*:x'. */
>  			category = "*";
> -			level = std::string(pair, len);
> +			level = std::string_view(pair, len);
>  		} else {
> -			category = std::string(pair, colon - pair);
> -			level = std::string(colon + 1, comma - colon - 1);
> +			category = std::string_view(pair, colon - pair);
> +			level = std::string_view(colon + 1, comma - colon - 1);
>  		}
>  
>  		/* Both the category and the level must be specified. */
> @@ -663,7 +664,7 @@ void Logger::parseLogLevels()
>  		if (severity == LogInvalid)
>  			continue;
>  
> -		levels_.push_back({ category, severity });
> +		levels_.emplace_back(category, severity);
>  	}
>  }
>  
> @@ -677,7 +678,7 @@ void Logger::parseLogLevels()
>   *
>   * \return The log severity, or LogInvalid if the string is invalid
>   */
> -LogSeverity Logger::parseLogLevel(const std::string &level)
> +LogSeverity Logger::parseLogLevel(std::string_view level)
>  {
>  	static const char *const names[] = {
>  		"DEBUG",
> @@ -744,7 +745,7 @@ void Logger::registerCategory(LogCategory *category)
>   * \param[in] name Name of the log category
>   * \return The pointer to the found log category or nullptr if not found
>   */
> -LogCategory *Logger::findCategory(const char *name) const
> +LogCategory *Logger::findCategory(std::string_view name) const
>  {
>  	if (auto it = std::find_if(categories_.begin(), categories_.end(),
>  				   [name](auto c) { return c->name() == name; });
> @@ -788,7 +789,7 @@ LogCategory *Logger::findCategory(const char *name) const
>   *
>   * \return The pointer to the LogCategory
>   */
> -LogCategory *LogCategory::create(const char *name)
> +LogCategory *LogCategory::create(std::string_view name)
>  {
>  	static Mutex mutex_;
>  	MutexLocker locker(mutex_);
> @@ -806,7 +807,7 @@ LogCategory *LogCategory::create(const char *name)
>   * \brief Construct a log category
>   * \param[in] name The category name
>   */
> -LogCategory::LogCategory(const char *name)
> +LogCategory::LogCategory(std::string_view name)
>  	: name_(name), severity_(LogSeverity::LogInfo)
>  {
>  }

-- 
Regards,

Laurent Pinchart