-
Notifications
You must be signed in to change notification settings - Fork 530
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
Implement async dropping queue #4143
Conversation
@@ -418,7 +418,11 @@ class UnboundedQueueSpec extends BaseSpec with QueueTests[Queue] { | |||
class DroppingQueueSpec extends BaseSpec with QueueTests[Queue] { | |||
sequential | |||
|
|||
"DroppingQueue" should { | |||
"DroppingQueue (concurrent)" should { | |||
droppingQueueTests(i => if (i < 1) Queue.dropping(i) else Queue.droppingForConcurrent(i)) |
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.
droppingForConcurrent
doesn't enforce capacity constraints because it's used internally.
d7d2039
to
9a7fc5b
Compare
extends BaseBoundedAsyncQueue[F, A](capacity) { | ||
|
||
def offer(a: A): F[Unit] = | ||
F.uncancelable { _ => |
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.
The fiber run loop handles the cancellation. The block has exactly one synchronous operation F.delay
. The delay
operation cannot be canceled in the middle, right?
So, is F.uncancelable
necessary here?
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.
The
delay
operation cannot be canceled in the middle, right?
That's right. uncancelable
is not needed here :)
9a7fc5b
to
8fe95e6
Compare
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.
This is excellent, thank you!
Closes #4141.
Benchmarks: