Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InterceptRunnable issues #992

Closed
alexandru opened this issue Aug 27, 2019 · 0 comments · Fixed by #998

Comments

@alexandru
Copy link
Member

commented Aug 27, 2019

First of all we are exposing this interface in monix.execution.internal:

trait InterceptableRunnable extends Runnable {
  def intercept(handler: UncaughtExceptionReporter): Runnable
}

This is a public interface. We should never have public APIs in internal packages. There's also no reason to expose it. We are going to have to break source compatibility in the next release, by making it private[internal] or private[execution], hopefully nobody used

On analyzing the code for InterceptableRunnable, this is wasteful:

def apply(r: Runnable, handler: UncaughtExceptionReporter): Runnable =
  if (handler eq null) r else apply(r).intercept(handler)

When executing stuff on the thread pool, it's bad enough that we end up wrapping each incoming Runnable in another Runnable. And we must not do it two or more times, if we can help it. That call needs to wrap the given Runnable at most once.

This is why having an InterceptableRunnable with an intercept method on it, isn't useful. Because the presence of that intercept method on it assumes that we are going to use it, ending up with more short-term allocations than necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.