pnowojski commented on a change in pull request #8692: [FLINK-12804] Introduce
mailbox-based ExecutorService
URL: https://github.com/apache/flink/pull/8692#discussion_r302952528
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/mailbox/MailboxImpl.java
##########
@@ -38,39 +40,44 @@
public class MailboxImpl implements Mailbox {
/**
- * The enqueued letters.
+ * Node for the embedded linked list that represents the mailbox queue.
*/
- @GuardedBy("lock")
- private final Runnable[] ringBuffer;
+ static final class Node {
- /**
- * Lock for all concurrent ops.
- */
- private final ReentrantLock lock;
+ /** The payload. */
+ final Runnable letter;
+
+ /** Link to the next node in the list. */
+ @Nullable
+ Node next;
+
+ Node(Runnable letter) {
+ this.letter = letter;
+ }
+ }
/**
- * Condition that is triggered when the buffer is no longer empty.
+ * The head of the linked list of mailbox actions.
*/
- @GuardedBy("lock")
- private final Condition notEmpty;
+ @Nullable
+ private Node head;
/**
- * Condition that is triggered when the buffer is no longer full.
+ * The tail of the linked list of mailbox actions.
*/
- @GuardedBy("lock")
- private final Condition notFull;
+ @Nullable
+ private Node tail;
Review comment:
I think the singly linked list implementation should be extracted to a
separate class. Especially that we might want to evolve this structure in the
future, with for example an iterator that can remove things efficiently.
Also, is there no singly linked list implementation anywhere in our class
path (dependencies?).
Another idea (maybe even better), to not pre mature optimise things: just
use doubly `LinkedList`? In fact I think I would be in favour of just doing
that as it will save us both implementation, maintaining & testing efforts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
