diff --git a/src/ID.cc b/src/ID.cc index 582c8fcf8e..894de949a5 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -66,7 +66,7 @@ void ID::SetVal(Val* v, bool arg_weak_ref) val = v; weak_ref = arg_weak_ref; - notifiers.Modified(this); + Modified(); #ifdef DEBUG UpdateValID(); diff --git a/src/ID.h b/src/ID.h index 7717a584f3..a1cbe494d3 100644 --- a/src/ID.h +++ b/src/ID.h @@ -15,7 +15,7 @@ class Func; typedef enum { INIT_NONE, INIT_FULL, INIT_EXTRA, INIT_REMOVE, } init_class; typedef enum { SCOPE_FUNCTION, SCOPE_MODULE, SCOPE_GLOBAL } IDScope; -class ID : public BroObj { +class ID : public BroObj, public notifier::Modifiable { public: ID(const char* name, IDScope arg_scope, bool arg_is_export); ~ID() override; diff --git a/src/StateAccess.cc b/src/StateAccess.cc index 853a8a7879..7d7c397981 100644 --- a/src/StateAccess.cc +++ b/src/StateAccess.cc @@ -4,110 +4,57 @@ #include "NetVar.h" #include "DebugLogger.h" -NotifierRegistry notifiers; +notifier::Registry notifier::registry; -NotifierRegistry::~NotifierRegistry() +notifier::Registry::~Registry() { - for ( auto i : ids ) - Unref(i.first); - - for ( auto i : vals ) - Unref(i.first); + for ( auto i : registrations ) + Unregister(i.first); } -void NotifierRegistry::Register(ID* id, NotifierRegistry::Notifier* notifier) +void notifier::Registry::Register(Modifiable* m, notifier::Notifier* notifier) { - DBG_LOG(DBG_NOTIFIERS, "registering ID %s for notifier %s", - id->Name(), notifier->Name()); + DBG_LOG(DBG_NOTIFIERS, "registering modifiable %p for notifier %s", + m, notifier->Name()); - Attr* attr = new Attr(ATTR_TRACKED); - - if ( id->Attrs() ) - { - if ( ! id->Attrs()->FindAttr(ATTR_TRACKED) ) - id->Attrs()->AddAttr(attr); - } - else - { - attr_list* a = new attr_list{attr}; - id->SetAttrs(new Attributes(a, id->Type(), false)); - } - - Unref(attr); - - ids.insert({id, notifier}); - Ref(id); + registrations.insert({m, notifier}); + ++m->notifiers; } -void NotifierRegistry::Register(Val* val, NotifierRegistry::Notifier* notifier) +void notifier::Registry::Unregister(Modifiable* m, notifier::Notifier* notifier) { - if ( ! val->IsMutableVal() ) - return; + DBG_LOG(DBG_NOTIFIERS, "unregistering modifiable %p from notifier %s", + m, notifier->Name()); - DBG_LOG(DBG_NOTIFIERS, "registering value %p for notifier %s", - val, notifier->Name()); - - vals.insert({val, notifier}); - Ref(val); - } - -void NotifierRegistry::Unregister(ID* id, NotifierRegistry::Notifier* notifier) - { - DBG_LOG(DBG_NOTIFIERS, "unregistering ID %s for notifier %s", - id->Name(), notifier->Name()); - - auto x = ids.equal_range(id); + auto x = registrations.equal_range(m); for ( auto i = x.first; i != x.second; i++ ) { if ( i->second == notifier ) { - ids.erase(i); - Unref(id); - break; - } - } - - if ( ids.find(id) == ids.end() ) - // Last registered notifier for this ID - id->Attrs()->RemoveAttr(ATTR_TRACKED); - } - -void NotifierRegistry::Unregister(Val* val, NotifierRegistry::Notifier* notifier) - { - DBG_LOG(DBG_NOTIFIERS, "unregistering Val %p for notifier %s", - val, notifier->Name()); - - auto x = vals.equal_range(val); - for ( auto i = x.first; i != x.second; i++ ) - { - if ( i->second == notifier ) - { - vals.erase(i); - Unref(val); + registrations.erase(i); + --i->first->notifiers; break; } } } -void NotifierRegistry::Modified(Val *val) +void notifier::Registry::Unregister(Modifiable* m) { - DBG_LOG(DBG_NOTIFIERS, "modification to tracked value %p", val); - - auto x = vals.equal_range(val); + auto x = registrations.equal_range(m); for ( auto i = x.first; i != x.second; i++ ) - i->second->Modified(val); + Unregister(m, i->second); } -void NotifierRegistry::Modified(ID *id) +void notifier::Registry::Modified(Modifiable* m) { - DBG_LOG(DBG_NOTIFIERS, "modification to tracked ID %s", id->Name()); + DBG_LOG(DBG_NOTIFIERS, "modification to modifiable %p", m); - auto x = ids.equal_range(id); + auto x = registrations.equal_range(m); for ( auto i = x.first; i != x.second; i++ ) - i->second->Modified(id); + i->second->Modified(m); } -const char* NotifierRegistry::Notifier::Name() const +const char* notifier::Notifier::Name() const { return fmt("%p", this); } diff --git a/src/StateAccess.h b/src/StateAccess.h index 528e3357d5..91a7fa6186 100644 --- a/src/StateAccess.h +++ b/src/StateAccess.h @@ -1,4 +1,14 @@ // A class describing a state-modyfing access to a Value or an ID. +// +// TODO UPDATE: We provide a notifier framework to inform interested parties +// of modifications to selected global IDs/Vals. To get notified about a +// change, derive a class from Notifier and register the interesting +// instances with the NotifierRegistry. +// +// Note: For containers (e.g., tables), notifications are only issued if the +// container itself is modified, *not* for changes to the values contained +// therein. + #ifndef STATEACESSS_H #define STATEACESSS_H @@ -7,63 +17,74 @@ #include #include -class Val; -class ID; -class MutableVal; -class HashKey; -class ODesc; -class TableVal; +#include "util.h" -// We provide a notifier framework to inform interested parties of -// modifications to selected global IDs/Vals. To get notified about a change, -// derive a class from Notifier and register the interesting IDs/Vals with -// the NotifierRegistry. -// -// Note: For containers (e.g., tables), notifications are only issued if the -// container itself is modified, *not* for changes to the values contained -// therein. +namespace notifier { -class NotifierRegistry { +class Modifiable; + +class Notifier { public: - class Notifier { - public: - virtual ~Notifier() { } + virtual ~Notifier() { } - // Called when a change is being performed. Note that when - // these methods are called, it is undefined whether the - // change has already been done or is just going to be - // performed soon. - virtual void Modified(ID* id) = 0; - virtual void Modified(Val* val) = 0; - virtual const char* Name() const; // for debugging - }; - - NotifierRegistry() { } - ~NotifierRegistry(); - - // Register a new notifier to be informed when ID/Val changes. Note - // that the registry will store a reference to the target, keeping - // the instance alive for as long as it's registered. - void Register(ID* id, Notifier* notifier); - void Register(Val* val, Notifier* notifier); - - // Cancel a notifier's tracking for this ID/Val, also releasing the - // referencee being held. - void Unregister(ID* id, Notifier* notifier); - void Unregister(Val* val, Notifier* notifier); - - // Inform all registered notifiiers of a modification to a value/ID. - void Modified(ID *id); - void Modified(Val *val); - -private: - typedef std::unordered_multimap ValMap; - typedef std::unordered_multimap IDMap; - - ValMap vals; - IDMap ids; + // Called afger a change has been performed. + virtual void Modified(Modifiable* m) = 0; + virtual const char* Name() const; // for debugging }; -extern NotifierRegistry notifiers; +// Singleton class. +class Registry { +public: + Registry() { } + ~Registry(); + + // Register a new notifier to be informed when an instance changes. + void Register(Modifiable* m, Notifier* notifier); + + // Cancel a notifier's tracking an instace. + void Unregister(Modifiable* m, Notifier* notifier); + + // Cancel all notifiers registered for an instance. + void Unregister(Modifiable* m); + +private: + friend class Modifiable; + + // Inform all registered notifiers of a modification to an instance. + void Modified(Modifiable* m); + + typedef std::unordered_multimap ModifiableMap; + ModifiableMap registrations; +}; + + +class Registry; +extern Registry registry; + +// Base class for objects wanting to signal modifications to the registry. +class Modifiable { +protected: + friend class Registry; + + Modifiable() {} + ~Modifiable() + { + if ( notifiers ) + registry.Unregister(this); + + }; + + void Modified() + { + if ( notifiers ) + registry.Modified(this); + } + + // Number of currently registered notifiers for this instance. + uint64 notifiers; +}; + +} + #endif diff --git a/src/Trigger.cc b/src/Trigger.cc index 213707b6b8..c1295b8218 100644 --- a/src/Trigger.cc +++ b/src/Trigger.cc @@ -33,7 +33,7 @@ TraversalCode TriggerTraversalCallback::PreExpr(const Expr* expr) trigger->Register(e->Id()); Val* v = e->Id()->ID_Val(); - if ( v && v->IsMutableVal() ) + if ( v && v->Modifiable() ) trigger->Register(v); break; }; @@ -382,38 +382,30 @@ void Trigger::Timeout() void Trigger::Register(ID* id) { assert(! disabled); - notifiers.Register(id, this); + notifier::registry.Register(id, this); Ref(id); - ids.insert(id); + objs.push_back(id); } void Trigger::Register(Val* val) { + if ( ! val->Modifiable() ) + return; + assert(! disabled); - notifiers.Register(val, this); + notifier::registry.Register(val->Modifiable(), this); Ref(val); - vals.insert(val); + objs.push_back(val); } void Trigger::UnregisterAll() { - loop_over_list(ids, i) - { - notifiers.Unregister(ids[i], this); - Unref(ids[i]); - } + for ( auto o : objs ) + Unref(o); // this will unregister with the registry as well - ids.clear(); - - loop_over_list(vals, j) - { - notifiers.Unregister(vals[j], this); - Unref(vals[j]); - } - - vals.clear(); + objs.clear(); } void Trigger::Attach(Trigger *trigger) diff --git a/src/Trigger.h b/src/Trigger.h index 8dc478e049..fe7d425a72 100644 --- a/src/Trigger.h +++ b/src/Trigger.h @@ -13,7 +13,7 @@ class TriggerTimer; class TriggerTraversalCallback; -class Trigger : public NotifierRegistry::Notifier, public BroObj { +class Trigger : public notifier::Notifier, public BroObj { public: // Don't access Trigger objects; they take care of themselves after // instantiation. Note that if the condition is already true, the @@ -61,9 +61,7 @@ public: { d->Add(""); } // Overidden from Notifier. We queue the trigger and evaluate it // later to avoid race conditions. - void Modified(ID* id) override - { QueueTrigger(this); } - void Modified(Val* val) override + void Modified(notifier::Modifiable* m) override { QueueTrigger(this); } const char* Name() const override; @@ -104,8 +102,7 @@ private: bool delayed; // true if a function call is currently being delayed bool disabled; - val_list vals; - id_list ids; + std::vector objs; typedef map ValCache; ValCache cache; diff --git a/src/Val.cc b/src/Val.cc index 4ba2ea6dd7..bea9ce4038 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -899,7 +899,7 @@ static void table_entry_val_delete_func(void* val) delete tv; } -TableVal::TableVal(TableType* t, Attributes* a) : MutableVal(t) +TableVal::TableVal(TableType* t, Attributes* a) : Val(t) { Init(t); SetAttrs(a); @@ -1982,7 +1982,7 @@ unsigned int TableVal::MemoryAllocation() const vector RecordVal::parse_time_records; -RecordVal::RecordVal(RecordType* t, bool init_fields) : MutableVal(t) +RecordVal::RecordVal(RecordType* t, bool init_fields) : Val(t) { origin = 0; record_type = t; @@ -2287,7 +2287,7 @@ Val* EnumVal::DoClone(CloneState* state) return Ref(); } -VectorVal::VectorVal(VectorType* t) : MutableVal(t) +VectorVal::VectorVal(VectorType* t) : Val(t) { vector_type = t->Ref()->AsVectorType(); val.vector_val = new vector(); diff --git a/src/Val.h b/src/Val.h index 48f456fa17..d98c352f6d 100644 --- a/src/Val.h +++ b/src/Val.h @@ -328,6 +328,8 @@ public: void Describe(ODesc* d) const override; virtual void DescribeReST(ODesc* d) const; + virtual notifier::Modifiable* Modifiable() { return 0; } + #ifdef DEBUG // For debugging, we keep a reference to the global ID to which a // value has been bound *last*. @@ -490,8 +492,6 @@ protected: MutableVal() {} ~MutableVal() override; - - void Modified() { notifiers.Modified(this); } }; #define Microseconds 1e-6 @@ -764,7 +764,7 @@ protected: }; class CompositeHash; -class TableVal : public MutableVal { +class TableVal : public Val, public notifier::Modifiable { public: explicit TableVal(TableType* t, Attributes* attrs = 0); ~TableVal() override; @@ -877,6 +877,8 @@ public: HashKey* ComputeHash(const Val* index) const { return table_hash->ComputeHash(index, 1); } + notifier::Modifiable* Modifiable() override { return this; } + protected: friend class Val; TableVal() {} @@ -915,7 +917,7 @@ protected: Val* def_val; }; -class RecordVal : public MutableVal { +class RecordVal : public Val, public notifier::Modifiable { public: explicit RecordVal(RecordType* t, bool init_fields = true); ~RecordVal() override; @@ -962,6 +964,8 @@ public: unsigned int MemoryAllocation() const override; void DescribeReST(ODesc* d) const override; + notifier::Modifiable* Modifiable() override { return this; } + // Extend the underlying arrays of record instances created during // parsing to match the number of fields in the record type (they may // mismatch as a result of parse-time record type redefinitions. @@ -1006,7 +1010,7 @@ protected: }; -class VectorVal : public MutableVal { +class VectorVal : public Val, public notifier::Modifiable { public: explicit VectorVal(VectorType* t); ~VectorVal() override; @@ -1054,6 +1058,8 @@ public: // Won't shrink size. unsigned int ResizeAtLeast(unsigned int new_num_elements); + notifier::Modifiable* Modifiable() override { return this; } + protected: friend class Val; VectorVal() { }