This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
The following commit(s) were added to refs/heads/master by this push:
new ccaa3ab4 Reduce logging overhead further (#552)
ccaa3ab4 is described below
commit ccaa3ab479a50d1a0ef10e8d914d2767f044da58
Author: Stephen Webb <[email protected]>
AuthorDate: Sat Oct 11 10:48:26 2025 +1100
Reduce logging overhead further (#552)
* The vector of appender pointers is now immutable
---
src/main/cpp/appenderattachableimpl.cpp | 159 +++++++++++++++++++-------------
1 file changed, 93 insertions(+), 66 deletions(-)
diff --git a/src/main/cpp/appenderattachableimpl.cpp
b/src/main/cpp/appenderattachableimpl.cpp
index 73d67355..304b7061 100644
--- a/src/main/cpp/appenderattachableimpl.cpp
+++ b/src/main/cpp/appenderattachableimpl.cpp
@@ -23,11 +23,45 @@ using namespace LOG4CXX_NS::helpers;
IMPLEMENT_LOG4CXX_OBJECT(AppenderAttachableImpl)
+using AppenderListPtr = std::shared_ptr<const AppenderList>;
+
+/** A vector of appender pointers. */
struct AppenderAttachableImpl::priv_data
{
- /** Array of appenders. */
- AppenderList appenderList;
+private: // Attributes
+#ifdef __cpp_lib_atomic_shared_ptr
+ std::atomic<AppenderListPtr> pAppenderList;
+#else // !defined(__cpp_lib_atomic_shared_ptr)
+ AppenderListPtr pAppenderList;
mutable std::mutex m_mutex;
+#endif // !defined(__cpp_lib_atomic_shared_ptr)
+
+public: // ...structors
+ priv_data(const AppenderList& newList = {})
+ : pAppenderList{ std::make_shared<const AppenderList>(newList) }
+ {}
+
+public: // Accessors
+ AppenderListPtr getAppenders() const
+ {
+#ifdef __cpp_lib_atomic_shared_ptr
+ return pAppenderList.load(std::memory_order_acquire);
+#else // !defined(__cpp_lib_atomic_shared_ptr)
+ std::lock_guard<std::mutex> lock( m_mutex );
+ return pAppenderList;
+#endif // !defined(__cpp_lib_atomic_shared_ptr)
+ }
+
+public: // Modifiers
+ void setAppenders(const AppenderList& newList)
+ {
+#ifdef __cpp_lib_atomic_shared_ptr
+ pAppenderList.store(std::make_shared<AppenderList>(newList),
std::memory_order_release);
+#else // !defined(__cpp_lib_atomic_shared_ptr)
+ std::lock_guard<std::mutex> lock( m_mutex );
+ pAppenderList = std::make_shared<const AppenderList>(newList);
+#endif // !defined(__cpp_lib_atomic_shared_ptr)
+ }
};
AppenderAttachableImpl::AppenderAttachableImpl()
@@ -36,76 +70,62 @@ AppenderAttachableImpl::AppenderAttachableImpl()
#if LOG4CXX_ABI_VERSION <= 15
AppenderAttachableImpl::AppenderAttachableImpl(Pool& pool)
- : m_priv()
{
}
#endif
-
AppenderAttachableImpl::~AppenderAttachableImpl()
{
-
}
+
void AppenderAttachableImpl::addAppender(const AppenderPtr newAppender)
{
- // Null values for newAppender parameter are strictly forbidden.
if (!newAppender)
- {
return;
- }
- if (!m_priv)
- m_priv = std::make_unique<AppenderAttachableImpl::priv_data>();
-
- std::lock_guard<std::mutex> lock( m_priv->m_mutex );
- AppenderList::iterator it = std::find(
- m_priv->appenderList.begin(),
m_priv->appenderList.end(), newAppender);
-
- if (it == m_priv->appenderList.end())
+ if (m_priv)
{
- m_priv->appenderList.push_back(newAppender);
+ auto allAppenders = m_priv->getAppenders();
+ if (allAppenders->end() == std::find(allAppenders->begin(),
allAppenders->end(), newAppender))
+ {
+ auto newAppenders = *allAppenders;
+ newAppenders.push_back(newAppender);
+ m_priv->setAppenders(newAppenders);
+ }
}
+ else
+ m_priv = std::make_unique<priv_data>(AppenderList{newAppender});
}
-int AppenderAttachableImpl::appendLoopOnAppenders(
- const spi::LoggingEventPtr& event,
- Pool& p)
+int AppenderAttachableImpl::appendLoopOnAppenders(const spi::LoggingEventPtr&
event, Pool& p)
{
- int numberAppended = 0;
+ int result = 0;
if (m_priv)
{
- // FallbackErrorHandler::error() may modify our list of
appenders
- // while we are iterating over them (if it holds the same
logger).
- // So, make a local copy of the appenders that we want to
iterate over
- // before actually iterating over them.
- AppenderList allAppenders = getAllAppenders();
- for (auto appender : allAppenders)
+ auto allAppenders = m_priv->getAppenders();
+ for (auto& appender : *allAppenders)
{
appender->doAppend(event, p);
- numberAppended++;
+ ++result;
}
}
-
- return numberAppended;
+ return result;
}
AppenderList AppenderAttachableImpl::getAllAppenders() const
{
AppenderList result;
if (m_priv)
- {
- std::lock_guard<std::mutex> lock( m_priv->m_mutex );
- result = m_priv->appenderList;
- }
+ result = *m_priv->getAppenders();
return result;
}
AppenderPtr AppenderAttachableImpl::getAppender(const LogString& name) const
{
AppenderPtr result;
- if (m_priv && !name.empty())
+ if (m_priv)
{
- std::lock_guard<std::mutex> lock( m_priv->m_mutex );
- for (auto appender : m_priv->appenderList)
+ auto allAppenders = m_priv->getAppenders();
+ for (auto& appender : *allAppenders)
{
if (name == appender->getName())
{
@@ -122,8 +142,8 @@ bool AppenderAttachableImpl::isAttached(const AppenderPtr
appender) const
bool result = false;
if (m_priv && appender)
{
- std::lock_guard<std::mutex> lock( m_priv->m_mutex );
- result = std::find(m_priv->appenderList.begin(),
m_priv->appenderList.end(), appender) != m_priv->appenderList.end();
+ auto allAppenders = m_priv->getAppenders();
+ result = allAppenders->end() !=
std::find(allAppenders->begin(), allAppenders->end(), appender);
}
return result;
}
@@ -132,10 +152,10 @@ void AppenderAttachableImpl::removeAllAppenders()
{
if (m_priv)
{
- for (auto a : getAllAppenders())
- a->close();
- std::lock_guard<std::mutex> lock( m_priv->m_mutex );
- m_priv->appenderList.clear();
+ auto allAppenders = m_priv->getAppenders();
+ for (auto& appender : *allAppenders)
+ appender->close();
+ m_priv->setAppenders({});
}
}
@@ -143,27 +163,31 @@ void AppenderAttachableImpl::removeAppender(const
AppenderPtr appender)
{
if (m_priv && appender)
{
- std::lock_guard<std::mutex> lock( m_priv->m_mutex );
- auto it = std::find(m_priv->appenderList.begin(),
m_priv->appenderList.end(), appender);
- if (it != m_priv->appenderList.end())
+ auto newAppenders = *m_priv->getAppenders();
+ auto pItem = std::find(newAppenders.begin(),
newAppenders.end(), appender);
+ if (newAppenders.end() != pItem)
{
- m_priv->appenderList.erase(it);
+ newAppenders.erase(pItem);
+ m_priv->setAppenders(newAppenders);
}
}
}
void AppenderAttachableImpl::removeAppender(const LogString& name)
{
- if (m_priv && !name.empty())
+ if (m_priv)
{
- std::lock_guard<std::mutex> lock( m_priv->m_mutex );
- auto it = std::find_if(m_priv->appenderList.begin(),
m_priv->appenderList.end()
+ auto newAppenders = *m_priv->getAppenders();
+ auto pItem = std::find_if(newAppenders.begin(),
newAppenders.end()
, [&name](const AppenderPtr& appender) -> bool
{
return name == appender->getName();
});
- if (it != m_priv->appenderList.end())
- m_priv->appenderList.erase(it);
+ if (newAppenders.end() != pItem)
+ {
+ newAppenders.erase(pItem);
+ m_priv->setAppenders(newAppenders);
+ }
}
}
@@ -172,16 +196,17 @@ bool AppenderAttachableImpl::replaceAppender(const
AppenderPtr& oldAppender, con
bool found = false;
if (m_priv && oldAppender && newAppender)
{
- auto oldName = oldAppender->getName();
- std::lock_guard<std::mutex> lock( m_priv->m_mutex );
- auto it = std::find_if(m_priv->appenderList.begin(),
m_priv->appenderList.end()
- , [&oldName](const AppenderPtr& appender) -> bool
+ auto name = oldAppender->getName();
+ auto newAppenders = *m_priv->getAppenders();
+ auto pItem = std::find_if(newAppenders.begin(),
newAppenders.end()
+ , [&name](const AppenderPtr& appender) -> bool
{
- return oldName == appender->getName();
+ return name == appender->getName();
});
- if (it != m_priv->appenderList.end())
+ if (newAppenders.end() != pItem)
{
- *it = newAppender;
+ *pItem = newAppender;
+ m_priv->setAppenders(newAppenders);
found = true;
}
}
@@ -190,13 +215,15 @@ bool AppenderAttachableImpl::replaceAppender(const
AppenderPtr& oldAppender, con
void AppenderAttachableImpl::replaceAppenders(const AppenderList& newList)
{
- auto oldAppenders = getAllAppenders();
- if (!m_priv)
- m_priv = std::make_unique<AppenderAttachableImpl::priv_data>();
- std::lock_guard<std::mutex> lock( m_priv->m_mutex );
- for (auto a : oldAppenders)
- a->close();
- m_priv->appenderList = newList;
+ if (m_priv)
+ {
+ auto allAppenders = m_priv->getAppenders();
+ for (auto& a : *allAppenders)
+ a->close();
+ m_priv->setAppenders(newList);
+ }
+ else
+ m_priv = std::make_unique<priv_data>(newList);
}
