[RFC,v1,5/7] libcamera: base: log: Use `std::string_view` to avoid some copies
diff mbox series

Message ID 20250121185554.301901-3-pobrn@protonmail.com
State Superseded
Headers show
Series
  • libcamera: base: log: Misc. changes
Related show

Commit Message

Barnabás Pőcze Jan. 21, 2025, 6:56 p.m. UTC
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@protonmail.com>
---
 include/libcamera/base/log.h |  5 +++--
 src/libcamera/base/log.cpp   | 25 +++++++++++++------------
 2 files changed, 16 insertions(+), 14 deletions(-)

Comments

Laurent Pinchart Jan. 24, 2025, 7:11 p.m. UTC | #1
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@protonmail.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@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)
>  {
>  }

Patch
diff mbox series

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)
 {
 }