multiscale / muscle3

The third major version of the MUltiScale Coupling Library and Environment
Apache License 2.0
25 stars 13 forks source link

Consider introducing event-based AOP #303

Open LourensVeen opened 2 weeks ago

LourensVeen commented 2 weeks ago

We now have three features that are orthogonal to MUSCLE3's primary functionality of sending messages between instances, but that are woven into the code: ApiGuard, MMSFValidator, and Profiler. Especially profiling of receives is messy, with the extra timestamps being passed back from the client.

It may be a good idea to introduce event-based AOP using a pub-sub mechanism. Then you could write in the code event(type, args...), calling a global event function from libmuscle.events, and this would then dispatch (in the same thread) to subscribers that would check whether that event should be occurring in that order, or that record timestamps and profile stuff.

This is dangerous because it does away with the strictly hierarchical structure and adds an event-routing system instead, and those are always spaghetti, but as long as we use it only for functionality that is orthogonal to the main flow, it could help. It may actually help to hard-code the dispatch in the event() function, so that you can easily see what's happening rather than having to find some seemingly-unrelated bit of initialisation code hidden somewhere among the rest. We would have to consider whether the above classes become singletons or whether they're still part of Instance somehow.

The code is not terrible as-is, but something to think about for when there's time to work on it.

LourensVeen commented 2 weeks ago

Thinking a bit more, I'd still like to have everything attached to Instance. But that's tricky, because you'd have to pass a pointer to the Instance everywhere. Unless you emit an event at every entry and exit of the Instance API, passing that Instance object, and put it into a thread-local global pointer. Then you can look that up when subsequent events are triggered, and use it to process them. Would work. More complexity and a bunch of extra code given the size of the Instance API. Hmm.