Skip to content

Commit

Permalink
replace std::mutex with a spinlock
Browse files Browse the repository at this point in the history
  • Loading branch information
odygrd authored Jul 21, 2024
1 parent be26b0b commit ca19592
Show file tree
Hide file tree
Showing 10 changed files with 118 additions and 34 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@
Previously, this would result in an error being displayed but no logs being written. This issue is now resolved: the
error is reported once, and logs will be written to the console without colors.
- Improved performance of `StringFromTime` and `TimestampFormatter` used by the backend worker thread.
- Replaced `std::mutex` with a spinlock, resulting in minor performance improvement for backend worker. This change
also avoids including `<mutex>` in the frontend, particularly when following the
[recommended_usage](https://github.com/odygrd/quill/blob/master/examples/recommended_usage/recommended_usage.cpp)
example
- Update bundled `libfmt` to `11.0.2`

## v5.0.0
Expand Down
Binary file modified docs/quill_v5_1_compiler_profile.speedscope.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions quill/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ set(HEADER_FILES
include/quill/core/QuillError.h
include/quill/core/Rdtsc.h
include/quill/core/SinkManager.h
include/quill/core/Spinlock.h
include/quill/core/ThreadContextManager.h
include/quill/core/ThreadUtilities.h
include/quill/core/TimeUtilities.h
Expand Down
18 changes: 9 additions & 9 deletions quill/include/quill/core/LoggerManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
#include "quill/core/Attributes.h"
#include "quill/core/Common.h"
#include "quill/core/LoggerBase.h"
#include "quill/core/Spinlock.h"

#include <algorithm>
#include <atomic>
#include <cassert>
#include <initializer_list>
#include <memory>
#include <mutex>
#include <string>
#include <vector>

Expand Down Expand Up @@ -42,15 +42,15 @@ class LoggerManager
/***/
QUILL_NODISCARD LoggerBase* get_logger(std::string const& logger_name) const
{
std::lock_guard<std::recursive_mutex> const lock{_mutex};
LockGuard const lock{_spinlock};
LoggerBase* logger = _find_logger(logger_name);
return logger && logger->is_valid_logger() ? logger : nullptr;
}

/***/
QUILL_NODISCARD std::vector<LoggerBase*> get_all_loggers() const
{
std::lock_guard<std::recursive_mutex> const lock{_mutex};
LockGuard const lock{_spinlock};

std::vector<LoggerBase*> loggers;

Expand All @@ -70,7 +70,7 @@ class LoggerManager
QUILL_NODISCARD LoggerBase* get_valid_logger() const
{
// Retrieves any valid logger without the need for constructing a vector
std::lock_guard<std::recursive_mutex> const lock{_mutex};
LockGuard const lock{_spinlock};

for (auto const& elem : _loggers)
{
Expand All @@ -87,7 +87,7 @@ class LoggerManager
/***/
QUILL_NODISCARD size_t get_number_of_loggers() const noexcept
{
std::lock_guard<std::recursive_mutex> const lock{_mutex};
LockGuard const lock{_spinlock};
return _loggers.size();
}

Expand All @@ -97,7 +97,7 @@ class LoggerManager
template <typename TCallback>
void for_each_logger(TCallback cb) const
{
std::lock_guard<std::recursive_mutex> const lock{_mutex};
LockGuard const lock{_spinlock};

for (auto const& elem : _loggers)
{
Expand All @@ -118,7 +118,7 @@ class LoggerManager
std::string const& time_pattern, Timezone timestamp_timezone,
ClockSourceType clock_source, UserClockSource* user_clock)
{
std::lock_guard<std::recursive_mutex> const lock{_mutex};
LockGuard const lock{_spinlock};

LoggerBase* logger_ptr = _find_logger(logger_name);

Expand Down Expand Up @@ -159,7 +159,7 @@ class LoggerManager
{
_has_invalidated_loggers.store(false, std::memory_order_release);

std::lock_guard<std::recursive_mutex> const lock{_mutex};
LockGuard const lock{_spinlock};
for (auto it = _loggers.begin(); it != _loggers.end();)
{
if (!it->get()->is_valid_logger())
Expand Down Expand Up @@ -221,7 +221,7 @@ class LoggerManager

private:
std::vector<std::unique_ptr<LoggerBase>> _loggers;
mutable std::recursive_mutex _mutex;
mutable Spinlock _spinlock;
std::atomic<bool> _has_invalidated_loggers{false};
};
} // namespace detail
Expand Down
12 changes: 11 additions & 1 deletion quill/include/quill/core/MathUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@ QUILL_NODISCARD constexpr bool is_power_of_two(uint64_t number) noexcept
return (number != 0) && ((number & (number - 1)) == 0);
}

/**
* Helper function to calculate the maximum power of two for type T
* @return maximum power of two for type T
*/
template <typename T>
QUILL_NODISCARD constexpr T max_power_of_two() noexcept
{
return (std::numeric_limits<T>::max() >> 1) + 1;
}

/**
* Round up to the next power of 2
* @param n input
Expand All @@ -32,7 +42,7 @@ QUILL_NODISCARD constexpr bool is_power_of_two(uint64_t number) noexcept
template <typename T>
QUILL_NODISCARD T next_power_of_two(T n)
{
constexpr T max_power_of_2 = (std::numeric_limits<T>::max() >> 1) + 1;
constexpr T max_power_of_2 = max_power_of_two<T>();

if (n >= max_power_of_2)
{
Expand Down
11 changes: 6 additions & 5 deletions quill/include/quill/core/SinkManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@

#include "quill/core/Attributes.h"
#include "quill/core/QuillError.h"
#include "quill/core/Spinlock.h"

#include <algorithm>
#include <cstdint>
#include <memory>
#include <mutex>
#include <string>
#include <type_traits>
#include <vector>
Expand Down Expand Up @@ -52,7 +52,7 @@ class SinkManager
QUILL_NODISCARD std::shared_ptr<Sink> get_sink(std::string const& sink_name) const
{
// The sinks are used by the backend thread, so after their creation we want to avoid mutating their member variables.
std::lock_guard<std::mutex> const lock{_mutex};
LockGuard const lock{_spinlock};

std::shared_ptr<Sink> sink = _find_sink(sink_name);

Expand All @@ -69,7 +69,7 @@ class SinkManager
std::shared_ptr<Sink> create_or_get_sink(std::string const& sink_name, Args&&... args)
{
// The sinks are used by the backend thread, so after their creation we want to avoid mutating their member variables.
std::lock_guard<std::mutex> const lock{_mutex};
LockGuard const lock{_spinlock};

std::shared_ptr<Sink> sink = _find_sink(sink_name);

Expand All @@ -95,7 +95,8 @@ class SinkManager
{
// this needs to take a lock each time. The backend logging thread should be carefully call
// it only when needed
std::lock_guard<std::mutex> const lock{_mutex};
LockGuard const lock{_spinlock};

uint32_t cnt{0};
for (auto it = _sinks.begin(); it != _sinks.end();)
{
Expand Down Expand Up @@ -146,7 +147,7 @@ class SinkManager

private:
std::vector<SinkInfo> _sinks;
mutable std::mutex _mutex;
mutable Spinlock _spinlock;
};
} // namespace detail
} // namespace quill
70 changes: 70 additions & 0 deletions quill/include/quill/core/Spinlock.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/**
* Copyright(c) 2020-present, Odysseas Georgoudis & quill contributors.
* Distributed under the MIT License (http://opensource.org/licenses/MIT)
*/

#pragma once

#include "quill/core/Attributes.h"
#include <atomic>
#include <cstdint>

namespace quill::detail
{
/***/
class Spinlock
{
public:
Spinlock() = default;

/**
* Deleted
*/
Spinlock(Spinlock const&) = delete;
Spinlock& operator=(Spinlock const&) = delete;

/***/
QUILL_ATTRIBUTE_HOT void lock() noexcept
{
do
{
while (_flag.load(std::memory_order_relaxed) == State::Locked)
{
// keep trying
}
} while (_flag.exchange(State::Locked, std::memory_order_acquire) == State::Locked);
}

/***/
QUILL_ATTRIBUTE_HOT void unlock() noexcept
{
_flag.store(State::Free, std::memory_order_release);
}

private:
enum class State : uint8_t
{
Free = 0,
Locked = 1
};

std::atomic<State> _flag{State::Free};
};

/***/
class LockGuard
{
public:
explicit LockGuard(Spinlock& s) : _spinlock(s) { _spinlock.lock(); }
~LockGuard() { _spinlock.unlock(); }

/**
* Deleted
*/
LockGuard(LockGuard const&) = delete;
LockGuard& operator=(LockGuard const&) = delete;

private:
Spinlock& _spinlock;
};
} // namespace quill::detail
17 changes: 8 additions & 9 deletions quill/include/quill/core/ThreadContextManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "quill/core/Attributes.h"
#include "quill/core/BoundedSPSCQueue.h"
#include "quill/core/Common.h"
#include "quill/core/Spinlock.h"
#include "quill/core/ThreadUtilities.h"
#include "quill/core/UnboundedSPSCQueue.h"

Expand All @@ -16,7 +17,6 @@
#include <cstdint>
#include <cstdlib>
#include <memory>
#include <mutex>
#include <new>
#include <string>
#include <string_view>
Expand Down Expand Up @@ -224,7 +224,7 @@ class ThreadContextManager
template <typename TCallback>
void for_each_thread_context(TCallback cb)
{
std::lock_guard<std::mutex> const lock{_mutex};
LockGuard const lock{_spinlock};

for (auto const& elem : _thread_contexts)
{
Expand All @@ -235,9 +235,9 @@ class ThreadContextManager
/***/
void register_thread_context(std::shared_ptr<ThreadContext> const& thread_context)
{
_mutex.lock();
_spinlock.lock();
_thread_contexts.push_back(thread_context);
_mutex.unlock();
_spinlock.unlock();
_new_thread_context_flag.store(true, std::memory_order_release);
}

Expand Down Expand Up @@ -274,7 +274,7 @@ class ThreadContextManager
/***/
void remove_shared_invalidated_thread_context(ThreadContext const* thread_context)
{
std::lock_guard<std::mutex> const lock{_mutex};
LockGuard const lock{_spinlock};

// We could use std::find_if, but since this header is included in Logger.h, which is essential
// for logging purposes, we aim to minimize the number of includes in that path.
Expand Down Expand Up @@ -321,11 +321,10 @@ class ThreadContextManager
~ThreadContextManager() = default;

private:
std::mutex _mutex; /**< Protect access when register contexts or removing contexts */
std::vector<std::shared_ptr<ThreadContext>> _thread_contexts; /**< The registered contexts */

alignas(CACHE_LINE_ALIGNED) std::atomic<bool> _new_thread_context_flag{false};
alignas(CACHE_LINE_ALIGNED) std::atomic<uint8_t> _invalid_thread_context_count{0};
Spinlock _spinlock; /**< Protect access when register contexts or removing contexts */
std::atomic<bool> _new_thread_context_flag{false};
std::atomic<uint8_t> _invalid_thread_context_count{0};
};

class ScopedThreadContext
Expand Down
5 changes: 2 additions & 3 deletions quill/include/quill/core/UnboundedSPSCQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
#include "quill/core/BoundedSPSCQueue.h"
#include "quill/core/Common.h"
#include "quill/core/QuillError.h"
#include "quill/core/MathUtils.h"

#include <atomic>
#include <cassert>
#include <cstddef>
#include <cstdint>
#include <limits>
#include <string>

namespace quill::detail
Expand Down Expand Up @@ -121,8 +121,7 @@ class UnboundedSPSCQueue
}

// bounded queue max power of 2 capacity since uint32_t type is used to hold the value 2147483648 bytes
uint64_t constexpr max_bounded_queue_capacity =
(std::numeric_limits<BoundedSPSCQueue::integer_type>::max() >> 1) + 1;
uint64_t constexpr max_bounded_queue_capacity = max_power_of_two<BoundedSPSCQueue::integer_type>();

if (QUILL_UNLIKELY(capacity > max_bounded_queue_capacity))
{
Expand Down
14 changes: 7 additions & 7 deletions quill/include/quill/sinks/Sink.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
#include "quill/core/Attributes.h"
#include "quill/core/LogLevel.h"
#include "quill/core/QuillError.h"
#include "quill/core/Spinlock.h"
#include "quill/filters/Filter.h"

#include <algorithm>
#include <atomic>
#include <cstdint>
#include <memory>
#include <mutex>
#include <string>
#include <string_view>
#include <utility>
Expand Down Expand Up @@ -109,7 +109,7 @@ class Sink
void add_filter(std::unique_ptr<Filter> filter)
{
// Lock and add this filter to our global collection
std::lock_guard<std::recursive_mutex> const lock{_global_filters_lock};
detail::LockGuard const lock{_global_filters_lock};

// Check if the same filter already exists
auto const search_filter_it = std::find_if(
Expand Down Expand Up @@ -156,7 +156,8 @@ class Sink
// if there is a new filter we have to update
_local_filters.clear();

std::lock_guard<std::recursive_mutex> const lock{_global_filters_lock};
detail::LockGuard const lock{_global_filters_lock};

for (auto const& filter : _global_filters)
{
_local_filters.push_back(filter.get());
Expand Down Expand Up @@ -188,12 +189,11 @@ class Sink

/** Global filter for this sink **/
std::vector<std::unique_ptr<Filter>> _global_filters;
std::recursive_mutex _global_filters_lock;

std::atomic<LogLevel> _log_level{LogLevel::TraceL3};

detail::Spinlock _global_filters_lock;
/** Indicator that a new filter was added **/
std::atomic<bool> _new_filter{false};

std::atomic<LogLevel> _log_level{LogLevel::TraceL3};
};

} // namespace quill

0 comments on commit ca19592

Please sign in to comment.