-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Set of standard producers and updated queue implementations with some #2963
Conversation
16af514
to
2ebde5e
Compare
long requested; | ||
Producer currentProducer; | ||
|
||
boolean emitting; |
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.
I think we could spruce up this class with the naming. This class doesn't emit anything, the emit loop actually performs requests so is a request loop (which may induce emissions elsewhere perhaps but I don't think that justifies the use of emit).
Perhaps emitting
-> busy
or requesting
and emitLoop
->requestLoop
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.
It's named after the emitter-loop pattern and I'd like to keep it named this way so developers can recognize it.
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.
Yeah I noticed the naming pattern from your blogs and I think instead of a queue drainer and an emitter they are better described as both queue drainers except one is aysnc biased and the other is sync biased.
2ebde5e
to
39be310
Compare
* The queue is initialized with a stub node which is set to both the producer and consumer node references. From this | ||
* point follow the notes on offer/poll. | ||
* | ||
* @author nitsanw |
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.
If we are copying this from somewhere I think we should include the URL to where it came from and associated copyright information.
This looks like good additions. If I understand correctly there are two things happening here:
The only thing I would like changed before merging is to include correct attribution to where the code comes from. |
Sure, I'll add the link. These aren't all the android-compatible variants so I plan to do a separate PR on the remaining and updating all platform-checked places. |
Then should the producers and queues be separated into different PRs? |
The default constructor of |
Since these don't change any existing code and are all internal, let's merge and move forward. |
Okay. |
Set of standard producers and updated queue implementations with some
platform-safe variants.