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
Fixes #16892: Upgrade to ZIO RC18 #2817
Conversation
PR updated with a new commit |
1 similar comment
PR updated with a new commit |
PR updated with a new commit |
|
||
final def makeDefault(yieldOpCount: Int): Executor = | ||
Executor.fromThreadPoolExecutor(_ => yieldOpCount) { | ||
val corePoolSize = Runtime.getRuntime.availableProcessors() * 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why twice the number of core ? is it documented somewhere the reasoning ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, we don't use it at all anymore. I think it was just for tests.
The reasoning was: we have a lot of threads used to process blocking fiber, and most of them actually block. So it is reasonnable to have a core pool size of CPUx2, given most of them will be block often.
In practice, IIRC, it worked well for small number of CPU, but for more (like 16), it was a bit too big.
PR updated with a new commit |
230d2c1
to
a650023
Compare
PR updated with a new commit |
UPDATE: the last version ( |
PR updated with a new commit |
1 similar comment
PR updated with a new commit |
Performance impact is really significative
This branch
To StringTemplate is twice as fast, yet copy ressources and write csv are much slower (and have same length in both, so it may be a logging bug) For the record, branch 6.0 is
|
@ncharles : copy resources / csv is a bug:
Both uses |
This PR is not mergeable to upper versions. |
OK, squash merging this PR |
62d98f9
to
8c76aa3
Compare
https://issues.rudder.io/issues/16892
Switch to RC18-1. Use https://docs.google.com/document/d/1P0mx1gSNU2UTi-9PXUeY8O74v5gyK-UETznM1SbkQjA/edit#heading=h.37bbueaecc51
Firs commit, make it compiles and tests pass. Middle commit: trivial changes.
Last commit: use
ZIO
default threadpool for blocking.Main changes:
ZSchedule
is renamedSchedule
;traverse
if renamed intoforeach
andsequence
intocollectAll
;.succeed
and.fail
are deleted => create an implicit for them;use
forkDaemon
whenever (== almost always) the fiber should be a daemon and is notjoin
ed;Change ZioCommon#internal to reflect changes with
ZLayer
. Now the runtime is built by hand, and we initialize the environment by using it to getZLayer[Any, Nothing, ZEnv]
in place ofZManaged[Any, ZLayer[...]]
.ZEnv
is clock, blocking, console, etc. So we have access to all that;ZLayer
presentation: https://github.com/adamgfraser/solving-the-dependency-injection-problem-with-zio/blob/master/solving-the-dependency-injection-problem-with-zio.pdfprovide
is replaced byprovideLayer
to useZLayer
.use
ZIO
default threadpool forPlatform
as it is consistently (on my machine) 20% faster than our.