From 5f8491f5f7c034508aae1aa989077fec4e1b1ec2 Mon Sep 17 00:00:00 2001
From: Alexey Kudravtsev <cdr@intellij.com>
Date: Wed, 6 Apr 2016 14:41:48 +0300
Subject: [PATCH] simplify implementation, make iterator know about underlying
 queue modification immediately

---
 .../util/containers/HashSetQueue.java         | 58 +++++++++----------
 .../util/containers/HashSetQueueTest.java     | 14 +++++
 2 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/platform/util/src/com/intellij/util/containers/HashSetQueue.java b/platform/util/src/com/intellij/util/containers/HashSetQueue.java
index 5092f1005cd0..dd60695421e2 100644
--- a/platform/util/src/com/intellij/util/containers/HashSetQueue.java
+++ b/platform/util/src/com/intellij/util/containers/HashSetQueue.java
@@ -23,17 +23,24 @@ import java.util.NoSuchElementException;
 import java.util.Queue;
 
 /**
- * Unbounded non-thread-safe {@link Queue} implementation backed by {@link gnu.trove.THashSet}.
+ * Unbounded non-thread-safe {@link Queue} with fast add/remove/contains.<br>
  * Differs from the conventional Queue by:<ul>
  * <li>The new method {@link #find(T)} which finds the queue element equivalent to its parameter in O(1) avg. time</li>
  * <li>The {@link #contains(Object)} method is O(1)</li>
  * <li>The {@link #remove(Object)} method is O(1)</li>
  * </ul>
+ * Implementation is backed by {@link gnu.trove.THashSet} containing double-linked QueueEntry nodes holding elements themselves.
  */
 public class HashSetQueue<T> extends AbstractCollection<T> implements Queue<T> {
   private final OpenTHashSet<QueueEntry<T>> set = new OpenTHashSet<QueueEntry<T>>();
-  private QueueEntry<T> last;
-  private QueueEntry<T> first;
+  // Entries in the queue are double-linked circularly, the TOMB serving as a sentinel.
+  // TOMB.next is the first entry; TOMB.prev is the last entry;
+  // TOMB.next == TOMB.prev == TOMB means the queue is empty
+  private final QueueEntry<T> TOMB = new QueueEntry<T>(cast(new Object()));
+
+  public HashSetQueue() {
+    TOMB.next = TOMB.prev = TOMB;
+  }
 
   private static class QueueEntry<T> {
     @NotNull private final T t;
@@ -65,15 +72,12 @@ public class HashSetQueue<T> extends AbstractCollection<T> implements Queue<T> {
     QueueEntry<T> newLast = new QueueEntry<T>(t);
     boolean added = set.add(newLast);
     if (!added) return false;
-    if (last == null) {
-      last = newLast;
-      first = newLast;
-    }
-    else {
-      last.next = newLast;
-      newLast.prev = last;
-      last = newLast;
-    }
+    QueueEntry<T> oldLast = TOMB.prev;
+
+    oldLast.next = newLast;
+    newLast.prev = oldLast;
+    newLast.next = TOMB;
+    TOMB.prev = newLast;
 
     return true;
   }
@@ -105,7 +109,7 @@ public class HashSetQueue<T> extends AbstractCollection<T> implements Queue<T> {
 
   @Override
   public T peek() {
-    return first == null ? null : first.t;
+    return TOMB.next == TOMB ? null : TOMB.next.t;
   }
 
   public T find(@NotNull T t) {
@@ -124,18 +128,10 @@ public class HashSetQueue<T> extends AbstractCollection<T> implements Queue<T> {
     if (entry == null) return false;
     QueueEntry<T> prev = entry.prev;
     QueueEntry<T> next = entry.next;
-    if (prev != null) {
-      prev.next = next;
-    }
-    else {
-      first = next;
-    }
-    if (next != null) {
-      next.prev = prev;
-    }
-    else {
-      last = prev;
-    }
+
+    prev.next = next;
+    next.prev = prev;
+
     set.remove(entry);
     return true;
   }
@@ -159,24 +155,22 @@ public class HashSetQueue<T> extends AbstractCollection<T> implements Queue<T> {
   @Override
   public Iterator<T> iterator() {
     return new Iterator<T>() {
-      QueueEntry<T> cursor = first;
+      private QueueEntry<T> cursor = TOMB;
       @Override
       public boolean hasNext() {
-        return cursor != null;
+        return cursor.next != TOMB;
       }
 
       @Override
       public T next() {
-        QueueEntry<T> entry = cursor;
         cursor = cursor.next;
-        return entry.t;
+        return cursor.t;
       }
 
       @Override
       public void remove() {
-        QueueEntry<T> toDelete = cursor == null ? last : cursor.prev;
-        if (toDelete == null) throw new NoSuchElementException();
-        HashSetQueue.this.remove(toDelete.t);
+        if (cursor == TOMB) throw new NoSuchElementException();
+        HashSetQueue.this.remove(cursor.t);
       }
     };
   }
diff --git a/platform/util/testSrc/com/intellij/util/containers/HashSetQueueTest.java b/platform/util/testSrc/com/intellij/util/containers/HashSetQueueTest.java
index a741d219ff54..3997e2389bc1 100644
--- a/platform/util/testSrc/com/intellij/util/containers/HashSetQueueTest.java
+++ b/platform/util/testSrc/com/intellij/util/containers/HashSetQueueTest.java
@@ -21,6 +21,7 @@ import gnu.trove.PrimeFinder;
 import junit.framework.TestCase;
 
 import java.util.ArrayList;
+import java.util.Iterator;
 
 public class HashSetQueueTest extends TestCase {
   private final Assertion CHECK = new Assertion();
@@ -114,4 +115,17 @@ public class HashSetQueueTest extends TestCase {
       toRemove = (toRemove + delta) % N;
     }
   }
+
+  public void testIteratorCatchesUpQueueModificationImmediately() {
+    assertTrue(myQueue.add("1"));
+    Iterator<String> iterator = myQueue.iterator();
+    assertTrue(iterator.hasNext());
+    assertEquals("1", iterator.next());
+    assertFalse(iterator.hasNext());
+
+    myQueue.add("2");
+    assertTrue(iterator.hasNext());
+    assertEquals("2", iterator.next());
+    assertFalse(iterator.hasNext());
+  }
 }