Redo NotfifierRegistry to no longer rely on StateAccess.

We simplify the API to a simple Modified() operation.

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());
+	ids.insert({id, notifier});
-
-	if ( i != ids.end() )
-		i->second->insert(notifier);
-	else
-		{
-		NotifierSet* s = new NotifierSet;
-		s->insert(notifier);
-		ids.insert(NotifierMap::value_type(id->Name(), s));
-		}
-
 	Ref(id);
 	}
 
 void NotifierRegistry::Register(Val* val, NotifierRegistry::Notifier* notifier)
 	{
-	if ( val->IsMutableVal() )
+	if ( ! val->IsMutableVal() )
-		Register(val->AsMutableVal()->UniqueID(), notifier);
+		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());
+	auto x = ids.equal_range(id);
-
+	for ( auto i = x.first; i != x.second; i++ )
-	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 )
 		{
-		delete s;
+		if ( i->second == notifier )
-		ids.erase(i);
+			{
+			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() )
+	DBG_LOG(DBG_NOTIFIERS, "unregistering Val %p for notifier %s",
-		Unregister(val->AsMutableVal()->UniqueID(), notifier);
+		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());
+	auto x = vals.equal_range(val);
-
+	for ( auto i = x.first; i != x.second; i++ )
-	if ( i == ids.end() )
+		i->second->Modified(val);
-		return;
+	}
 
+void NotifierRegistry::Modified(ID *id)
+	{
 	DBG_LOG(DBG_NOTIFIERS, "modification to tracked ID %s", id->Name());
 
-	NotifierSet* s = i->second;
+	auto x = ids.equal_range(id);
-
+	for ( auto i = x.first; i != x.second; i++ )
-	if ( id->IsInternalGlobal() )
+		i->second->Modified(id);
-		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);
 	}
 
 const char* NotifierRegistry::Notifier::Name() const

src/StateAccess.h

@@ -4,7 +4,7 @@
 #define STATEACESSS_H
 
 #include <set>
-#include <map>
+#include <unordered_map>
 #include <string>
 
 class Val;
@@ -105,32 +105,39 @@ public:
 	public:
 		virtual ~Notifier()	{ }
 
-		// Called when a change is being performed. Note that when these
+		// Called when a change is being performed. Note that when
-		// methods are called, it is undefined whether the change has
+		// these methods are called, it is undefined whether the
-		// already been done or is just going to be performed soon.
+		// change has already been done or is just going to be
-		virtual void Access(ID* id, const StateAccess& sa) = 0;
+		// performed soon.
-		virtual void Access(Val* val, const StateAccess& sa) = 0;
+		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:
+	// Inform all registered notifiiers of a modification to a value/ID.
-	friend class StateAccess;
+	void Modified(ID *id);
-	void AccessPerformed(const StateAccess& sa);
+	void Modified(Val *val);
 
-	typedef std::set<Notifier*> NotifierSet;
+private:
-	typedef std::map<std::string, NotifierSet*> NotifierMap;
+	typedef std::unordered_multimap<Val*, Notifier*> ValMap;
-	NotifierMap ids;
+	typedef std::unordered_multimap<ID*, Notifier*> IDMap;
+
+	ValMap vals;
+	IDMap ids;
 };
 
 extern NotifierRegistry notifiers;

src/Trigger.h

@@ -61,9 +61,9 @@ public:
 		{ d->Add("<trigger>"); }
 	// 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;