Optimize Semaphore
#9662
Optimize Semaphore
#9662
Conversation
|
|
|
I also copied benchmark from #9093, here is the result: |
1201b68 to
f186f03
Compare
|
💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe. |
|
The original benchmark is quite unfair for ZIO in a way that the Java version use try ... finally ..., which is more performance that New benchmark result here: |
b861376 to
959f1d6
Compare
| * Creates a new `Semaphore`, baced by ConcurrentSemaphore with the specified | ||
| * number of permits, and fairness. | ||
| */ | ||
| def make2(permits: => Long, fair: => Boolean = false)(implicit trace: Trace): UIO[Semaphore] = |
There was a problem hiding this comment.
Why not just a separate makeUnfair constructor?
There was a problem hiding this comment.
I just follow Java signature here, can be updated if you want
| @@ -32,7 +33,7 @@ import scala.collection.immutable.{Queue => ScalaQueue} | |||
| * If you need functionality that `Semaphore` doesnt' provide, use a | |||
| * [[TSemaphore]] and define it in a [[zio.stm.ZSTM]] transaction. | |||
| */ | |||
| sealed trait Semaphore extends Serializable { | |||
| private[zio] trait Semaphore extends Serializable { | |||
There was a problem hiding this comment.
| private[zio] trait Semaphore extends Serializable { | |
| sealed trait Semaphore extends Serializable { |
in object Semaphore { private[zio] abstract class SemaphoreBase extends Semaphore
|
|
||
| private final val unitReservation = Exit.succeed(ZeroReservation) | ||
|
|
||
| private final class Waiter( |
There was a problem hiding this comment.
Is there a limitation of Promise that makes it unusable for this case? It seems like you are re-encoding a lot of behavior here.
There was a problem hiding this comment.
Promise has more overhead as it is design for common use cases. Waiter is a Promise that only allow 1 party to await. Because of that the implementation can be more specialized (we don't need as many compare and set loop for Waiter as for Promise).
The code inside Waiter are pretty much dead code, the real logics are handled by semaphore itself. I drafted the logic and forgot to remove it.
There was a problem hiding this comment.
I also think that semaphore is low level enough to be implemented as low as possible and not based on a more generic structure like Promise
|
|
||
| } | ||
|
|
||
| private[SemaphoreImpls] sealed trait WaiterQueue { |
There was a problem hiding this comment.
I'm slightly concerned with adding a new Queue implementation if it's just used for one structure. Instead, can we consider shading jctools? It would be useful elsewhere - and likely more performant.
There was a problem hiding this comment.
This queue was not created for performance, but for the ability to reduce permit and poll waiter out atomically. I created this because I failed to think of a way to do it with 1 queue and 1 atomic var separately.
About performance, this is only as fast as ConcurrentLinkedQueue. I'd love to use jctools too if I figured out how to do it
There was a problem hiding this comment.
If you shade the dependency, you can bring it in.
There was a problem hiding this comment.
Again, I create this queue because I cant think of a way to reduce permit and poll in an atomic step, not because I dont want to use existing queue implementations
| private final val unitReservation = Exit.succeed(ZeroReservation) | ||
|
|
||
| private final class Waiter( | ||
| val blockingOn: () => FiberId, |
There was a problem hiding this comment.
This doesn't need to be a thunk.
| } | ||
| } | ||
|
|
||
| private def await(reservation: Reservation)(implicit trace: Trace): UIO[Unit] = ZIO.suspendSucceed { |
There was a problem hiding this comment.
If this is always used within a suspension, you can name it awaitUnsafe, then remove the suspendSuceed
0e116dc to
87ab9fa
Compare
|
This is ready for review |
|
@kyri-petrou sorry to ping you directly, but this PR had been sitting here for a while now, so I want to know what we gonna do about this. I'm fine with either continue or close this PR |
|
@HollandDM Can you resolve the conflict, please? |
87ab9fa to
6953bca
Compare
/claim #9093
This PR attempts to optimize ZIO
Semaphoreimplementation by:Promiseto a dumb-ed downWaiterobject, this eliminatePromisespin wait.WaiterQueue, a simplified and customized version of Java'sConcurrentLinkedQueue, as the semaphore wait queue.This PR yield some moderate boosts for ZIO's
Semaphore, but it's still not catch up to the same level as Java'sSemaphoreyet. Further optimizations + discussions are welcomed