Overhead reduction does not require std::atomic<std::shared_ptr>'

Author: stephen-webb

src/main/cpp/appenderattachableimpl.cpp

@@ -23,194 +23,61 @@ using namespace LOG4CXX_NS::helpers;
 
 IMPLEMENT_LOG4CXX_OBJECT(AppenderAttachableImpl)
 
-#ifndef __cpp_lib_atomic_shared_ptr
-
-struct AppenderAttachableImpl::priv_data
-{
-	/** Array of appenders. */
-	AppenderList  appenderList;
-	mutable std::mutex m_mutex;
-};
-
-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())
-	{
-		m_priv->appenderList.push_back(newAppender);
-	}
-}
-
-int AppenderAttachableImpl::appendLoopOnAppenders(
-	const spi::LoggingEventPtr& event,
-	Pool& p)
-{
-	int numberAppended = 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)
-		{
-			appender->doAppend(event, p);
-			numberAppended++;
-		}
-	}
-
-	return numberAppended;
-}
-
-AppenderList AppenderAttachableImpl::getAllAppenders() const
-{
-	AppenderList result;
-	if (m_priv)
-	{
-		std::lock_guard<std::mutex> lock( m_priv->m_mutex );
-		result = m_priv->appenderList;
-	}
-	return result;
-}
-
-AppenderPtr AppenderAttachableImpl::getAppender(const LogString& name) const
-{
-	AppenderPtr result;
-	if (m_priv && !name.empty())
-	{
-		std::lock_guard<std::mutex> lock( m_priv->m_mutex );
-		for (auto appender : m_priv->appenderList)
-		{
-			if (name == appender->getName())
-			{
-				result = appender;
-				break;
-			}
-		}
-	}
-	return result;
-}
-
-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();
-	}
-	return result;
-}
-
-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();
-	}
-}
-
-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())
-		{
-			m_priv->appenderList.erase(it);
-		}
-	}
-}
-
-void AppenderAttachableImpl::removeAppender(const LogString& name)
-{
-	if (m_priv && !name.empty())
-	{
-		std::lock_guard<std::mutex> lock( m_priv->m_mutex );
-		auto it = std::find_if(m_priv->appenderList.begin(), m_priv->appenderList.end()
-			, [&name](const AppenderPtr& appender) -> bool
-			{
-				return name == appender->getName();
-			});
-		if (it != m_priv->appenderList.end())
-			m_priv->appenderList.erase(it);
-	}
-}
-
-bool AppenderAttachableImpl::replaceAppender(const AppenderPtr& oldAppender, const AppenderPtr& newAppender)
-{
-	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
-			{
-				return oldName == appender->getName();
-			});
-		if (it != m_priv->appenderList.end())
-		{
-			*it = newAppender;
-			found = true;
-		}
-	}
-	return found;
-}
-
-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;
-}
-
-#else // __cpp_lib_atomic_shared_ptr
-
 using AppenderListPtr = std::shared_ptr<const AppenderList>;
 
 /** A vector of appender pointers. */
 struct AppenderAttachableImpl::priv_data
 {
-private:
+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:
+public: // ...structors
 	priv_data(const AppenderList& newList = {})
 		: pAppenderList{ std::make_shared<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<AppenderList>(newList);
+#endif // !defined(__cpp_lib_atomic_shared_ptr)
 	}
 };
 
+AppenderAttachableImpl::AppenderAttachableImpl()
+{
+}
+
+#if LOG4CXX_ABI_VERSION <= 15
+AppenderAttachableImpl::AppenderAttachableImpl(Pool& pool)
+{
+}
+#endif
+AppenderAttachableImpl::~AppenderAttachableImpl()
+{
+}
+
+
 void AppenderAttachableImpl::addAppender(const AppenderPtr newAppender)
 {
 	if (!newAppender)
@@ -359,20 +226,4 @@ void AppenderAttachableImpl::replaceAppenders(const AppenderList& newList)
 		m_priv = std::make_unique<priv_data>(newList);
 }
 
-#endif // __cpp_lib_atomic_shared_ptr
-
-AppenderAttachableImpl::AppenderAttachableImpl()
-{
-}
-
-AppenderAttachableImpl::~AppenderAttachableImpl()
-{
-}
-
-#if LOG4CXX_ABI_VERSION <= 15
-AppenderAttachableImpl::AppenderAttachableImpl(Pool& pool)
-{
-}
-#endif
-