Conversation
| Async[F].delay { | ||
| val jContext: JContext = ctx.underlying | ||
| val _ = jContext.makeCurrent() | ||
| }.flatMap(_ => k(cb)) |
There was a problem hiding this comment.
I think this is problematic because there's no guarantee that delay and the subsequent flatMap are executed together. Any ideas?
There was a problem hiding this comment.
Perhaps we can use evalOn with a custom ExecutonContext:
def tracedContext(ctx: JContext): ExecutionContext =
new ExecutionContext {
def execute(runnable: Runnable): Unit = {
val scope = ctx.makeCurrent()
try {
runnable.run()
} finally {
scope.close()
}
}
def reportFailure(cause: Throwable): Unit =
cause.printStackTrace()
}
def asyncWithContext[F[_]: Async, A](k: (Either[Throwable, A] => Unit) => F[Option[F[Unit]]])(implicit L: Local[F, Context]): F[A] =
Local[F, Context].ask.flatMap { ctx =>
Async[F].evalOn(
Async[F].async[A](cb => k(cb)),
tracedContext(ctx.underlying)
)
}There was a problem hiding this comment.
But implications on performance are unknown :(
There was a problem hiding this comment.
evalOn is applicable for all scenarios, actually. But again, it may (and highly likely will) affect the performance.
def evalWithJContext[F[_]: Async, A](fa: F[A])(implicit L: Local[F, Context]): F[A] =
Local[F, Context].ask.flatMap { ctx =>
Async[F].evalOn(fa, tracedContext(ctx.underlying))
}There was a problem hiding this comment.
Good idea :) Why do you think it'll affect performance (assuming fa pretty much immediately calls the async API instead of doing a bunch of CPU work)
There was a problem hiding this comment.
AFAIK context shifts may be costly.
The current implementation of the tracedContext is basically a same-thread execution.
Here is the logic for the evalOn in the CE: https://github.com/typelevel/cats-effect/blob/3cf97ae4fd643f333febb4554af2fb603ed2c9d2/core/shared/src/main/scala/cats/effect/IOFiber.scala#L976-L988.
If I get it right, fa will be executed right within the fiber's loop.
@armanbilge perhaps you have some insides?
There was a problem hiding this comment.
I've added your suggestion for now. I'm thinking that a typical async API involves using another threadpool so context shift isn't avoidable in that case. (But yeah will be good if an additional context shift can be avoided)
| createOtel4s[IO].flatMap(otel4s => program(otel4s)) | ||
| def run: IO[Unit] = | ||
| OtelJava.autoConfigured[IO]() { otel4s => | ||
| implicit val local: Local[IO, Context] = otel4s.localContext |
There was a problem hiding this comment.
| implicit val local: Local[IO, Context] = otel4s.localContext | |
| import otel4s.localContext |
|
Seems like one of the builds had an OOM. If someone can help me retrigger it it should pass 🤞 |
| def tracedContext(ctx: JContext): ExecutionContext = | ||
| new ExecutionContext { | ||
| def execute(runnable: Runnable): Unit = { | ||
| val scope = ctx.makeCurrent() | ||
| try { | ||
| runnable.run() | ||
| } finally { | ||
| scope.close() | ||
| } | ||
| } | ||
| def reportFailure(cause: Throwable): Unit = | ||
| cause.printStackTrace() | ||
| } |
There was a problem hiding this comment.
Sorry, just getting caught up ...
Hmm, I'm concerned by this. If I'm reading correctly you have effectively implemented a ParasiticExecutionContext. If you run arbitrary effects on this you may stackoverflow and/or dead-lock.
4 participants
Rough draft for now. Any feedback/nitpick welcome :)