Skip to content

Commit ccaa3ab

Browse files
swebb2066swebb2066
authored
Reduce logging overhead further (#552)
* The vector of appender pointers is now immutable
1 parent ab11809 commit ccaa3ab

File tree

1 file changed

+93
-66
lines changed

1 file changed

+93
-66
lines changed

‎src/main/cpp/appenderattachableimpl.cpp‎

Lines changed: 93 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,45 @@ using namespace LOG4CXX_NS::helpers;
2323

2424
IMPLEMENT_LOG4CXX_OBJECT(AppenderAttachableImpl)
2525

26+
using AppenderListPtr = std::shared_ptr<const AppenderList>;
27+
28+
/** A vector of appender pointers. */
2629
struct AppenderAttachableImpl::priv_data
2730
{
28-
/** Array of appenders. */
29-
AppenderList appenderList;
31+
private: // Attributes
32+
#ifdef __cpp_lib_atomic_shared_ptr
33+
std::atomic<AppenderListPtr> pAppenderList;
34+
#else // !defined(__cpp_lib_atomic_shared_ptr)
35+
AppenderListPtr pAppenderList;
3036
mutable std::mutex m_mutex;
37+
#endif // !defined(__cpp_lib_atomic_shared_ptr)
38+
39+
public: // ...structors
40+
priv_data(const AppenderList& newList = {})
41+
: pAppenderList{ std::make_shared<const AppenderList>(newList) }
42+
{}
43+
44+
public: // Accessors
45+
AppenderListPtr getAppenders() const
46+
{
47+
#ifdef __cpp_lib_atomic_shared_ptr
48+
return pAppenderList.load(std::memory_order_acquire);
49+
#else // !defined(__cpp_lib_atomic_shared_ptr)
50+
std::lock_guard<std::mutex> lock( m_mutex );
51+
return pAppenderList;
52+
#endif // !defined(__cpp_lib_atomic_shared_ptr)
53+
}
54+
55+
public: // Modifiers
56+
void setAppenders(const AppenderList& newList)
57+
{
58+
#ifdef __cpp_lib_atomic_shared_ptr
59+
pAppenderList.store(std::make_shared<AppenderList>(newList), std::memory_order_release);
60+
#else // !defined(__cpp_lib_atomic_shared_ptr)
61+
std::lock_guard<std::mutex> lock( m_mutex );
62+
pAppenderList = std::make_shared<const AppenderList>(newList);
63+
#endif // !defined(__cpp_lib_atomic_shared_ptr)
64+
}
3165
};
3266

3367
AppenderAttachableImpl::AppenderAttachableImpl()
@@ -36,76 +70,62 @@ AppenderAttachableImpl::AppenderAttachableImpl()
3670

3771
#if LOG4CXX_ABI_VERSION <= 15
3872
AppenderAttachableImpl::AppenderAttachableImpl(Pool& pool)
39-
: m_priv()
4073
{
4174
}
4275
#endif
43-
4476
AppenderAttachableImpl::~AppenderAttachableImpl()
4577
{
46-
4778
}
4879

80+
4981
void AppenderAttachableImpl::addAppender(const AppenderPtr newAppender)
5082
{
51-
// Null values for newAppender parameter are strictly forbidden.
5283
if (!newAppender)
53-
{
5484
return;
55-
}
56-
if (!m_priv)
57-
m_priv = std::make_unique<AppenderAttachableImpl::priv_data>();
58-
59-
std::lock_guard<std::mutex> lock( m_priv->m_mutex );
60-
AppenderList::iterator it = std::find(
61-
m_priv->appenderList.begin(), m_priv->appenderList.end(), newAppender);
62-
63-
if (it == m_priv->appenderList.end())
85+
if (m_priv)
6486
{
65-
m_priv->appenderList.push_back(newAppender);
87+
auto allAppenders = m_priv->getAppenders();
88+
if (allAppenders->end() == std::find(allAppenders->begin(), allAppenders->end(), newAppender))
89+
{
90+
auto newAppenders = *allAppenders;
91+
newAppenders.push_back(newAppender);
92+
m_priv->setAppenders(newAppenders);
93+
}
6694
}
95+
else
96+
m_priv = std::make_unique<priv_data>(AppenderList{newAppender});
6797
}
6898

69-
int AppenderAttachableImpl::appendLoopOnAppenders(
70-
const spi::LoggingEventPtr& event,
71-
Pool& p)
99+
int AppenderAttachableImpl::appendLoopOnAppenders(const spi::LoggingEventPtr& event, Pool& p)
72100
{
73-
int numberAppended = 0;
101+
int result = 0;
74102
if (m_priv)
75103
{
76-
// FallbackErrorHandler::error() may modify our list of appenders
77-
// while we are iterating over them (if it holds the same logger).
78-
// So, make a local copy of the appenders that we want to iterate over
79-
// before actually iterating over them.
80-
AppenderList allAppenders = getAllAppenders();
81-
for (auto appender : allAppenders)
104+
auto allAppenders = m_priv->getAppenders();
105+
for (auto& appender : *allAppenders)
82106
{
83107
appender->doAppend(event, p);
84-
numberAppended++;
108+
++result;
85109
}
86110
}
87-
88-
return numberAppended;
111+
return result;
89112
}
90113

91114
AppenderList AppenderAttachableImpl::getAllAppenders() const
92115
{
93116
AppenderList result;
94117
if (m_priv)
95-
{
96-
std::lock_guard<std::mutex> lock( m_priv->m_mutex );
97-
result = m_priv->appenderList;
98-
}
118+
result = *m_priv->getAppenders();
99119
return result;
100120
}
101121

102122
AppenderPtr AppenderAttachableImpl::getAppender(const LogString& name) const
103123
{
104124
AppenderPtr result;
105-
if (m_priv && !name.empty())
125+
if (m_priv)
106126
{
107-
std::lock_guard<std::mutex> lock( m_priv->m_mutex );
108-
for (auto appender : m_priv->appenderList)
127+
auto allAppenders = m_priv->getAppenders();
128+
for (auto& appender : *allAppenders)
109129
{
110130
if (name == appender->getName())
111131
{
@@ -122,8 +142,8 @@ bool AppenderAttachableImpl::isAttached(const AppenderPtr appender) const
122142
bool result = false;
123143
if (m_priv && appender)
124144
{
125-
std::lock_guard<std::mutex> lock( m_priv->m_mutex );
126-
result = std::find(m_priv->appenderList.begin(), m_priv->appenderList.end(), appender) != m_priv->appenderList.end();
145+
auto allAppenders = m_priv->getAppenders();
146+
result = allAppenders->end() != std::find(allAppenders->begin(), allAppenders->end(), appender);
127147
}
128148
return result;
129149
}
@@ -132,38 +152,42 @@ void AppenderAttachableImpl::removeAllAppenders()
132152
{
133153
if (m_priv)
134154
{
135-
for (auto a : getAllAppenders())
136-
a->close();
137-
std::lock_guard<std::mutex> lock( m_priv->m_mutex );
138-
m_priv->appenderList.clear();
155+
auto allAppenders = m_priv->getAppenders();
156+
for (auto& appender : *allAppenders)
157+
appender->close();
158+
m_priv->setAppenders({});
139159
}
140160
}
141161

142162
void AppenderAttachableImpl::removeAppender(const AppenderPtr appender)
143163
{
144164
if (m_priv && appender)
145165
{
146-
std::lock_guard<std::mutex> lock( m_priv->m_mutex );
147-
auto it = std::find(m_priv->appenderList.begin(), m_priv->appenderList.end(), appender);
148-
if (it != m_priv->appenderList.end())
166+
auto newAppenders = *m_priv->getAppenders();
167+
auto pItem = std::find(newAppenders.begin(), newAppenders.end(), appender);
168+
if (newAppenders.end() != pItem)
149169
{
150-
m_priv->appenderList.erase(it);
170+
newAppenders.erase(pItem);
171+
m_priv->setAppenders(newAppenders);
151172
}
152173
}
153174
}
154175

155176
void AppenderAttachableImpl::removeAppender(const LogString& name)
156177
{
157-
if (m_priv && !name.empty())
178+
if (m_priv)
158179
{
159-
std::lock_guard<std::mutex> lock( m_priv->m_mutex );
160-
auto it = std::find_if(m_priv->appenderList.begin(), m_priv->appenderList.end()
180+
auto newAppenders = *m_priv->getAppenders();
181+
auto pItem = std::find_if(newAppenders.begin(), newAppenders.end()
161182
, [&name](const AppenderPtr& appender) -> bool
162183
{
163184
return name == appender->getName();
164185
});
165-
if (it != m_priv->appenderList.end())
166-
m_priv->appenderList.erase(it);
186+
if (newAppenders.end() != pItem)
187+
{
188+
newAppenders.erase(pItem);
189+
m_priv->setAppenders(newAppenders);
190+
}
167191
}
168192
}
169193

@@ -172,16 +196,17 @@ bool AppenderAttachableImpl::replaceAppender(const AppenderPtr& oldAppender, con
172196
bool found = false;
173197
if (m_priv && oldAppender && newAppender)
174198
{
175-
auto oldName = oldAppender->getName();
176-
std::lock_guard<std::mutex> lock( m_priv->m_mutex );
177-
auto it = std::find_if(m_priv->appenderList.begin(), m_priv->appenderList.end()
178-
, [&oldName](const AppenderPtr& appender) -> bool
199+
auto name = oldAppender->getName();
200+
auto newAppenders = *m_priv->getAppenders();
201+
auto pItem = std::find_if(newAppenders.begin(), newAppenders.end()
202+
, [&name](const AppenderPtr& appender) -> bool
179203
{
180-
return oldName == appender->getName();
204+
return name == appender->getName();
181205
});
182-
if (it != m_priv->appenderList.end())
206+
if (newAppenders.end() != pItem)
183207
{
184-
*it = newAppender;
208+
*pItem = newAppender;
209+
m_priv->setAppenders(newAppenders);
185210
found = true;
186211
}
187212
}
@@ -190,13 +215,15 @@ bool AppenderAttachableImpl::replaceAppender(const AppenderPtr& oldAppender, con
190215

191216
void AppenderAttachableImpl::replaceAppenders(const AppenderList& newList)
192217
{
193-
auto oldAppenders = getAllAppenders();
194-
if (!m_priv)
195-
m_priv = std::make_unique<AppenderAttachableImpl::priv_data>();
196-
std::lock_guard<std::mutex> lock( m_priv->m_mutex );
197-
for (auto a : oldAppenders)
198-
a->close();
199-
m_priv->appenderList = newList;
218+
if (m_priv)
219+
{
220+
auto allAppenders = m_priv->getAppenders();
221+
for (auto& a : *allAppenders)
222+
a->close();
223+
m_priv->setAppenders(newList);
224+
}
225+
else
226+
m_priv = std::make_unique<priv_data>(newList);
200227
}
201228

202229

0 commit comments

Comments
 (0)