From 3008d5ef31e6dafb9fd87f09ffb29ff961a5464b Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Mon, 18 Apr 2016 11:29:43 +0200 Subject: [PATCH] Improve performance for field accesses fixes #11612 --- lib/base/array.cpp | 30 ++++++++++ lib/base/array.hpp | 3 + lib/base/dictionary.cpp | 20 +++++++ lib/base/dictionary.hpp | 4 ++ lib/base/object.cpp | 79 ++++++++++++++++++++++++ lib/base/object.hpp | 6 ++ lib/config/expression.cpp | 4 +- lib/config/vmops.hpp | 123 +------------------------------------- 8 files changed, 147 insertions(+), 122 deletions(-) diff --git a/lib/base/array.cpp b/lib/base/array.cpp index 86eeeece1..3152834b4 100644 --- a/lib/base/array.cpp +++ b/lib/base/array.cpp @@ -23,6 +23,8 @@ #include "base/primitivetype.hpp" #include "base/dictionary.hpp" #include "base/configwriter.hpp" +#include "base/convert.hpp" +#include "base/exception.hpp" #include using namespace icinga; @@ -208,3 +210,31 @@ String Array::ToString(void) const ConfigWriter::EmitArray(msgbuf, 1, const_cast(this)); return msgbuf.str(); } + +Value Array::GetFieldByName(const String& field, bool sandboxed, const DebugInfo& debugInfo) const +{ + int index; + + try { + index = Convert::ToLong(field); + } catch (...) { + return Object::GetFieldByName(field, sandboxed, debugInfo); + } + + ObjectLock olock(this); + + if (index < 0 || index >= GetLength()) + BOOST_THROW_EXCEPTION(ScriptError("Array index '" + Convert::ToString(index) + "' is out of bounds.", debugInfo)); + + return Get(index); +} + +void Array::SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo) +{ + ObjectLock olock(this); + + int index = Convert::ToLong(field); + if (index >= GetLength()) + Resize(index + 1); + Set(index, value); +} diff --git a/lib/base/array.hpp b/lib/base/array.hpp index b7642cfae..5981ab01e 100644 --- a/lib/base/array.hpp +++ b/lib/base/array.hpp @@ -124,6 +124,9 @@ public: virtual String ToString(void) const override; + virtual Value GetFieldByName(const String& field, bool sandboxed, const DebugInfo& debugInfo) const override; + virtual void SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo) override; + private: std::vector m_Data; /**< The data for the array. */ }; diff --git a/lib/base/dictionary.cpp b/lib/base/dictionary.cpp index 99bdee6c4..6ac2fecae 100644 --- a/lib/base/dictionary.cpp +++ b/lib/base/dictionary.cpp @@ -197,3 +197,23 @@ String Dictionary::ToString(void) const ConfigWriter::EmitScope(msgbuf, 1, const_cast(this)); return msgbuf.str(); } + +Value Dictionary::GetFieldByName(const String& field, bool sandboxed, const DebugInfo& debugInfo) const +{ + Value value; + + if (Get(field, &value)) + return value; + else + return GetPrototypeField(const_cast(this), field, false, debugInfo); +} + +void Dictionary::SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo) +{ + Set(field, value); +} + +bool Dictionary::HasOwnField(const String& field) const +{ + return Contains(field); +} diff --git a/lib/base/dictionary.hpp b/lib/base/dictionary.hpp index 487fd4502..38007dfe8 100644 --- a/lib/base/dictionary.hpp +++ b/lib/base/dictionary.hpp @@ -117,6 +117,10 @@ public: virtual String ToString(void) const override; + virtual Value GetFieldByName(const String& field, bool sandboxed, const DebugInfo& debugInfo) const override; + virtual void SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo) override; + virtual bool HasOwnField(const String& field) const override; + private: std::map m_Data; /**< The data for the dictionary. */ }; diff --git a/lib/base/object.cpp b/lib/base/object.cpp index cc53b8a25..a98ec4c5b 100644 --- a/lib/base/object.cpp +++ b/lib/base/object.cpp @@ -24,7 +24,9 @@ #include "base/utility.hpp" #include "base/timer.hpp" #include "base/logger.hpp" +#include "base/exception.hpp" #include +#include using namespace icinga; @@ -98,6 +100,63 @@ Value Object::GetField(int id) const BOOST_THROW_EXCEPTION(std::runtime_error("Invalid field ID.")); } +bool Object::HasOwnField(const String& field) const +{ + Type::Ptr type = GetReflectionType(); + + if (!type) + return false; + + return type->GetFieldId(field) != -1; +} + +Value Object::GetFieldByName(const String& field, bool sandboxed, const DebugInfo& debugInfo) const +{ + Type::Ptr type = GetReflectionType(); + + if (!type) + return Empty; + + int fid = type->GetFieldId(field); + + if (fid == -1) + return GetPrototypeField(const_cast(this), field, true, debugInfo); + + if (sandboxed) { + Field fieldInfo = type->GetFieldInfo(fid); + + if (fieldInfo.Attributes & FANoUserView) + BOOST_THROW_EXCEPTION(ScriptError("Accessing the field '" + field + "' for type '" + type->GetName() + "' is not allowed in sandbox mode.", debugInfo)); + } + + return GetField(fid); +} + +void Object::SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo) +{ + Type::Ptr type = GetReflectionType(); + + if (!type) + BOOST_THROW_EXCEPTION(ScriptError("Cannot set field on object.", debugInfo)); + + int fid = type->GetFieldId(field); + + if (fid == -1) + BOOST_THROW_EXCEPTION(ScriptError("Attribute '" + field + "' does not exist.", debugInfo)); + + try { + SetField(fid, value); + } catch (const boost::bad_lexical_cast&) { + Field fieldInfo = type->GetFieldInfo(fid); + Type::Ptr ftype = Type::GetByName(fieldInfo.TypeName); + BOOST_THROW_EXCEPTION(ScriptError("Attribute '" + field + "' cannot be set to value of type '" + value.GetTypeName() + "', expected '" + ftype->GetName() + "'", debugInfo)); + } catch (const std::bad_cast&) { + Field fieldInfo = type->GetFieldInfo(fid); + Type::Ptr ftype = Type::GetByName(fieldInfo.TypeName); + BOOST_THROW_EXCEPTION(ScriptError("Attribute '" + field + "' cannot be set to value of type '" + value.GetTypeName() + "', expected '" + ftype->GetName() + "'", debugInfo)); + } +} + void Object::Validate(int types, const ValidationUtils& utils) { /* Nothing to do here. */ @@ -128,6 +187,26 @@ Type::Ptr Object::GetReflectionType(void) const return Object::TypeInstance; } +Value icinga::GetPrototypeField(const Value& context, const String& field, bool not_found_error, const DebugInfo& debugInfo) +{ + Type::Ptr ctype = context.GetReflectionType(); + Type::Ptr type = ctype; + + do { + Object::Ptr object = type->GetPrototype(); + + if (object && object->HasOwnField(field)) + return object->GetFieldByName(field, false, debugInfo); + + type = type->GetBaseType(); + } while (type); + + if (not_found_error) + BOOST_THROW_EXCEPTION(ScriptError("Invalid field access (for value of type '" + ctype->GetName() + "'): '" + field + "'", debugInfo)); + else + return Empty; +} + #ifdef I2_LEAK_DEBUG void icinga::TypeAddObject(Object *object) { diff --git a/lib/base/object.hpp b/lib/base/object.hpp index 2cb77263d..654e56950 100644 --- a/lib/base/object.hpp +++ b/lib/base/object.hpp @@ -41,6 +41,7 @@ class Value; class Object; class Type; class String; +class DebugInfo; class ValidationUtils; extern I2_BASE_API Value Empty; @@ -123,6 +124,9 @@ public: virtual void SetField(int id, const Value& value, bool suppress_events = false, const Value& cookie = Empty); virtual Value GetField(int id) const; + virtual Value GetFieldByName(const String& field, bool sandboxed, const DebugInfo& debugInfo) const; + virtual void SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo); + virtual bool HasOwnField(const String& field) const; virtual void ValidateField(int id, const Value& value, const ValidationUtils& utils); virtual void NotifyField(int id, const Value& cookie = Empty); virtual Object::Ptr NavigateField(int id) const; @@ -158,6 +162,8 @@ private: friend void intrusive_ptr_release(Object *object); }; +I2_BASE_API Value GetPrototypeField(const Value& context, const String& field, bool not_found_error, const DebugInfo& debugInfo); + void TypeAddObject(Object *object); void TypeRemoveObject(Object *object); diff --git a/lib/config/expression.cpp b/lib/config/expression.cpp index f2ef686f5..bd87767e3 100644 --- a/lib/config/expression.cpp +++ b/lib/config/expression.cpp @@ -122,7 +122,7 @@ ExpressionResult VariableExpression::DoEvaluate(ScriptFrame& frame, DebugHint *d if (frame.Locals && frame.Locals->Get(m_Variable, &value)) return value; - else if (frame.Self.IsObject() && frame.Locals != static_cast(frame.Self) && VMOps::HasField(frame.Self, m_Variable)) + else if (frame.Self.IsObject() && frame.Locals != static_cast(frame.Self) && static_cast(frame.Self)->HasOwnField(m_Variable)) return VMOps::GetField(frame.Self, m_Variable, frame.Sandboxed, m_DebugInfo); else return ScriptGlobal::Get(m_Variable); @@ -137,7 +137,7 @@ bool VariableExpression::GetReference(ScriptFrame& frame, bool init_dict, Value if (dhint) *dhint = NULL; - } else if (frame.Self.IsObject() && frame.Locals != static_cast(frame.Self) && VMOps::HasField(frame.Self, m_Variable)) { + } else if (frame.Self.IsObject() && frame.Locals != static_cast(frame.Self) && static_cast(frame.Self)->HasOwnField(m_Variable)) { *parent = frame.Self; if (dhint && *dhint) diff --git a/lib/config/vmops.hpp b/lib/config/vmops.hpp index d0c0f9b6d..c330b6cd8 100644 --- a/lib/config/vmops.hpp +++ b/lib/config/vmops.hpp @@ -49,7 +49,7 @@ public: Value value; if (frame.Locals && frame.Locals->Get(name, &value)) return value; - else if (frame.Self.IsObject() && frame.Locals != static_cast(frame.Self) && HasField(frame.Self, name)) + else if (frame.Self.IsObject() && frame.Locals != static_cast(frame.Self) && static_cast(frame.Self)->HasOwnField(name)) return GetField(frame.Self, name, frame.Sandboxed, debugInfo); else return ScriptGlobal::Get(name); @@ -191,42 +191,6 @@ public: return Empty; } - static inline bool HasField(const Object::Ptr& context, const String& field) - { - Dictionary::Ptr dict = dynamic_pointer_cast(context); - - if (dict) - return dict->Contains(field); - else { - Type::Ptr type = context->GetReflectionType(); - - if (!type) - return false; - - return type->GetFieldId(field) != -1; - } - } - - static inline Value GetPrototypeField(const Value& context, const String& field, bool not_found_error = true, const DebugInfo& debugInfo = DebugInfo()) - { - Type::Ptr ctype = context.GetReflectionType(); - Type::Ptr type = ctype; - - do { - Object::Ptr object = type->GetPrototype(); - - if (object && HasField(object, field)) - return GetField(object, field, false, debugInfo); - - type = type->GetBaseType(); - } while (type); - - if (not_found_error) - BOOST_THROW_EXCEPTION(ScriptError("Invalid field access (for value of type '" + ctype->GetName() + "'): '" + field + "'", debugInfo)); - else - return Empty; - } - static inline Value GetField(const Value& context, const String& field, bool sandboxed = false, const DebugInfo& debugInfo = DebugInfo()) { if (context.IsEmpty() && !context.IsString()) @@ -237,51 +201,7 @@ public: Object::Ptr object = context; - Dictionary::Ptr dict = dynamic_pointer_cast(object); - - if (dict) { - Value value; - if (dict->Get(field, &value)) - return value; - else - return GetPrototypeField(context, field, false, debugInfo); - } - - Array::Ptr arr = dynamic_pointer_cast(object); - - if (arr) { - int index; - - try { - index = Convert::ToLong(field); - } catch (...) { - return GetPrototypeField(context, field, true, debugInfo); - } - - if (index < 0 || index >= arr->GetLength()) - BOOST_THROW_EXCEPTION(ScriptError("Array index '" + Convert::ToString(index) + "' is out of bounds.", debugInfo)); - - return arr->Get(index); - } - - Type::Ptr type = object->GetReflectionType(); - - if (!type) - return Empty; - - int fid = type->GetFieldId(field); - - if (fid == -1) - return GetPrototypeField(context, field, true, debugInfo); - - if (sandboxed) { - Field fieldInfo = type->GetFieldInfo(fid); - - if (fieldInfo.Attributes & FANoUserView) - BOOST_THROW_EXCEPTION(ScriptError("Accessing the field '" + field + "' for type '" + type->GetName() + "' is not allowed in sandbox mode.")); - } - - return object->GetField(fid); + return object->GetFieldByName(field, sandboxed, debugInfo); } static inline void SetField(const Object::Ptr& context, const String& field, const Value& value, const DebugInfo& debugInfo = DebugInfo()) @@ -289,44 +209,7 @@ public: if (!context) BOOST_THROW_EXCEPTION(ScriptError("Cannot set field '" + field + "' on a value that is not an object.", debugInfo)); - Dictionary::Ptr dict = dynamic_pointer_cast(context); - - if (dict) { - dict->Set(field, value); - return; - } - - Array::Ptr arr = dynamic_pointer_cast(context); - - if (arr) { - int index = Convert::ToLong(field); - if (index >= arr->GetLength()) - arr->Resize(index + 1); - arr->Set(index, value); - return; - } - - Type::Ptr type = context->GetReflectionType(); - - if (!type) - BOOST_THROW_EXCEPTION(ScriptError("Cannot set field on object.", debugInfo)); - - int fid = type->GetFieldId(field); - - if (fid == -1) - BOOST_THROW_EXCEPTION(ScriptError("Attribute '" + field + "' does not exist.", debugInfo)); - - try { - context->SetField(fid, value); - } catch (const boost::bad_lexical_cast&) { - Field fieldInfo = type->GetFieldInfo(fid); - Type::Ptr ftype = Type::GetByName(fieldInfo.TypeName); - BOOST_THROW_EXCEPTION(ScriptError("Attribute '" + field + "' cannot be set to value of type '" + value.GetTypeName() + "', expected '" + ftype->GetName() + "'", debugInfo)); - } catch (const std::bad_cast&) { - Field fieldInfo = type->GetFieldInfo(fid); - Type::Ptr ftype = Type::GetByName(fieldInfo.TypeName); - BOOST_THROW_EXCEPTION(ScriptError("Attribute '" + field + "' cannot be set to value of type '" + value.GetTypeName() + "', expected '" + ftype->GetName() + "'", debugInfo)); - } + return context->SetFieldByName(field, value, debugInfo); } private: -- 2.40.0