From 80b72cfb1cb14cc711990f1276e31be719e6ac7a Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Mon, 18 Dec 2017 09:58:55 +0100 Subject: [PATCH 1/2] Avoid allocations in ScriptUtils::Match --- lib/base/scriptutils.cpp | 134 +++++++++++++++++++++------------------ 1 file changed, 71 insertions(+), 63 deletions(-) diff --git a/lib/base/scriptutils.cpp b/lib/base/scriptutils.cpp index 785b4b942..36caa2bc9 100644 --- a/lib/base/scriptutils.cpp +++ b/lib/base/scriptutils.cpp @@ -110,10 +110,8 @@ bool ScriptUtils::Regex(const std::vector& args) if (args.size() < 2) BOOST_THROW_EXCEPTION(std::invalid_argument("Regular expression and text must be specified.")); - Array::Ptr texts = new Array(); - String pattern = args[0]; - Value argTexts = args[1]; + const Value& argTexts = args[1]; MatchType mode; if (args.size() > 2) @@ -121,34 +119,40 @@ bool ScriptUtils::Regex(const std::vector& args) else mode = MatchAll; - if (argTexts.IsObjectType()) + boost::regex expr(pattern.GetData()); + + Array::Ptr texts; + + if (argTexts.IsObject()) texts = argTexts; - else { - texts = new Array(); - texts->Add(argTexts); - } - if (texts->GetLength() == 0) - return false; + if (texts) { + ObjectLock olock(texts); - ObjectLock olock(texts); - for (const String& text : texts) { - bool res = false; - try { - boost::regex expr(pattern.GetData()); - boost::smatch what; - res = boost::regex_search(text.GetData(), what, expr); - } catch (boost::exception&) { - res = false; /* exception means something went terribly wrong */ + if (texts->GetLength() == 0) + return false; + + for (const String& text : texts) { + bool res = false; + try { + boost::smatch what; + res = boost::regex_search(text.GetData(), what, expr); + } catch (boost::exception&) { + res = false; /* exception means something went terribly wrong */ + } + + if (mode == MatchAny && res) + return true; + else if (mode == MatchAll && !res) + return false; } - if (mode == MatchAny && res) - return true; - else if (mode == MatchAll && !res) - return false; + return true; + } else { + String text = argTexts; + boost::smatch what; + return boost::regex_search(text.GetData(), what, expr); } - - return mode == MatchAll; } bool ScriptUtils::Match(const std::vector& args) @@ -156,10 +160,8 @@ bool ScriptUtils::Match(const std::vector& args) if (args.size() < 2) BOOST_THROW_EXCEPTION(std::invalid_argument("Pattern and text must be specified.")); - Array::Ptr texts = new Array(); - String pattern = args[0]; - Value argTexts = args[1]; + const Value& argTexts = args[1]; MatchType mode; if (args.size() > 2) @@ -167,27 +169,31 @@ bool ScriptUtils::Match(const std::vector& args) else mode = MatchAll; - if (argTexts.IsObjectType()) + Array::Ptr texts; + + if (argTexts.IsObject()) texts = argTexts; - else { - texts = new Array(); - texts->Add(argTexts); - } - if (texts->GetLength() == 0) - return false; + if (texts) { + ObjectLock olock(texts); - ObjectLock olock(texts); - for (const String& text : texts) { - bool res = Utility::Match(pattern, text); - - if (mode == MatchAny && res) - return true; - else if (mode == MatchAll && !res) + if (texts->GetLength() == 0) return false; - } - return mode == MatchAll; + for (const String& text : texts) { + bool res = Utility::Match(pattern, text); + + if (mode == MatchAny && res) + return true; + else if (mode == MatchAll && !res) + return false; + } + + return true; + } else { + String text = argTexts; + return Utility::Match(pattern, argTexts); + } } bool ScriptUtils::CidrMatch(const std::vector& args) @@ -195,10 +201,8 @@ bool ScriptUtils::CidrMatch(const std::vector& args) if (args.size() < 2) BOOST_THROW_EXCEPTION(std::invalid_argument("CIDR and IP address must be specified.")); - Array::Ptr ips = new Array(); - String pattern = args[0]; - Value argIps = args[1]; + const Value& argIps = args[1]; MatchType mode; if (args.size() > 2) @@ -206,27 +210,31 @@ bool ScriptUtils::CidrMatch(const std::vector& args) else mode = MatchAll; - if (argIps.IsObjectType()) + Array::Ptr ips; + + if (argIps.IsObject()) ips = argIps; - else { - ips = new Array(); - ips->Add(argIps); - } - if (ips->GetLength() == 0) - return false; + if (ips) { + ObjectLock olock(ips); - ObjectLock olock(ips); - for (const String& ip : ips) { - bool res = Utility::CidrMatch(pattern, ip); - - if (mode == MatchAny && res) - return true; - else if (mode == MatchAll && !res) + if (ips->GetLength() == 0) return false; - } - return mode == MatchAll; + for (const String& ip : ips) { + bool res = Utility::CidrMatch(pattern, ip); + + if (mode == MatchAny && res) + return true; + else if (mode == MatchAll && !res) + return false; + } + + return true; + } else { + String ip = argIps; + return Utility::CidrMatch(pattern, ip); + } } double ScriptUtils::Len(const Value& value) From 4866f777ccb95ccef9d754fe9e4105b84141c0af Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Mon, 18 Dec 2017 10:30:20 +0100 Subject: [PATCH 2/2] Avoid unnecessary allocations for script frames --- lib/base/function.cpp | 5 ++--- lib/base/scriptframe.cpp | 8 ++++---- lib/base/scriptframe.hpp | 4 ++-- lib/config/vmops.hpp | 2 ++ 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/base/function.cpp b/lib/base/function.cpp index cb34d835f..da2302f6d 100644 --- a/lib/base/function.cpp +++ b/lib/base/function.cpp @@ -38,14 +38,13 @@ Function::Function(const String& name, const Callback& function, const std::vect Value Function::Invoke(const std::vector& arguments) { - ScriptFrame frame; + ScriptFrame frame(false); return m_Callback(arguments); } Value Function::InvokeThis(const Value& otherThis, const std::vector& arguments) { - ScriptFrame frame; - frame.Self = otherThis; + ScriptFrame frame(otherThis, false); return m_Callback(arguments); } diff --git a/lib/base/scriptframe.cpp b/lib/base/scriptframe.cpp index 3d41c4ec8..1576a80b5 100644 --- a/lib/base/scriptframe.cpp +++ b/lib/base/scriptframe.cpp @@ -40,14 +40,14 @@ INITIALIZE_ONCE_WITH_PRIORITY([]() { ScriptFrame::AddImport(deprecatedNS); }, 50); -ScriptFrame::ScriptFrame(void) - : Locals(new Dictionary()), Self(ScriptGlobal::GetGlobals()), Sandboxed(false), Depth(0) +ScriptFrame::ScriptFrame(bool allocLocals) + : Locals(allocLocals ? new Dictionary() : nullptr), Self(ScriptGlobal::GetGlobals()), Sandboxed(false), Depth(0) { InitializeFrame(); } -ScriptFrame::ScriptFrame(const Value& self) - : Locals(new Dictionary()), Self(self), Sandboxed(false), Depth(0) +ScriptFrame::ScriptFrame(const Value& self, bool allocLocals) + : Locals(allocLocals ? new Dictionary() : nullptr), Self(self), Sandboxed(false), Depth(0) { InitializeFrame(); } diff --git a/lib/base/scriptframe.hpp b/lib/base/scriptframe.hpp index 25e71823d..06fdad0fd 100644 --- a/lib/base/scriptframe.hpp +++ b/lib/base/scriptframe.hpp @@ -36,8 +36,8 @@ struct I2_BASE_API ScriptFrame bool Sandboxed; int Depth; - ScriptFrame(void); - ScriptFrame(const Value& self); + ScriptFrame(bool allocLocals = true); + ScriptFrame(const Value& self, bool allocLocals = true); ~ScriptFrame(void); void IncreaseStackDepth(void); diff --git a/lib/config/vmops.hpp b/lib/config/vmops.hpp index 5f0c27eac..97c5d6358 100644 --- a/lib/config/vmops.hpp +++ b/lib/config/vmops.hpp @@ -119,6 +119,8 @@ public: ScriptFrame *frame = ScriptFrame::GetCurrentFrame(); + frame->Locals = new Dictionary(); + if (evaluatedClosedVars) evaluatedClosedVars->CopyTo(frame->Locals);