From 02214dafc40cf3cf7996e879b99ec6cde1f187f1 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Thu, 6 Jun 2019 02:49:18 +0000 Subject: [PATCH] Redo NotfifierRegistry to no longer rely on StateAccess. We simplify the API to a simple Modified() operation. --- src/StateAccess.cc | 109 +++++++++++++++++++++++---------------------- src/StateAccess.h | 37 ++++++++------- src/Trigger.h | 4 +- 3 files changed, 80 insertions(+), 70 deletions(-) diff --git a/src/StateAccess.cc b/src/StateAccess.cc index 997eb5a48d..cc867468c8 100644 --- a/src/StateAccess.cc +++ b/src/StateAccess.cc @@ -149,7 +149,7 @@ void StateAccess::Replay() Val* v = target.id->ID_Val(); TypeTag t = v ? v->Type()->Tag() : TYPE_VOID; - + if ( opcode != OP_ASSIGN && ! v ) { // FIXME: I think this warrants an internal error, @@ -514,22 +514,17 @@ void StateAccess::Describe(ODesc* d) const void StateAccess::Log(StateAccess* access) { - bool tracked = false; - if ( access->target_type == TYPE_ID ) { if ( access->target.id->FindAttr(ATTR_TRACKED) ) - tracked = true; + notifiers.Modified(access->target.id); } else { if ( access->target.val->GetProperties() & MutableVal::TRACKED ) - tracked = true; + notifiers.Modified(access->target.val); } - if ( tracked ) - notifiers.AccessPerformed(*access); - #ifdef DEBUG ODesc desc; access->Describe(&desc); @@ -543,6 +538,15 @@ void StateAccess::Log(StateAccess* access) NotifierRegistry notifiers; +NotifierRegistry::~NotifierRegistry() + { + for ( auto i : ids ) + Unref(i.first); + + for ( auto i : vals ) + Unref(i.first); + } + void NotifierRegistry::Register(ID* id, NotifierRegistry::Notifier* notifier) { DBG_LOG(DBG_NOTIFIERS, "registering ID %s for notifier %s", @@ -563,24 +567,20 @@ void NotifierRegistry::Register(ID* id, NotifierRegistry::Notifier* notifier) Unref(attr); - NotifierMap::iterator i = ids.find(id->Name()); - - if ( i != ids.end() ) - i->second->insert(notifier); - else - { - NotifierSet* s = new NotifierSet; - s->insert(notifier); - ids.insert(NotifierMap::value_type(id->Name(), s)); - } - + ids.insert({id, notifier}); Ref(id); } void NotifierRegistry::Register(Val* val, NotifierRegistry::Notifier* notifier) { - if ( val->IsMutableVal() ) - Register(val->AsMutableVal()->UniqueID(), notifier); + if ( ! val->IsMutableVal() ) + return; + + 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) @@ -588,52 +588,55 @@ void NotifierRegistry::Unregister(ID* id, NotifierRegistry::Notifier* notifier) DBG_LOG(DBG_NOTIFIERS, "unregistering ID %s for notifier %s", id->Name(), notifier->Name()); - NotifierMap::iterator i = ids.find(id->Name()); - - if ( i == ids.end() ) - return; - - Attr* attr = id->Attrs()->FindAttr(ATTR_TRACKED); - id->Attrs()->RemoveAttr(ATTR_TRACKED); - Unref(attr); - - NotifierSet* s = i->second; - s->erase(notifier); - - if ( s->size() == 0 ) + auto x = ids.equal_range(id); + for ( auto i = x.first; i != x.second; i++ ) { - delete s; - ids.erase(i); + if ( i->second == notifier ) + { + ids.erase(i); + Unref(id); + break; + } } - Unref(id); + 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) { - if ( val->IsMutableVal() ) - Unregister(val->AsMutableVal()->UniqueID(), 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); + break; + } + } } -void NotifierRegistry::AccessPerformed(const StateAccess& sa) +void NotifierRegistry::Modified(Val *val) { - ID* id = sa.Target(); + DBG_LOG(DBG_NOTIFIERS, "modification to tracked value %p", val); - NotifierMap::iterator i = ids.find(id->Name()); - - if ( i == ids.end() ) - return; + auto x = vals.equal_range(val); + for ( auto i = x.first; i != x.second; i++ ) + i->second->Modified(val); + } +void NotifierRegistry::Modified(ID *id) + { DBG_LOG(DBG_NOTIFIERS, "modification to tracked ID %s", id->Name()); - NotifierSet* s = i->second; - - if ( id->IsInternalGlobal() ) - for ( NotifierSet::iterator j = s->begin(); j != s->end(); j++ ) - (*j)->Access(id->ID_Val(), sa); - else - for ( NotifierSet::iterator j = s->begin(); j != s->end(); j++ ) - (*j)->Access(id, sa); + auto x = ids.equal_range(id); + for ( auto i = x.first; i != x.second; i++ ) + i->second->Modified(id); } const char* NotifierRegistry::Notifier::Name() const diff --git a/src/StateAccess.h b/src/StateAccess.h index 15e2ae4676..86f1dbe88c 100644 --- a/src/StateAccess.h +++ b/src/StateAccess.h @@ -4,7 +4,7 @@ #define STATEACESSS_H #include -#include +#include #include class Val; @@ -105,32 +105,39 @@ public: public: 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 Access(ID* id, const StateAccess& sa) = 0; - virtual void Access(Val* val, const StateAccess& sa) = 0; + // 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() { } + ~NotifierRegistry(); - // Inform the given notifier if ID/Val changes. + // 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 notification for this ID/Val. + // 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); -private: - friend class StateAccess; - void AccessPerformed(const StateAccess& sa); + // Inform all registered notifiiers of a modification to a value/ID. + void Modified(ID *id); + void Modified(Val *val); - typedef std::set NotifierSet; - typedef std::map NotifierMap; - NotifierMap ids; +private: + typedef std::unordered_multimap ValMap; + typedef std::unordered_multimap IDMap; + + ValMap vals; + IDMap ids; }; extern NotifierRegistry notifiers; diff --git a/src/Trigger.h b/src/Trigger.h index 0f7889d19a..8dc478e049 100644 --- a/src/Trigger.h +++ b/src/Trigger.h @@ -61,9 +61,9 @@ public: { d->Add(""); } // Overidden from Notifier. We queue the trigger and evaluate it // later to avoid race conditions. - void Access(ID* id, const StateAccess& sa) override + void Modified(ID* id) override { QueueTrigger(this); } - void Access(Val* val, const StateAccess& sa) override + void Modified(Val* val) override { QueueTrigger(this); } const char* Name() const override;