Browse Source

Remove chain of Comparators in output queue.

Instead of having lots of objects implement Comparable or Comparator,
pass the comparator in to the PriorityBlockingQueue.

This requires recreating the queue if the QueueHandler is ever changed,
but the previous behaviour resulted in an unstable/undefined ordering,
so it's probably better this way anyway.

This means that QueueItems no longer need to know about QueueHandler,
which makes me slightly happier.
pull/127/head
Chris Smith 7 years ago
parent
commit
8942ddaeea

+ 18
- 3
irc/src/com/dmdirc/parser/irc/outputqueue/OutputQueue.java View File

@@ -41,7 +41,7 @@ public class OutputQueue {
41 41
     /** Are we discarding all futher input? */
42 42
     private boolean discarding = false;
43 43
     /** The output queue! */
44
-    private final BlockingQueue<QueueItem> queue = new PriorityBlockingQueue<>();
44
+    private BlockingQueue<QueueItem> queue = new PriorityBlockingQueue<>();
45 45
     /** Object for synchronising access to the {@link #queueHandler}. */
46 46
     private final Object queueHandlerLock = new Object();
47 47
     /** Thread for the sending queue. */
@@ -71,7 +71,7 @@ public class OutputQueue {
71 71
      * @param outputStream Output Stream to use.
72 72
      */
73 73
     public void setOutputStream(final OutputStream outputStream) {
74
-        this.out = new PrintWriter(outputStream, true);
74
+        out = new PrintWriter(outputStream, true);
75 75
     }
76 76
 
77 77
     /**
@@ -207,7 +207,7 @@ public class OutputQueue {
207 207
         if (queueEnabled && priority != QueuePriority.IMMEDIATE) {
208 208
             synchronized (queueHandlerLock) {
209 209
                 if (queueHandler == null || !queueHandler.isAlive()) {
210
-                    queueHandler = queueHandlerFactory.getQueueHandler(this, out);
210
+                    setQueueHandler(queueHandlerFactory.getQueueHandler(this, out));
211 211
                     queueHandler.start();
212 212
                 }
213 213
 
@@ -217,4 +217,19 @@ public class OutputQueue {
217 217
             out.printf("%s\r\n", line);
218 218
         }
219 219
     }
220
+
221
+    /**
222
+     * Sets the hanlder that this queue will use. This will cause the existing {@link #queue}
223
+     * to be replaced with a new version with an updated comparator.
224
+     *
225
+     * @param queueHandler The new queue handler to use.
226
+     */
227
+    private void setQueueHandler(final QueueHandler queueHandler) {
228
+        this.queueHandler = queueHandler;
229
+        final BlockingQueue<QueueItem> newQueue = new PriorityBlockingQueue<>(
230
+                10, queueHandler.getQueueItemComparator());
231
+        newQueue.addAll(queue);
232
+        queue = newQueue;
233
+    }
234
+
220 235
 }

+ 7
- 15
irc/src/com/dmdirc/parser/irc/outputqueue/QueueHandler.java View File

@@ -25,14 +25,12 @@ package com.dmdirc.parser.irc.outputqueue;
25 25
 import com.dmdirc.parser.common.QueuePriority;
26 26
 
27 27
 import java.io.PrintWriter;
28
-import java.time.LocalDateTime;
29
-import java.time.temporal.ChronoUnit;
30 28
 import java.util.Comparator;
31 29
 
32 30
 /**
33 31
  * Sending queue.
34 32
  */
35
-public abstract class QueueHandler extends Thread implements Comparator<QueueItem> {
33
+public abstract class QueueHandler extends Thread {
36 34
 
37 35
     /** The output queue that owns us. */
38 36
     protected OutputQueue outputQueue;
@@ -80,23 +78,16 @@ public abstract class QueueHandler extends Thread implements Comparator<QueueIte
80 78
      * @return A QueueItem for teh given parameters
81 79
      */
82 80
     public QueueItem getQueueItem(final String line, final QueuePriority priority) {
83
-        return new QueueItem(this, line, priority);
84
-    }
85
-
86
-    @Override
87
-    public int compare(final QueueItem mainObject, final QueueItem otherObject) {
88
-        return comparator.compare(mainObject, otherObject);
81
+        return new QueueItem(line, priority);
89 82
     }
90 83
 
91 84
     /**
92
-     * Determines whether the given item has been starved (i.e., it has sat waiting in the queue
93
-     * for too long due to higher priority items).
85
+     * Gets the comparator to use to sort queue items.
94 86
      *
95
-     * @param item The item to check
96
-     * @return True if the item has been queued for longer than 10 seconds, false otherwise
87
+     * @return The comparator to use to sort queue items.
97 88
      */
98
-    private static boolean isStarved(final QueueItem item) {
99
-        return item.getTime().isBefore(LocalDateTime.now().minus(10, ChronoUnit.SECONDS));
89
+    public Comparator<QueueItem> getQueueItemComparator() {
90
+        return comparator;
100 91
     }
101 92
 
102 93
     /**
@@ -108,4 +99,5 @@ public abstract class QueueHandler extends Thread implements Comparator<QueueIte
108 99
      */
109 100
     @Override
110 101
     public abstract void run();
102
+
111 103
 }

+ 6
- 22
irc/src/com/dmdirc/parser/irc/outputqueue/QueueItem.java View File

@@ -30,7 +30,7 @@ import java.time.LocalDateTime;
30 30
 /**
31 31
  * Queued Item.
32 32
  */
33
-public class QueueItem implements Comparable<QueueItem> {
33
+public class QueueItem {
34 34
 
35 35
     /** Global Item Number. */
36 36
     private static long number;
@@ -42,30 +42,25 @@ public class QueueItem implements Comparable<QueueItem> {
42 42
     private final long itemNumber;
43 43
     /** What is the priority of this line? */
44 44
     private final QueuePriority priority;
45
-    /** Our handler. */
46
-    private final QueueHandler handler;
47 45
 
48 46
     /**
49 47
      * Create a new QueueItem.
50 48
      *
51
-     * @param handler Handler for this QueueItem
52 49
      * @param line Line to send
53 50
      * @param priority Priority for the queue item
54 51
      */
55
-    public QueueItem(final QueueHandler handler, final String line, final QueuePriority priority) {
56
-        this(handler, Clock.systemDefaultZone(), line, priority);
52
+    public QueueItem(final String line, final QueuePriority priority) {
53
+        this(Clock.systemDefaultZone(), line, priority);
57 54
     }
58 55
 
59 56
     /**
60 57
      * Create a new QueueItem.
61 58
      *
62
-     * @param handler Handler for this QueueItem
63 59
      * @param line Line to send
64 60
      * @param priority Priority for the queue item
65 61
      */
66
-    public QueueItem(final QueueHandler handler, final Clock clock, final String line,
62
+    public QueueItem(final Clock clock, final String line,
67 63
             final QueuePriority priority) {
68
-        this.handler = handler;
69 64
         this.line = line;
70 65
         this.priority = priority;
71 66
 
@@ -109,20 +104,9 @@ public class QueueItem implements Comparable<QueueItem> {
109 104
         return priority;
110 105
     }
111 106
 
112
-    /**
113
-     * Compare objects.
114
-     * This will use the compareQueueItem method of the current QueueHandler.
115
-     *
116
-     * @param o Object to compare to
117
-     * @return Position of this item in reference to the given item.
118
-     */
119
-    @Override
120
-    public int compareTo(final QueueItem o) {
121
-        return handler.compare(this, o);
122
-    }
123
-
124 107
     @Override
125 108
     public String toString() {
126
-        return String.format("[%s %d] %s", priority, time, line);
109
+        return String.format("[%s %s] %s", priority, time, line);
127 110
     }
111
+
128 112
 }

+ 5
- 5
irc/test/com/dmdirc/parser/irc/outputqueue/QueueComparatorsTest.java View File

@@ -45,19 +45,19 @@ public class QueueComparatorsTest {
45 45
     private static final Clock NOW_MINUS_TEN = Clock.fixed(Instant.ofEpochMilli(90 * 1000),
46 46
             ZoneId.systemDefault());
47 47
 
48
-    private final QueueItem currentLowPriority1 = new QueueItem(null, NOW_MINUS_FIVE,
48
+    private final QueueItem currentLowPriority1 = new QueueItem(NOW_MINUS_FIVE,
49 49
             "", QueuePriority.LOW);
50 50
 
51
-    private final QueueItem currentLowPriority2 = new QueueItem(null, NOW_MINUS_FIVE,
51
+    private final QueueItem currentLowPriority2 = new QueueItem(NOW_MINUS_FIVE,
52 52
             "", QueuePriority.LOW);
53 53
 
54
-    private final QueueItem currentNormalPriority = new QueueItem(null, NOW_MINUS_FIVE,
54
+    private final QueueItem currentNormalPriority = new QueueItem(NOW_MINUS_FIVE,
55 55
             "", QueuePriority.NORMAL);
56 56
 
57
-    private final QueueItem currentHighPriority = new QueueItem(null, NOW_MINUS_FIVE,
57
+    private final QueueItem currentHighPriority = new QueueItem(NOW_MINUS_FIVE,
58 58
             "", QueuePriority.HIGH);
59 59
 
60
-    private final QueueItem oldLowPriority = new QueueItem(null, NOW_MINUS_TEN,
60
+    private final QueueItem oldLowPriority = new QueueItem(NOW_MINUS_TEN,
61 61
             "", QueuePriority.LOW);
62 62
 
63 63
     @Test

Loading…
Cancel
Save