Namespace: don't acquire shared locks on frozen namespaces

This makes freezing a namespace an irrevocable operation but in return allows
omitting further lock operations. This results in a performance improvement as
reading an atomic bool is faster than acquiring and releasing a shared lock.

ObjectLocks on namespaces remain untouched as these mostly affect write
operations which there should be none of after freezing (if there are some,
they will throw exceptions anyways).
This commit is contained in:
Julian Brost 2023-01-09 16:12:59 +01:00
parent cc0e2ec181
commit 24b57f0d3a
7 changed files with 54 additions and 39 deletions

View File

@ -60,28 +60,28 @@ private:
INITIALIZE_ONCE_WITH_PRIORITY([]() { \
Function::Ptr sf = new icinga::Function(#ns "#" #name, callback, String(args).Split(":"), false); \
Namespace::Ptr nsp = ScriptGlobal::Get(#ns); \
nsp->Set(#name, sf, true, true); \
nsp->Set(#name, sf, true); \
}, InitializePriority::RegisterFunctions)
#define REGISTER_SAFE_FUNCTION(ns, name, callback, args) \
INITIALIZE_ONCE_WITH_PRIORITY([]() { \
Function::Ptr sf = new icinga::Function(#ns "#" #name, callback, String(args).Split(":"), true); \
Namespace::Ptr nsp = ScriptGlobal::Get(#ns); \
nsp->Set(#name, sf, true, true); \
nsp->Set(#name, sf, true); \
}, InitializePriority::RegisterFunctions)
#define REGISTER_FUNCTION_NONCONST(ns, name, callback, args) \
INITIALIZE_ONCE_WITH_PRIORITY([]() { \
Function::Ptr sf = new icinga::Function(#ns "#" #name, callback, String(args).Split(":"), false); \
Namespace::Ptr nsp = ScriptGlobal::Get(#ns); \
nsp->Set(#name, sf, false, true); \
nsp->Set(#name, sf, false); \
}, InitializePriority::RegisterFunctions)
#define REGISTER_SAFE_FUNCTION_NONCONST(ns, name, callback, args) \
INITIALIZE_ONCE_WITH_PRIORITY([]() { \
Function::Ptr sf = new icinga::Function(#ns "#" #name, callback, String(args).Split(":"), true); \
Namespace::Ptr nsp = ScriptGlobal::Get(#ns); \
nsp->SetAttribute(#name, sf, false, true); \
nsp->SetAttribute(#name, sf, false); \
}, InitializePriority::RegisterFunctions)
}

View File

@ -24,5 +24,5 @@ INITIALIZE_ONCE([]() {
jsonNS->Freeze();
Namespace::Ptr systemNS = ScriptGlobal::Get("System");
systemNS->Set("Json", jsonNS, true, true);
systemNS->Set("Json", jsonNS, true);
});

View File

@ -180,5 +180,5 @@ INITIALIZE_ONCE([]() {
mathNS->Freeze();
Namespace::Ptr systemNS = ScriptGlobal::Get("System");
systemNS->Set("Math", mathNS, true, true);
systemNS->Set("Math", mathNS, true);
});

View File

@ -33,7 +33,7 @@ Value Namespace::Get(const String& field) const
bool Namespace::Get(const String& field, Value *value) const
{
std::shared_lock<decltype(m_DataMutex)> lock(m_DataMutex);
auto lock(ReadLockUnlessFrozen());
auto nsVal = m_Data.find(field);
@ -45,21 +45,22 @@ bool Namespace::Get(const String& field, Value *value) const
return true;
}
void Namespace::Set(const String& field, const Value& value, bool isConst, bool overrideFrozen, const DebugInfo& debugInfo)
void Namespace::Set(const String& field, const Value& value, bool isConst, const DebugInfo& debugInfo)
{
ObjectLock olock(this);
if (m_Frozen) {
BOOST_THROW_EXCEPTION(ScriptError("Namespace is read-only and must not be modified.", debugInfo));
}
std::unique_lock<decltype(m_DataMutex)> dlock (m_DataMutex);
auto nsVal = m_Data.find(field);
if (nsVal == m_Data.end()) {
if (m_Frozen && !overrideFrozen) {
BOOST_THROW_EXCEPTION(ScriptError("Namespace is read-only and must not be modified.", debugInfo));
}
m_Data[field] = NamespaceValue{value, isConst};
} else {
if (nsVal->second.Const && !overrideFrozen) {
if (nsVal->second.Const) {
BOOST_THROW_EXCEPTION(ScriptError("Constant must not be modified.", debugInfo));
}
@ -74,46 +75,43 @@ void Namespace::Set(const String& field, const Value& value, bool isConst, bool
*/
size_t Namespace::GetLength() const
{
std::shared_lock<decltype(m_DataMutex)> lock(m_DataMutex);
auto lock(ReadLockUnlessFrozen());
return m_Data.size();
}
bool Namespace::Contains(const String& field) const
{
std::shared_lock<decltype(m_DataMutex)> lock(m_DataMutex);
auto lock (ReadLockUnlessFrozen());
return m_Data.find(field) != m_Data.end();
}
void Namespace::Remove(const String& field, bool overrideFrozen)
void Namespace::Remove(const String& field)
{
ObjectLock olock(this);
if (m_Frozen && !overrideFrozen) {
if (m_Frozen) {
BOOST_THROW_EXCEPTION(ScriptError("Namespace is read-only and must not be modified."));
}
std::unique_lock<decltype(m_DataMutex)> dlock (m_DataMutex);
if (!overrideFrozen) {
auto attr = m_Data.find(field);
if (attr != m_Data.end() && attr->second.Const) {
BOOST_THROW_EXCEPTION(ScriptError("Constants must not be removed."));
}
}
auto it = m_Data.find(field);
if (it == m_Data.end())
if (it == m_Data.end()) {
return;
}
if (it->second.Const) {
BOOST_THROW_EXCEPTION(ScriptError("Constants must not be removed."));
}
m_Data.erase(it);
}
/**
* Freeze the namespace, preventing further updates unless overrideFrozen is set.
* Freeze the namespace, preventing further updates.
*
* This only prevents inserting, replacing or deleting values from the namespace. This operation has no effect on
* objects referenced by the values, these remain mutable if they were before.
@ -124,9 +122,18 @@ void Namespace::Freeze() {
m_Frozen = true;
}
std::shared_lock<std::shared_timed_mutex> Namespace::ReadLockUnlessFrozen() const
{
if (m_Frozen.load(std::memory_order_relaxed)) {
return std::shared_lock<std::shared_timed_mutex>();
} else {
return std::shared_lock<std::shared_timed_mutex>(m_DataMutex);
}
}
Value Namespace::GetFieldByName(const String& field, bool, const DebugInfo& debugInfo) const
{
std::shared_lock<decltype(m_DataMutex)> lock(m_DataMutex);
auto lock (ReadLockUnlessFrozen());
auto nsVal = m_Data.find(field);
@ -138,7 +145,12 @@ Value Namespace::GetFieldByName(const String& field, bool, const DebugInfo& debu
void Namespace::SetFieldByName(const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo)
{
Set(field, value, false, overrideFrozen, debugInfo);
// The override frozen parameter is mandated by the interface but ignored here. If the namespace is frozen, this
// disables locking for read operations, so it must not be modified again to ensure the consistency of the internal
// data structures.
(void) overrideFrozen;
Set(field, value, false, debugInfo);
}
bool Namespace::HasOwnField(const String& field) const

View File

@ -8,6 +8,7 @@
#include "base/shared-object.hpp"
#include "base/value.hpp"
#include "base/debuginfo.hpp"
#include <atomic>
#include <map>
#include <vector>
#include <memory>
@ -68,9 +69,9 @@ public:
Value Get(const String& field) const;
bool Get(const String& field, Value *value) const;
void Set(const String& field, const Value& value, bool isConst = false, bool overrideFrozen = false, const DebugInfo& debugInfo = DebugInfo());
void Set(const String& field, const Value& value, bool isConst = false, const DebugInfo& debugInfo = DebugInfo());
bool Contains(const String& field) const;
void Remove(const String& field, bool overrideFrozen = false);
void Remove(const String& field);
void Freeze();
Iterator Begin();
@ -86,10 +87,12 @@ public:
static Object::Ptr GetPrototype();
private:
std::shared_lock<std::shared_timed_mutex> ReadLockUnlessFrozen() const;
std::map<String, NamespaceValue> m_Data;
mutable std::shared_timed_mutex m_DataMutex;
bool m_ConstValues;
bool m_Frozen;
std::atomic<bool> m_Frozen;
};
Namespace::Iterator begin(const Namespace::Ptr& x);

View File

@ -29,18 +29,18 @@ INITIALIZE_ONCE_WITH_PRIORITY([]() {
l_SystemNS->Set("BuildHostName", ICINGA_BUILD_HOST_NAME);
l_SystemNS->Set("BuildCompilerName", ICINGA_BUILD_COMPILER_NAME);
l_SystemNS->Set("BuildCompilerVersion", ICINGA_BUILD_COMPILER_VERSION);
globalNS->Set("System", l_SystemNS, false, true);
globalNS->Set("System", l_SystemNS, true);
l_SystemNS->Set("Configuration", new Configuration(), false, true);
l_SystemNS->Set("Configuration", new Configuration());
l_TypesNS = new Namespace(true);
globalNS->Set("Types", l_TypesNS, false, true);
globalNS->Set("Types", l_TypesNS, true);
l_StatsNS = new Namespace(true);
globalNS->Set("StatsFunctions", l_StatsNS, false, true);
globalNS->Set("StatsFunctions", l_StatsNS, true);
l_InternalNS = new Namespace(true);
globalNS->Set("Internal", l_InternalNS, false, true);
globalNS->Set("Internal", l_InternalNS, true);
}, InitializePriority::CreateNamespaces);
INITIALIZE_ONCE_WITH_PRIORITY([]() {

View File

@ -54,8 +54,8 @@ void IcingaApplication::StaticInitialize()
/* Ensure that the System namespace is already initialized. Otherwise this is a programming error. */
VERIFY(systemNS);
systemNS->Set("ApplicationType", "IcingaApplication", false, true);
systemNS->Set("ApplicationVersion", Application::GetAppVersion(), false, true);
systemNS->Set("ApplicationType", "IcingaApplication", true);
systemNS->Set("ApplicationVersion", Application::GetAppVersion(), true);
Namespace::Ptr globalNS = ScriptGlobal::GetGlobals();
VERIFY(globalNS);