Expose the flusher thread object to user in order to allow setting of thread name and thread affinity when needed #3009
Conversation
… thread name and thread affinity when needed
| @@ -38,6 +38,7 @@ class SPDLOG_API periodic_worker { | |||
| } | |||
| }); | |||
| } | |||
| std::thread *get_thread() { return &worker_thread_; } | |||
There was a problem hiding this comment.
why not return reference like in the rest of this pr?
There was a problem hiding this comment.
Fair question :)
To be honest I was just lazy to change some legacy APIs in my code. But that's not a good enough reason and indeed it makes no sense. I'll update the PR.
include/spdlog/spdlog-inl.h
Outdated
| @@ -89,4 +89,9 @@ SPDLOG_INLINE void apply_logger_env_levels(std::shared_ptr<logger> logger) { | |||
| details::registry::instance().apply_logger_env_levels(std::move(logger)); | |||
| } | |||
|
|
|||
| SPDLOG_INLINE std::unique_ptr<spdlog::details::periodic_worker> &get_flusher() | |||
| { | |||
| return details::registry::instance().get_flusher(); | |||
There was a problem hiding this comment.
This change is not needed, as the registry can be accessed
There was a problem hiding this comment.
It is not a must, I agree, but it allows to access directly from spdlog namespace (similar to spdlog::register_logger for example) which makes it neater IMHO.
There was a problem hiding this comment.
Better not add new api if not really necessary.
There was a problem hiding this comment.
ok
uploaded a new PR without this.
There was a problem hiding this comment.
There is a missing mutex lock in the get_flusher()
Add
std::lock_guard<std::mutex> lock(flusher_mutex_);…ence and not a pointer
… thread name and thread affinity when needed (#3009) * Expose the flusher thread object to user in order to allow setting of thread name and thread affinity when needed * Code review fix - periodic_worker thread getter should return a reference and not a pointer
In our app there is a need to set the thread name and thread affinity for all spawned thread. In order to do so with the spdlog flusher thread, I had to expose the thread from the internal data structure (periodic_worker) via the registery. I thought this might be worthy of a merge back into the main stream.