Notify supervisors before notifying other observers on fiber end

[mschuwalow]

Now that we have better traces, I started playing around with profiling again.
In order to support it we would need to expose a few more events to supervisors.

A few notes:

• onAsyncStart: Called on start of an asynchronous wait. This is necessary to track which fibers are semantically 'running' and which are waiting for something.
• onAsyncEnd: Called on end of an asynchronous wait. Counterpart to onAsyncStart.
• In principle both onAsyncStart and onAsyncEnd could be figured out by supervisors by enabling opSupervision and matching on the Event. Question here is whether we want to support supervisors doing this without opSupervision. (Profilers will have opSupervision enabled anyway, so 🤷 )
• onObserverNotify: The usecase here is that we need to know when a fiber might wake up other fibers. onFiberEnd is not suitable for this as it is implemented using an observer. Right now observers are maintained as a list and new obsevers are prepended. This leads to the semantics of "whoever observed last will get notified first". As the supervisor will observe the fiber as the very first thing before starting https://github.com/zio/zio/blob/series/2.x/core/shared/src/main/scala/zio/ZIO.scala#L2487, it will only be notified after all other observers are notified / fibers are woken up. An alternative solution would be to reverse the list of observers before notifying, switching the semantics to "whoever observed first will get notified first"

Edit:
Instead of adding a new callback to the supervisor it's enough to enforce that the existing onEnd callback is called before any fibers that were forked after supervision are woken up.

[adamgfraser]

I don't think the supervisor interface has changed since we did the initial implementation of the profiler. Would like to avoid adding additional methods to the supervisor interface if possible. Maybe a good first step is upgrading zio-profiling to ZIO 2.0 and then we can look at what the output is with and without this change and the best way to support it?

[mschuwalow]

I don't think the supervisor interface has changed since we did the initial implementation of the profiler. Would like to avoid adding additional methods to the supervisor interface if possible. Maybe a good first step is upgrading zio-profiling to ZIO 2.0 and then we can look at what the output is with and without this change and the best way to support it?

The only blocker for this is onObserverNotify / reversing the order in which observers are notified 👍
I have a branch of the profiler ready for 2.0 once that change is in. Afterwards I would like to also look at adding a normal tracing or sampling profiler and polishing it to the point it's actually usable.

Overall the output is much better now, though I believe something like flamegraphs are still impossible as we have no 'hierarchy' in our traces.

[mschuwalow]

@adamgfraser seems like all existing tests are fine with reversing the order in which observers are notified. If we are happy with this change (it would mean that supervisors in general would not be called as the very last thing), that would be enough to make the profiler work.

Updated the pr and will add a test that ensures that supervisors are called before fibers are woken up

[mschuwalow]

Potentially this makes sense to have in the rts spec instead. wdyt?

[jdegoes]

@mschuwalow You still want this in?

Maybe we could separate into 2 PRs, one to just reverse order of observers?

[mschuwalow]

@mschuwalow You still want this in?

Maybe we could separate into 2 PRs, one to just reverse order of observers?

Yep, still interested in this.

Right now this only has the reversal of observers and tests accompanying it -- which turns of to be enough for me.
I left the original conversation as is to make the motivation of this change a bit clearer.