Kernel#Sync: pass annotation: keyword to newly created Reactor

[paddor]

Just for symmetry with Kernel#Async. In a library I changed the specs to always run in a Sync {...} block which meant I had to change the main reactor from Async(annotation: 'x') { ... } to Async(annotation: 'x') { ... }.wait because Kernel#Sync didn't take an annotation. This PR allows it to be just Sync(annotation: 'x') { ... }.

[ioquatix]

How do you think we should handle the following case?

Async(annotation: "foo") do
  Sync(annotation: "bar") do |task|
    puts task.annotation
  end
end

[paddor]

Could Kernel#Sync replace the annotation of the current task and then restore it to the previous value?

[ioquatix]

Could Kernel#Sync replace the annotation of the current task and then restore it to the previous value?

Yeah, I considered that too. I think it's a reasonable idea. I think we can take advantage of https://github.com/ioquatix/fiber-annotation/blob/main/lib/fiber/annotation.rb#L31-L45

[paddor]

Task#annotate / Fiber#annotate would be nice but they don't yield the task to the block.

[paddor]

Ah wait, that's not a big deal. I'm working on it...

[ioquatix]

We can always update the fiber annotation gem to support ... arguments to yield or something like that.

[paddor]

I think this is ready.

[ioquatix]

Thanks for your contribution.

I understand the value of this change, and think that it's ultimately a good change.

However, I want to point out there will be a slight performance hit. Hopefully things like YJIT mitigate it on the fast path (no annotation, existing task).

I also had some thoughts around how to present annotations in backtrace, e.g. an annotation is stack frame specific and gets added to exception messages, etc. In other words, there is more work to be done in this area FYI.

[paddor]

Thanks for the guidance and merging.

I'd be a little surprised if the performance hit was even measurable.

Annotations in the backtrace sounds like a good idea. Let me know if I can help in any way.

[ioquatix]

Annotating the call stack would be an alternative to annotating the fiber. However, for the purpose of Async, I think it wouldn't look that different.

We'd need to write up a proposal on <bugs.ruby-lang.org> and propose a similar but non-finer specific interface for "annotate the current stack frame". If it was accepted, then Sync(annotation:) would simply use that (and we'd also need an interface for getting the top most annotation).

[ioquatix]

https://bugs.ruby-lang.org/issues/19056 was the previous discussion, and at the meeting we discussed having per-call-stack annotations (could be added to exception backtrace too).

[paddor]

So something like Callstack.annotate(my_annotation) { ... }? And I guess (because Matz said it would have to be stacked) it would have to append the annotation into an array (like breadcrumbs), so Callstack.annotations would return the entire Array (like Class#ancestors), and pop from that array when the annotated block is done.

What did you mean by "APM" in that discussion on bugs.ruby-lang.org? If it's advanced power management, I don't know how that relates to this.

[ioquatix]

Application performance monitoring (APM) is the practice of tracking key software application performance metrics using monitoring software and telemetry data. Practitioners use APM to ensure system availability, optimize service performance and response times, and improve user experiences.

[paddor]

Thank you.