Use Cats Effect EC as HttpClient executor
#641
Conversation
| ec match { | ||
| case exec: util.concurrent.Executor => builder.executor(exec) | ||
| case _ => builder.executor(ec.execute) | ||
| } |
There was a problem hiding this comment.
Hopefully we will upstream this into Cats Effect.
|
So I've been poking around in the JDK sources and again, I'm pretty sure this makes sense. I think it would be strange to be running block ops on this, but I'm no expert 😅 |
There was a problem hiding this comment.
I agree that it seems unplausible that the Executor is asked to running blocking tasks. Another point in favor of this is that by default, Executors.newCachedThreadPool is used:
These pools will typically improve the performance of programs that execute many short-lived asynchronous tasks.
Do you think it would be a good idea to update the documentation at https://jdk-http-client.http4s.org/0.7/#custom-clients to also set the executor like this? Or do we want to wait for typelevel/cats-effect#3001?
|
Oh, good eye! Yup, this is almost definitely the right thing to do then. Good point, I can update the docs. We can just demonstrate it with wrapping for now, since the default work-stealing threadpool in CE isn't even an |
There was a problem hiding this comment.
Approved, this can be followed up on when a release with typelevel/cats-effect#3001 lands 👍
I'm not 100% sure on this change yet, but I'm pretty sure this executor is not used for blocking operations. In which case, we should definitely prefer to use the Cats Effect compute pool here. It means one less threadpool in the application and thus less contention and less page faults, which is a big win.
/** * Sets the executor to be used for asynchronous and dependent tasks. * * <p> If this method is not invoked prior to {@linkplain #build() * building}, a default executor is created for each newly built {@code * HttpClient}.https://github.com/openjdk/jdk11/blob/37115c8ea4aff13a8148ee2b8832b20888a5d880/src/java.net.http/share/classes/java/net/http/HttpClient.java#L253