From 218e780a44325e2c41a9deba6135cea1d9982d54 Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Sat, 20 Aug 2016 23:46:44 +0200 Subject: [PATCH] Avoid unnecessary string copies fixes #12509 --- lib/base/netstring.cpp | 13 +++++++- lib/base/netstring.hpp | 1 + lib/base/string.hpp | 42 +++++++++++++------------- lib/config/configcompilercontext.cpp | 17 ++++++----- lib/config/configcompilercontext.hpp | 6 ++-- lib/icinga/dependency-apply.cpp | 7 +++-- lib/icinga/dependency.cpp | 18 +++-------- lib/icinga/host.cpp | 8 ++--- lib/icinga/notification-apply.cpp | 2 ++ lib/icinga/scheduleddowntime-apply.cpp | 7 +++-- lib/icinga/service-apply.cpp | 7 +++-- lib/icinga/service.cpp | 6 ++-- lib/icinga/timeperiod.cpp | 6 ++++ 13 files changed, 79 insertions(+), 61 deletions(-) diff --git a/lib/base/netstring.cpp b/lib/base/netstring.cpp index 98937b4fb..022d5b0a3 100644 --- a/lib/base/netstring.cpp +++ b/lib/base/netstring.cpp @@ -110,8 +110,19 @@ StreamReadStatus NetString::ReadStringFromStream(const Stream::Ptr& stream, Stri void NetString::WriteStringToStream(const Stream::Ptr& stream, const String& str) { std::ostringstream msgbuf; - msgbuf << str.GetLength() << ":" << str << ","; + WriteStringToStream(msgbuf, str); String msg = msgbuf.str(); stream->Write(msg.CStr(), msg.GetLength()); } + +/** + * Writes data into a stream using the netstring format. + * + * @param stream The stream. + * @param str The String that is to be written. + */ +void NetString::WriteStringToStream(std::ostream& stream, const String& str) +{ + stream << str.GetLength() << ":" << str << ","; +} diff --git a/lib/base/netstring.hpp b/lib/base/netstring.hpp index 9be09f1a2..a15e6a11a 100644 --- a/lib/base/netstring.hpp +++ b/lib/base/netstring.hpp @@ -40,6 +40,7 @@ class I2_BASE_API NetString public: static StreamReadStatus ReadStringFromStream(const Stream::Ptr& stream, String *message, StreamReadContext& context, bool may_wait = false); static void WriteStringToStream(const Stream::Ptr& stream, const String& message); + static void WriteStringToStream(std::ostream& stream, const String& message); private: NetString(void); diff --git a/lib/base/string.hpp b/lib/base/string.hpp index d27cd3c34..0605896e5 100644 --- a/lib/base/string.hpp +++ b/lib/base/string.hpp @@ -327,7 +327,7 @@ private: inline std::ostream& operator<<(std::ostream& stream, const String& str) { - stream << static_cast(str); + stream << str.GetData(); return stream; } @@ -341,102 +341,102 @@ inline std::istream& operator>>(std::istream& stream, String& str) inline String operator+(const String& lhs, const String& rhs) { - return static_cast(lhs)+static_cast(rhs); + return lhs.GetData() + rhs.GetData(); } inline String operator+(const String& lhs, const char *rhs) { - return static_cast(lhs)+rhs; + return lhs.GetData() + rhs; } inline String operator+(const char *lhs, const String& rhs) { - return lhs + static_cast(rhs); + return lhs + rhs.GetData(); } inline bool operator==(const String& lhs, const String& rhs) { - return static_cast(lhs) == static_cast(rhs); + return lhs.GetData() == rhs.GetData(); } inline bool operator==(const String& lhs, const char *rhs) { - return static_cast(lhs) == rhs; + return lhs.GetData() == rhs; } inline bool operator==(const char *lhs, const String& rhs) { - return lhs == static_cast(rhs); + return lhs == rhs.GetData(); } inline bool operator<(const String& lhs, const char *rhs) { - return static_cast(lhs) < rhs; + return lhs.GetData() < rhs; } inline bool operator<(const char *lhs, const String& rhs) { - return lhs < static_cast(rhs); + return lhs < rhs.GetData(); } inline bool operator>(const String& lhs, const String& rhs) { - return static_cast(lhs) > static_cast(rhs); + return lhs.GetData() > rhs.GetData(); } inline bool operator>(const String& lhs, const char *rhs) { - return static_cast(lhs) > rhs; + return lhs.GetData() > rhs; } inline bool operator>(const char *lhs, const String& rhs) { - return lhs > static_cast(rhs); + return lhs > rhs.GetData(); } inline bool operator<=(const String& lhs, const String& rhs) { - return static_cast(lhs) <= static_cast(rhs); + return lhs.GetData() <= rhs.GetData(); } inline bool operator<=(const String& lhs, const char *rhs) { - return static_cast(lhs) <= rhs; + return lhs.GetData() <= rhs; } inline bool operator<=(const char *lhs, const String& rhs) { - return lhs <= static_cast(rhs); + return lhs <= rhs.GetData(); } inline bool operator>=(const String& lhs, const String& rhs) { - return static_cast(lhs) >= static_cast(rhs); + return lhs.GetData() >= rhs.GetData(); } inline bool operator>=(const String& lhs, const char *rhs) { - return static_cast(lhs) >= rhs; + return lhs.GetData() >= rhs; } inline bool operator>=(const char *lhs, const String& rhs) { - return lhs >= static_cast(rhs); + return lhs >= rhs.GetData(); } inline bool operator!=(const String& lhs, const String& rhs) { - return static_cast(lhs) != static_cast(rhs); + return lhs.GetData() != rhs.GetData(); } inline bool operator!=(const String& lhs, const char *rhs) { - return static_cast(lhs) != rhs; + return lhs.GetData() != rhs; } inline bool operator!=(const char *lhs, const String& rhs) { - return lhs != static_cast(rhs); + return lhs != rhs.GetData(); } inline String::Iterator range_begin(String& x) diff --git a/lib/config/configcompilercontext.cpp b/lib/config/configcompilercontext.cpp index c006dcef1..0a66968d7 100644 --- a/lib/config/configcompilercontext.cpp +++ b/lib/config/configcompilercontext.cpp @@ -23,7 +23,6 @@ #include "base/netstring.hpp" #include "base/exception.hpp" #include -#include using namespace icinga; @@ -32,6 +31,10 @@ ConfigCompilerContext *ConfigCompilerContext::GetInstance(void) return Singleton::GetInstance(); } +ConfigCompilerContext::ConfigCompilerContext(void) + : m_ObjectsFP(NULL) +{ } + void ConfigCompilerContext::OpenObjectsFile(const String& filename) { m_ObjectsPath = filename; @@ -42,7 +45,7 @@ void ConfigCompilerContext::OpenObjectsFile(const String& filename) if (!*fp) BOOST_THROW_EXCEPTION(std::runtime_error("Could not open '" + m_ObjectsTempFile + "' file")); - m_ObjectsFP = new StdioStream(fp, true); + m_ObjectsFP = fp; } void ConfigCompilerContext::WriteObject(const Dictionary::Ptr& object) @@ -54,14 +57,14 @@ void ConfigCompilerContext::WriteObject(const Dictionary::Ptr& object) { boost::mutex::scoped_lock lock(m_Mutex); - NetString::WriteStringToStream(m_ObjectsFP, json); + NetString::WriteStringToStream(*m_ObjectsFP, json); } } void ConfigCompilerContext::CancelObjectsFile(void) { - m_ObjectsFP->Close(); - m_ObjectsFP.reset(); + delete m_ObjectsFP; + m_ObjectsFP = NULL; #ifdef _WIN32 _unlink(m_ObjectsTempFile.CStr()); @@ -72,8 +75,8 @@ void ConfigCompilerContext::CancelObjectsFile(void) void ConfigCompilerContext::FinishObjectsFile(void) { - m_ObjectsFP->Close(); - m_ObjectsFP.reset(); + delete m_ObjectsFP; + m_ObjectsFP = NULL; #ifdef _WIN32 _unlink(m_ObjectsPath.CStr()); diff --git a/lib/config/configcompilercontext.hpp b/lib/config/configcompilercontext.hpp index 1e4721112..73886a202 100644 --- a/lib/config/configcompilercontext.hpp +++ b/lib/config/configcompilercontext.hpp @@ -21,9 +21,9 @@ #define CONFIGCOMPILERCONTEXT_H #include "config/i2-config.hpp" -#include "base/stdiostream.hpp" #include "base/dictionary.hpp" #include +#include namespace icinga { @@ -34,6 +34,8 @@ namespace icinga class I2_CONFIG_API ConfigCompilerContext { public: + ConfigCompilerContext(void); + void OpenObjectsFile(const String& filename); void WriteObject(const Dictionary::Ptr& object); void CancelObjectsFile(void); @@ -44,7 +46,7 @@ public: private: String m_ObjectsPath; String m_ObjectsTempFile; - StdioStream::Ptr m_ObjectsFP; + std::fstream *m_ObjectsFP; mutable boost::mutex m_Mutex; }; diff --git a/lib/icinga/dependency-apply.cpp b/lib/icinga/dependency-apply.cpp index e25eaa86f..168aeb336 100644 --- a/lib/icinga/dependency-apply.cpp +++ b/lib/icinga/dependency-apply.cpp @@ -48,8 +48,10 @@ bool Dependency::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, cons DebugInfo di = rule.GetDebugInfo(); +#ifdef _DEBUG Log(LogDebug, "Dependency") << "Applying dependency '" << name << "' to object '" << checkable->GetName() << "' for rule " << di; +#endif /* _DEBUG */ ConfigItemBuilder::Ptr builder = new ConfigItemBuilder(di); builder->SetType("Dependency"); @@ -123,10 +125,9 @@ bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyR BOOST_THROW_EXCEPTION(ScriptError("Dictionary iterator requires value to be a dictionary.", di)); Array::Ptr arr = vinstances; - Array::Ptr arrclone = arr->ShallowClone(); - ObjectLock olock(arrclone); - BOOST_FOREACH(const Value& instance, arrclone) { + ObjectLock olock(arr); + BOOST_FOREACH(const Value& instance, arr) { String name = rule.GetName(); if (!rule.GetFKVar().IsEmpty()) { diff --git a/lib/icinga/dependency.cpp b/lib/icinga/dependency.cpp index 2369a63a9..5d3ea9507 100644 --- a/lib/icinga/dependency.cpp +++ b/lib/icinga/dependency.cpp @@ -87,15 +87,10 @@ void Dependency::OnAllConfigLoaded(void) Host::Ptr childHost = Host::GetByName(GetChildHostName()); if (childHost) { - if (GetChildServiceName().IsEmpty()) { - Log(LogDebug, "Dependency") - << "Dependency '" << GetName() << "' child host '" << GetChildHostName() << "."; + if (GetChildServiceName().IsEmpty()) m_Child = childHost; - } else { - Log(LogDebug, "Dependency") - << "Dependency '" << GetName() << "' child host '" << GetChildHostName() << "' service '" << GetChildServiceName() << "' ."; + else m_Child = childHost->GetServiceByShortName(GetChildServiceName()); - } } if (!m_Child) @@ -106,15 +101,10 @@ void Dependency::OnAllConfigLoaded(void) Host::Ptr parentHost = Host::GetByName(GetParentHostName()); if (parentHost) { - if (GetParentServiceName().IsEmpty()) { - Log(LogDebug, "Dependency") - << "Dependency '" << GetName() << "' parent host '" << GetParentHostName() << "."; + if (GetParentServiceName().IsEmpty()) m_Parent = parentHost; - } else { - Log(LogDebug, "Dependency") - << "Dependency '" << GetName() << "' parent host '" << GetParentHostName() << "' service '" << GetParentServiceName() << "' ."; + else m_Parent = parentHost->GetServiceByShortName(GetParentServiceName()); - } } if (!m_Parent) diff --git a/lib/icinga/host.cpp b/lib/icinga/host.cpp index f3c907fcd..d9d971659 100644 --- a/lib/icinga/host.cpp +++ b/lib/icinga/host.cpp @@ -67,16 +67,16 @@ void Host::OnAllConfigLoaded(void) void Host::CreateChildObjects(const Type::Ptr& childType) { - if (childType->GetName() == "ScheduledDowntime") + if (childType == ScheduledDowntime::TypeInstance) ScheduledDowntime::EvaluateApplyRules(this); - if (childType->GetName() == "Notification") + if (childType == Notification::TypeInstance) Notification::EvaluateApplyRules(this); - if (childType->GetName() == "Dependency") + if (childType == Dependency::TypeInstance) Dependency::EvaluateApplyRules(this); - if (childType->GetName() == "Service") + if (childType == Service::TypeInstance) Service::EvaluateApplyRules(this); } diff --git a/lib/icinga/notification-apply.cpp b/lib/icinga/notification-apply.cpp index edc4c2b57..a64e2554b 100644 --- a/lib/icinga/notification-apply.cpp +++ b/lib/icinga/notification-apply.cpp @@ -48,8 +48,10 @@ bool Notification::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, co DebugInfo di = rule.GetDebugInfo(); +#ifdef _DEBUG Log(LogDebug, "Notification") << "Applying notification '" << name << "' to object '" << checkable->GetName() << "' for rule " << di; +#endif /* _DEBUG */ ConfigItemBuilder::Ptr builder = new ConfigItemBuilder(di); builder->SetType("Notification"); diff --git a/lib/icinga/scheduleddowntime-apply.cpp b/lib/icinga/scheduleddowntime-apply.cpp index 6eff9fad1..c61e9dc51 100644 --- a/lib/icinga/scheduleddowntime-apply.cpp +++ b/lib/icinga/scheduleddowntime-apply.cpp @@ -47,8 +47,10 @@ bool ScheduledDowntime::EvaluateApplyRuleInstance(const Checkable::Ptr& checkabl DebugInfo di = rule.GetDebugInfo(); +#ifdef _DEBUG Log(LogDebug, "ScheduledDowntime") << "Applying scheduled downtime '" << rule.GetName() << "' to object '" << checkable->GetName() << "' for rule " << di; +#endif /* _DEBUG */ ConfigItemBuilder::Ptr builder = new ConfigItemBuilder(di); builder->SetType("ScheduledDowntime"); @@ -121,10 +123,9 @@ bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const BOOST_THROW_EXCEPTION(ScriptError("Dictionary iterator requires value to be a dictionary.", di)); Array::Ptr arr = vinstances; - Array::Ptr arrclone = arr->ShallowClone(); - ObjectLock olock(arrclone); - BOOST_FOREACH(const Value& instance, arrclone) { + ObjectLock olock(arr); + BOOST_FOREACH(const Value& instance, arr) { String name = rule.GetName(); if (!rule.GetFKVar().IsEmpty()) { diff --git a/lib/icinga/service-apply.cpp b/lib/icinga/service-apply.cpp index 24a424781..6fa495664 100644 --- a/lib/icinga/service-apply.cpp +++ b/lib/icinga/service-apply.cpp @@ -46,8 +46,10 @@ bool Service::EvaluateApplyRuleInstance(const Host::Ptr& host, const String& nam DebugInfo di = rule.GetDebugInfo(); +#ifdef _DEBUG Log(LogDebug, "Service") << "Applying service '" << name << "' to host '" << host->GetName() << "' for rule " << di; +#endif /* _DEBUG */ ConfigItemBuilder::Ptr builder = new ConfigItemBuilder(di); builder->SetType("Service"); @@ -109,10 +111,9 @@ bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule) BOOST_THROW_EXCEPTION(ScriptError("Dictionary iterator requires value to be a dictionary.", di)); Array::Ptr arr = vinstances; - Array::Ptr arrclone = arr->ShallowClone(); - ObjectLock olock(arrclone); - BOOST_FOREACH(const Value& instance, arrclone) { + ObjectLock olock(arr); + BOOST_FOREACH(const Value& instance, arr) { String name = rule.GetName(); if (!rule.GetFKVar().IsEmpty()) { diff --git a/lib/icinga/service.cpp b/lib/icinga/service.cpp index 1cb904f17..dfd94c8f7 100644 --- a/lib/icinga/service.cpp +++ b/lib/icinga/service.cpp @@ -97,13 +97,13 @@ void Service::OnAllConfigLoaded(void) void Service::CreateChildObjects(const Type::Ptr& childType) { - if (childType->GetName() == "ScheduledDowntime") + if (childType == ScheduledDowntime::TypeInstance) ScheduledDowntime::EvaluateApplyRules(this); - if (childType->GetName() == "Notification") + if (childType == Notification::TypeInstance) Notification::EvaluateApplyRules(this); - if (childType->GetName() == "Dependency") + if (childType == Dependency::TypeInstance) Dependency::EvaluateApplyRules(this); } diff --git a/lib/icinga/timeperiod.cpp b/lib/icinga/timeperiod.cpp index 9899f6d34..43b8f3447 100644 --- a/lib/icinga/timeperiod.cpp +++ b/lib/icinga/timeperiod.cpp @@ -51,7 +51,9 @@ void TimePeriod::Start(bool runtimeCreated) /* Pre-fill the time period for the next 24 hours. */ double now = Utility::GetTime(); UpdateRegion(now, now + 24 * 3600, true); +#ifdef _DEBUG Dump(); +#endif /* _DEBUG */ } void TimePeriod::AddSegment(double begin, double end) @@ -176,7 +178,9 @@ void TimePeriod::RemoveSegment(double begin, double end) SetSegments(newSegments); +#ifdef _DEBUG Dump(); +#endif /* _DEBUG */ } void TimePeriod::RemoveSegment(const Dictionary::Ptr& segment) @@ -351,7 +355,9 @@ void TimePeriod::UpdateTimerHandler(void) } tp->UpdateRegion(valid_end, now + 24 * 3600, false); +#ifdef _DEBUG tp->Dump(); +#endif /* _DEBUG */ } }