6897553: LinkedList performance improvements

Summary: LinkedList of size N creates N+1 instead of N+2 objects.  Comparing against null is faster than comparing against sentinel node
Reviewed-by: dl, jjb, forax

src/share/classes/java/util/LinkedList.java

@@ -26,22 +26,22 @@
 package java.util;
 
 /**
- * Linked list implementation of the <tt>List</tt> interface.  Implements all
+ * Linked list implementation of the {@code List} interface.  Implements all
  * optional list operations, and permits all elements (including
- * <tt>null</tt>).  In addition to implementing the <tt>List</tt> interface,
- * the <tt>LinkedList</tt> class provides uniformly named methods to
- * <tt>get</tt>, <tt>remove</tt> and <tt>insert</tt> an element at the
+ * {@code null}).  In addition to implementing the {@code List} interface,
+ * the {@code LinkedList} class provides uniformly named methods to
+ * {@code get}, {@code remove} and {@code insert} an element at the
  * beginning and end of the list.  These operations allow linked lists to be
  * used as a stack, {@linkplain Queue queue}, or {@linkplain Deque
- * double-ended queue}. <p>
+ * double-ended queue}.
  *
- * The class implements the <tt>Deque</tt> interface, providing
- * first-in-first-out queue operations for <tt>add</tt>,
- * <tt>poll</tt>, along with other stack and deque operations.<p>
+ * <p>The class implements the {@code Deque} interface, providing
+ * first-in-first-out queue operations for {@code add},
+ * {@code poll}, along with other stack and deque operations.
  *
- * All of the operations perform as could be expected for a doubly-linked
+ * <p>All of the operations perform as could be expected for a doubly-linked
  * list.  Operations that index into the list will traverse the list from
- * the beginning or the end, whichever is closer to the specified index.<p>
+ * the beginning or the end, whichever is closer to the specified index.
  *
  * <p><strong>Note that this implementation is not synchronized.</strong>
  * If multiple threads access a linked list concurrently, and at least
@@ -58,11 +58,11 @@ package java.util;
  * unsynchronized access to the list:<pre>
  *   List list = Collections.synchronizedList(new LinkedList(...));</pre>
  *
- * <p>The iterators returned by this class's <tt>iterator</tt> and
- * <tt>listIterator</tt> methods are <i>fail-fast</i>: if the list is
+ * <p>The iterators returned by this class's {@code iterator} and
+ * {@code listIterator} methods are <i>fail-fast</i>: if the list is
  * structurally modified at any time after the iterator is created, in
- * any way except through the Iterator's own <tt>remove</tt> or
- * <tt>add</tt> methods, the iterator will throw a {@link
+ * any way except through the Iterator's own {@code remove} or
+ * {@code add} methods, the iterator will throw a {@link
  * ConcurrentModificationException}.  Thus, in the face of concurrent
  * modification, the iterator fails quickly and cleanly, rather than
  * risking arbitrary, non-deterministic behavior at an undetermined
@@ -71,7 +71,7 @@ package java.util;
  * <p>Note that the fail-fast behavior of an iterator cannot be guaranteed
  * as it is, generally speaking, impossible to make any hard guarantees in the
  * presence of unsynchronized concurrent modification.  Fail-fast iterators
- * throw <tt>ConcurrentModificationException</tt> on a best-effort basis.
+ * throw {@code ConcurrentModificationException} on a best-effort basis.
  * Therefore, it would be wrong to write a program that depended on this
  * exception for its correctness:   <i>the fail-fast behavior of iterators
  * should be used only to detect bugs.</i>
@@ -83,7 +83,6 @@ package java.util;
  * @author  Josh Bloch
  * @see     List
  * @see     ArrayList
- * @see     Vector
  * @since 1.2
  * @param <E> the type of elements held in this collection
  */
@@ -92,14 +91,26 @@ public class LinkedList<E>
     extends AbstractSequentialList<E>
     implements List<E>, Deque<E>, Cloneable, java.io.Serializable
 {
-    private transient Entry<E> header = new Entry<E>(null, null, null);
-    private transient int size = 0;
+    transient int size = 0;
+
+    /**
+     * Pointer to first node.
+     * Invariant: (first == null && last == null) ||
+     *            (first.prev == null && first.item != null)
+     */
+    transient Node<E> first;
+
+    /**
+     * Pointer to last node.
+     * Invariant: (first == null && last == null) ||
+     *            (last.next == null && last.item != null)
+     */
+    transient Node<E> last;
 
     /**
      * Constructs an empty list.
      */
     public LinkedList() {
-        header.next = header.previous = header;
     }
 
     /**
@@ -115,6 +126,119 @@ public class LinkedList<E>
         addAll(c);
     }
 
+    /**
+     * Links e as first element.
+     */
+    private void linkFirst(E e) {
+        final Node<E> f = first;
+        final Node<E> newNode = new Node<E>(null, e, f);
+        first = newNode;
+        if (f == null)
+            last = newNode;
+        else
+            f.prev = newNode;
+        size++;
+        modCount++;
+    }
+
+    /**
+     * Links e as last element.
+     */
+    void linkLast(E e) {
+        final Node<E> l = last;
+        final Node<E> newNode = new Node<E>(l, e, null);
+        last = newNode;
+        if (l == null)
+            first = newNode;
+        else
+            l.next = newNode;
+        size++;
+        modCount++;
+    }
+
+    /**
+     * Inserts element e before non-null Node succ.
+     */
+    void linkBefore(E e, Node<E> succ) {
+        // assert succ != null;
+        final Node<E> pred = succ.prev;
+        final Node<E> newNode = new Node<E>(pred, e, succ);
+        succ.prev = newNode;
+        if (pred == null)
+            first = newNode;
+        else
+            pred.next = newNode;
+        size++;
+        modCount++;
+    }
+
+    /**
+     * Unlinks non-null first node f.
+     */
+    private E unlinkFirst(Node<E> f) {
+        // assert f == first && f != null;
+        final E element = f.item;
+        final Node<E> next = f.next;
+        f.item = null;
+        f.next = null; // help GC
+        first = next;
+        if (next == null)
+            last = null;
+        else
+            next.prev = null;
+        size--;
+        modCount++;
+        return element;
+    }
+
+    /**
+     * Unlinks non-null last node l.
+     */
+    private E unlinkLast(Node<E> l) {
+        // assert l == last && l != null;
+        final E element = l.item;
+        final Node<E> prev = l.prev;
+        l.item = null;
+        l.prev = null; // help GC
+        last = prev;
+        if (prev == null)
+            first = null;
+        else
+            prev.next = null;
+        size--;
+        modCount++;
+        return element;
+    }
+
+    /**
+     * Unlinks non-null node x.
+     */
+    E unlink(Node<E> x) {
+        // assert x != null;
+        final E element = x.item;
+        final Node<E> next = x.next;
+        final Node<E> prev = x.prev;
+
+        if (prev == null) {
+            first = next;
+        } else {
+            prev.next = next;
+            x.prev = null;
+        }
+
+        if (next == null) {
+            last = prev;
+        } else {
+            next.prev = prev;
+            x.next = null;
+        }
+
+        x.item = null;
+        size--;
+        modCount++;
+        return element;
+    }
+
     /**
      * Returns the first element in this list.
      *
@@ -122,10 +246,10 @@ public class LinkedList<E>
      * @throws NoSuchElementException if this list is empty
      */
     public E getFirst() {
-        if (size==0)
+        final Node<E> f = first;
+        if (f == null)
             throw new NoSuchElementException();
-
-        return header.next.element;
+        return f.item;
     }
 
     /**
@@ -135,10 +259,10 @@ public class LinkedList<E>
      * @throws NoSuchElementException if this list is empty
      */
     public E getLast()  {
-        if (size==0)
+        final Node<E> l = last;
+        if (l == null)
             throw new NoSuchElementException();
-
-        return header.previous.element;
+        return l.item;
     }
 
     /**
@@ -148,7 +272,10 @@ public class LinkedList<E>
      * @throws NoSuchElementException if this list is empty
      */
     public E removeFirst() {
-        return remove(header.next);
+        final Node<E> f = first;
+        if (f == null)
+            throw new NoSuchElementException();
+        return unlinkFirst(f);
     }
 
     /**
@@ -158,7 +285,10 @@ public class LinkedList<E>
      * @throws NoSuchElementException if this list is empty
      */
     public E removeLast() {
-        return remove(header.previous);
+        final Node<E> l = last;
+        if (l == null)
+            throw new NoSuchElementException();
+        return unlinkLast(l);
     }
 
     /**
@@ -167,7 +297,7 @@ public class LinkedList<E>
      * @param e the element to add
      */
     public void addFirst(E e) {
-        addBefore(e, header.next);
+        linkFirst(e);
     }
 
     /**
@@ -178,17 +308,17 @@ public class LinkedList<E>
      * @param e the element to add
      */
     public void addLast(E e) {
-        addBefore(e, header);
+        linkLast(e);
     }
 
     /**
-     * Returns <tt>true</tt> if this list contains the specified element.
-     * More formally, returns <tt>true</tt> if and only if this list contains
-     * at least one element <tt>e</tt> such that
+     * Returns {@code true} if this list contains the specified element.
+     * More formally, returns {@code true} if and only if this list contains
+     * at least one element {@code e} such that
      * <tt>(o==null&nbsp;?&nbsp;e==null&nbsp;:&nbsp;o.equals(e))</tt>.
      *
      * @param o element whose presence in this list is to be tested
-     * @return <tt>true</tt> if this list contains the specified element
+     * @return {@code true} if this list contains the specified element
      */
     public boolean contains(Object o) {
         return indexOf(o) != -1;
@@ -209,10 +339,10 @@ public class LinkedList<E>
      * <p>This method is equivalent to {@link #addLast}.
      *
      * @param e element to be appended to this list
-     * @return <tt>true</tt> (as specified by {@link Collection#add})
+     * @return {@code true} (as specified by {@link Collection#add})
      */
     public boolean add(E e) {
-        addBefore(e, header);
+        linkLast(e);
         return true;
     }
 
@@ -220,27 +350,27 @@ public class LinkedList<E>
      * Removes the first occurrence of the specified element from this list,
      * if it is present.  If this list does not contain the element, it is
      * unchanged.  More formally, removes the element with the lowest index
-     * <tt>i</tt> such that
+     * {@code i} such that
      * <tt>(o==null&nbsp;?&nbsp;get(i)==null&nbsp;:&nbsp;o.equals(get(i)))</tt>
-     * (if such an element exists).  Returns <tt>true</tt> if this list
+     * (if such an element exists).  Returns {@code true} if this list
      * contained the specified element (or equivalently, if this list
      * changed as a result of the call).
      *
      * @param o element to be removed from this list, if present
-     * @return <tt>true</tt> if this list contained the specified element
+     * @return {@code true} if this list contained the specified element
      */
     public boolean remove(Object o) {
-        if (o==null) {
-            for (Entry<E> e = header.next; e != header; e = e.next) {
-                if (e.element==null) {
-                    remove(e);
+        if (o == null) {
+            for (Node<E> x = first; x != null; x = x.next) {
+                if (x.item == null) {
+                    unlink(x);
                     return true;
                 }
             }
         } else {
-            for (Entry<E> e = header.next; e != header; e = e.next) {
-                if (o.equals(e.element)) {
-                    remove(e);
+            for (Node<E> x = first; x != null; x = x.next) {
+                if (o.equals(x.item)) {
+                    unlink(x);
                     return true;
                 }
             }
@@ -257,7 +387,7 @@ public class LinkedList<E>
      * this list, and it's nonempty.)
      *
      * @param c collection containing elements to be added to this list
-     * @return <tt>true</tt> if this list changed as a result of the call
+     * @return {@code true} if this list changed as a result of the call
      * @throws NullPointerException if the specified collection is null
      */
     public boolean addAll(Collection<? extends E> c) {
@@ -275,45 +405,66 @@ public class LinkedList<E>
      * @param index index at which to insert the first element
      *              from the specified collection
      * @param c collection containing elements to be added to this list
-     * @return <tt>true</tt> if this list changed as a result of the call
+     * @return {@code true} if this list changed as a result of the call
      * @throws IndexOutOfBoundsException {@inheritDoc}
      * @throws NullPointerException if the specified collection is null
      */
     public boolean addAll(int index, Collection<? extends E> c) {
-        if (index < 0 || index > size)
-            throw new IndexOutOfBoundsException("Index: "+index+
-                                                ", Size: "+size);
+        checkPositionIndex(index);
+
         Object[] a = c.toArray();
         int numNew = a.length;
-        if (numNew==0)
+        if (numNew == 0)
             return false;
-        modCount++;
 
-        Entry<E> successor = (index==size ? header : entry(index));
-        Entry<E> predecessor = successor.previous;
-        for (int i=0; i<numNew; i++) {
-            Entry<E> e = new Entry<E>((E)a[i], successor, predecessor);
-            predecessor.next = e;
-            predecessor = e;
+        Node<E> pred, succ;
+        if (index == size) {
+            succ = null;
+            pred = last;
+        } else {
+            succ = node(index);
+            pred = succ.prev;
+        }
+
+        for (Object o : a) {
+            @SuppressWarnings("unchecked") E e = (E) o;
+            Node<E> newNode = new Node<E>(pred, e, null);
+            if (pred == null)
+                first = newNode;
+            else
+                pred.next = newNode;
+            pred = newNode;
+        }
+
+        if (succ == null) {
+            last = pred;
+        } else {
+            pred.next = succ;
+            succ.prev = pred;
         }
-        successor.previous = predecessor;
 
         size += numNew;
+        modCount++;
         return true;
     }
 
     /**
      * Removes all of the elements from this list.
+     * The list will be empty after this call returns.
      */
     public void clear() {
-        Entry<E> e = header.next;
-        while (e != header) {
-            Entry<E> next = e.next;
-            e.next = e.previous = null;
-            e.element = null;
-            e = next;
-        }
-        header.next = header.previous = header;
+        // Clearing all of the links between nodes is "unnecessary", but:
+        // - helps a generational GC if the discarded nodes inhabit
+        //   more than one generation
+        // - is sure to free memory even if there is a reachable Iterator
+        for (Node<E> x = first; x != null; ) {
+            Node<E> next = x.next;
+            x.item = null;
+            x.next = null;
+            x.prev = null;
+            x = next;
+        }
+        first = last = null;
         size = 0;
         modCount++;
     }
@@ -329,7 +480,8 @@ public class LinkedList<E>
      * @throws IndexOutOfBoundsException {@inheritDoc}
      */
     public E get(int index) {
-        return entry(index).element;
+        checkElementIndex(index);
+        return node(index).item;
     }
 
     /**
@@ -342,9 +494,10 @@ public class LinkedList<E>
      * @throws IndexOutOfBoundsException {@inheritDoc}
      */
     public E set(int index, E element) {
-        Entry<E> e = entry(index);
-        E oldVal = e.element;
-        e.element = element;
+        checkElementIndex(index);
+        Node<E> x = node(index);
+        E oldVal = x.item;
+        x.item = element;
         return oldVal;
     }
 
@@ -358,7 +511,12 @@ public class LinkedList<E>
      * @throws IndexOutOfBoundsException {@inheritDoc}
      */
     public void add(int index, E element) {
-        addBefore(element, (index==size ? header : entry(index)));
+        checkPositionIndex(index);
+
+        if (index == size)
+            linkLast(element);
+        else
+            linkBefore(element, node(index));
     }
 
     /**
@@ -371,34 +529,69 @@ public class LinkedList<E>
      * @throws IndexOutOfBoundsException {@inheritDoc}
      */
     public E remove(int index) {
-        return remove(entry(index));
+        checkElementIndex(index);
+        return unlink(node(index));
+    }
+
+    /**
+     * Tells if the argument is the index of an existing element.
+     */
+    private boolean isElementIndex(int index) {
+        return index >= 0 && index < size;
+    }
+
+    /**
+     * Tells if the argument is the index of a valid position for an
+     * iterator or an add operation.
+     */
+    private boolean isPositionIndex(int index) {
+        return index >= 0 && index <= size;
+    }
+
+    /**
+     * Constructs an IndexOutOfBoundsException detail message.
+     * Of the many possible refactorings of the error handling code,
+     * this "outlining" performs best with both server and client VMs.
+     */
+    private String outOfBoundsMsg(int index) {
+        return "Index: "+index+", Size: "+size;
+    }
+
+    private void checkElementIndex(int index) {
+        if (!isElementIndex(index))
+            throw new IndexOutOfBoundsException(outOfBoundsMsg(index));
+    }
+
+    private void checkPositionIndex(int index) {
+        if (!isPositionIndex(index))
+            throw new IndexOutOfBoundsException(outOfBoundsMsg(index));
     }
 
     /**
-     * Returns the indexed entry.
+     * Returns the (non-null) Node at the specified element index.
      */
-    private Entry<E> entry(int index) {
-        if (index < 0 || index >= size)
-            throw new IndexOutOfBoundsException("Index: "+index+
-                                                ", Size: "+size);
-        Entry<E> e = header;
+    Node<E> node(int index) {
+        // assert isElementIndex(index);
+
         if (index < (size >> 1)) {
-            for (int i = 0; i <= index; i++)
-                e = e.next;
+            Node<E> x = first;
+            for (int i = 0; i < index; i++)
+                x = x.next;
+            return x;
         } else {
-            for (int i = size; i > index; i--)
-                e = e.previous;
+            Node<E> x = last;
+            for (int i = size - 1; i > index; i--)
+                x = x.prev;
+            return x;
         }
-        return e;
     }
 
-
     // Search Operations
 
     /**
      * Returns the index of the first occurrence of the specified element
      * in this list, or -1 if this list does not contain the element.
-     * More formally, returns the lowest index <tt>i</tt> such that
+     * More formally, returns the lowest index {@code i} such that
      * <tt>(o==null&nbsp;?&nbsp;get(i)==null&nbsp;:&nbsp;o.equals(get(i)))</tt>,
      * or -1 if there is no such index.
      *
@@ -408,15 +601,15 @@ public class LinkedList<E>
      */
     public int indexOf(Object o) {
         int index = 0;
-        if (o==null) {
-            for (Entry e = header.next; e != header; e = e.next) {
-                if (e.element==null)
+        if (o == null) {
+            for (Node<E> x = first; x != null; x = x.next) {
+                if (x.item == null)
                     return index;
                 index++;
             }
         } else {
-            for (Entry e = header.next; e != header; e = e.next) {
-                if (o.equals(e.element))
+            for (Node<E> x = first; x != null; x = x.next) {
+                if (o.equals(x.item))
                     return index;
                 index++;
             }
@@ -427,7 +620,7 @@ public class LinkedList<E>
     /**
      * Returns the index of the last occurrence of the specified element
      * in this list, or -1 if this list does not contain the element.
-     * More formally, returns the highest index <tt>i</tt> such that
+     * More formally, returns the highest index {@code i} such that
      * <tt>(o==null&nbsp;?&nbsp;get(i)==null&nbsp;:&nbsp;o.equals(get(i)))</tt>,
      * or -1 if there is no such index.
      *
@@ -437,16 +630,16 @@ public class LinkedList<E>
      */
     public int lastIndexOf(Object o) {
         int index = size;
-        if (o==null) {
-            for (Entry e = header.previous; e != header; e = e.previous) {
+        if (o == null) {
+            for (Node<E> x = last; x != null; x = x.prev) {
                 index--;
-                if (e.element==null)
+                if (x.item == null)
                     return index;
             }
         } else {
-            for (Entry e = header.previous; e != header; e = e.previous) {
+            for (Node<E> x = last; x != null; x = x.prev) {
                 index--;
-                if (o.equals(e.element))
+                if (o.equals(x.item))
                     return index;
             }
         }
@@ -457,17 +650,18 @@ public class LinkedList<E>
 
     /**
      * Retrieves, but does not remove, the head (first element) of this list.
-     * @return the head of this list, or <tt>null</tt> if this list is empty
+     *
+     * @return the head of this list, or {@code null} if this list is empty
      * @since 1.5
      */
     public E peek() {
-        if (size==0)
-            return null;
-        return getFirst();
+        final Node<E> f = first;
+        return (f == null) ? null : f.item;
     }
 
     /**
      * Retrieves, but does not remove, the head (first element) of this list.
+     *
      * @return the head of this list
      * @throws NoSuchElementException if this list is empty
      * @since 1.5
@@ -477,14 +671,14 @@ public class LinkedList<E>
     }
 
     /**
-     * Retrieves and removes the head (first element) of this list
-     * @return the head of this list, or <tt>null</tt> if this list is empty
+     * Retrieves and removes the head (first element) of this list.
+     *
+     * @return the head of this list, or {@code null} if this list is empty
      * @since 1.5
      */
     public E poll() {
-        if (size==0)
-            return null;
-        return removeFirst();
+        final Node<E> f = first;
+        return (f == null) ? null : unlinkFirst(f);
     }
 
     /**
@@ -502,7 +696,7 @@ public class LinkedList<E>
      * Adds the specified element as the tail (last element) of this list.
      *
      * @param e the element to add
-     * @return <tt>true</tt> (as specified by {@link Queue#offer})
+     * @return {@code true} (as specified by {@link Queue#offer})
      * @since 1.5
      */
     public boolean offer(E e) {
@@ -514,7 +708,7 @@ public class LinkedList<E>
      * Inserts the specified element at the front of this list.
      *
      * @param e the element to insert
-     * @return <tt>true</tt> (as specified by {@link Deque#offerFirst})
+     * @return {@code true} (as specified by {@link Deque#offerFirst})
      * @since 1.6
      */
     public boolean offerFirst(E e) {
@@ -526,7 +720,7 @@ public class LinkedList<E>
      * Inserts the specified element at the end of this list.
      *
      * @param e the element to insert
-     * @return <tt>true</tt> (as specified by {@link Deque#offerLast})
+     * @return {@code true} (as specified by {@link Deque#offerLast})
      * @since 1.6
      */
     public boolean offerLast(E e) {
@@ -536,58 +730,54 @@ public class LinkedList<E>
 
     /**
      * Retrieves, but does not remove, the first element of this list,
-     * or returns <tt>null</tt> if this list is empty.
+     * or returns {@code null} if this list is empty.
      *
-     * @return the first element of this list, or <tt>null</tt>
+     * @return the first element of this list, or {@code null}
      *         if this list is empty
      * @since 1.6
      */
     public E peekFirst() {
-        if (size==0)
-            return null;
-        return getFirst();
+        final Node<E> f = first;
+        return (f == null) ? null : f.item;
      }
 
     /**
      * Retrieves, but does not remove, the last element of this list,
-     * or returns <tt>null</tt> if this list is empty.
+     * or returns {@code null} if this list is empty.
      *
-     * @return the last element of this list, or <tt>null</tt>
+     * @return the last element of this list, or {@code null}
      *         if this list is empty
      * @since 1.6
      */
     public E peekLast() {
-        if (size==0)
-            return null;
-        return getLast();
+        final Node<E> l = last;
+        return (l == null) ? null : l.item;
     }
 
     /**
      * Retrieves and removes the first element of this list,
-     * or returns <tt>null</tt> if this list is empty.
+     * or returns {@code null} if this list is empty.
      *
-     * @return the first element of this list, or <tt>null</tt> if
+     * @return the first element of this list, or {@code null} if
      *     this list is empty
      * @since 1.6
      */
     public E pollFirst() {
-        if (size==0)
-            return null;
-        return removeFirst();
+        final Node<E> f = first;
+        return (f == null) ? null : unlinkFirst(f);
     }
 
     /**
      * Retrieves and removes the last element of this list,
-     * or returns <tt>null</tt> if this list is empty.
+     * or returns {@code null} if this list is empty.
      *
-     * @return the last element of this list, or <tt>null</tt> if
+     * @return the last element of this list, or {@code null} if
      *     this list is empty
      * @since 1.6
      */
     public E pollLast() {
-        if (size==0)
-            return null;
-        return removeLast();
+        final Node<E> l = last;
+        return (l == null) ? null : unlinkLast(l);
     }
 
     /**
@@ -624,7 +814,7 @@ public class LinkedList<E>
      * does not contain the element, it is unchanged.
      *
      * @param o element to be removed from this list, if present
-     * @return <tt>true</tt> if the list contained the specified element
+     * @return {@code true} if the list contained the specified element
      * @since 1.6
      */
     public boolean removeFirstOccurrence(Object o) {
@@ -637,21 +827,21 @@ public class LinkedList<E>
      * does not contain the element, it is unchanged.
      *
      * @param o element to be removed from this list, if present
-     * @return <tt>true</tt> if the list contained the specified element
+     * @return {@code true} if the list contained the specified element
      * @since 1.6
      */
     public boolean removeLastOccurrence(Object o) {
-        if (o==null) {
-            for (Entry<E> e = header.previous; e != header; e = e.previous) {
-                if (e.element==null) {
-                    remove(e);
+        if (o == null) {
+            for (Node<E> x = last; x != null; x = x.prev) {
+                if (x.item == null) {
+                    unlink(x);
                     return true;
                 }
             }
         } else {
-            for (Entry<E> e = header.previous; e != header; e = e.previous) {
-                if (o.equals(e.element)) {
-                    remove(e);
+            for (Node<E> x = last; x != null; x = x.prev) {
+                if (o.equals(x.item)) {
+                    unlink(x);
                     return true;
                 }
             }
@@ -662,76 +852,68 @@ public class LinkedList<E>
     /**
      * Returns a list-iterator of the elements in this list (in proper
      * sequence), starting at the specified position in the list.
-     * Obeys the general contract of <tt>List.listIterator(int)</tt>.<p>
+     * Obeys the general contract of {@code List.listIterator(int)}.<p>
      *
      * The list-iterator is <i>fail-fast</i>: if the list is structurally
      * modified at any time after the Iterator is created, in any way except
-     * through the list-iterator's own <tt>remove</tt> or <tt>add</tt>
+     * through the list-iterator's own {@code remove} or {@code add}
      * methods, the list-iterator will throw a
-     * <tt>ConcurrentModificationException</tt>.  Thus, in the face of
+     * {@code ConcurrentModificationException}.  Thus, in the face of
      * concurrent modification, the iterator fails quickly and cleanly, rather
      * than risking arbitrary, non-deterministic behavior at an undetermined
      * time in the future.
      *
      * @param index index of the first element to be returned from the
-     *              list-iterator (by a call to <tt>next</tt>)
+     *              list-iterator (by a call to {@code next})
      * @return a ListIterator of the elements in this list (in proper
      *         sequence), starting at the specified position in the list
      * @throws IndexOutOfBoundsException {@inheritDoc}
      * @see List#listIterator(int)
      */
     public ListIterator<E> listIterator(int index) {
+        checkPositionIndex(index);
         return new ListItr(index);
     }
 
     private class ListItr implements ListIterator<E> {
-        private Entry<E> lastReturned = header;
-        private Entry<E> next;
+        private Node<E> lastReturned = null;
+        private Node<E> next;
         private int nextIndex;
         private int expectedModCount = modCount;
 
         ListItr(int index) {
-            if (index < 0 || index > size)
-                throw new IndexOutOfBoundsException("Index: "+index+
-                                                    ", Size: "+size);
-            if (index < (size >> 1)) {
-                next = header.next;
-                for (nextIndex=0; nextIndex<index; nextIndex++)
-                    next = next.next;
-            } else {
-                next = header;
-                for (nextIndex=size; nextIndex>index; nextIndex--)
-                    next = next.previous;
-            }
+            // assert isPositionIndex(index);
+            next = (index == size) ? null : node(index);
+            nextIndex = index;
         }
 
         public boolean hasNext() {
-            return nextIndex != size;
+            return nextIndex < size;
         }
 
         public E next() {
             checkForComodification();
-            if (nextIndex == size)
+            if (!hasNext())
                 throw new NoSuchElementException();
 
             lastReturned = next;
             next = next.next;
             nextIndex++;
-            return lastReturned.element;
+            return lastReturned.item;
         }
 
         public boolean hasPrevious() {
-            return nextIndex != 0;
+            return nextIndex > 0;
         }
 
         public E previous() {
-            if (nextIndex == 0)
+            checkForComodification();
+            if (!hasPrevious())
                 throw new NoSuchElementException();
 
-            lastReturned = next = next.previous;
+            lastReturned = next = (next == null) ? last : next.prev;
             nextIndex--;
-            checkForComodification();
-            return lastReturned.element;
+            return lastReturned.item;
         }
 
         public int nextIndex() {
@@ -739,36 +921,38 @@ public class LinkedList<E>
         }
 
         public int previousIndex() {
-            return nextIndex-1;
+            return nextIndex - 1;
         }
 
         public void remove() {
             checkForComodification();
-            Entry<E> lastNext = lastReturned.next;
-            try {
-                LinkedList.this.remove(lastReturned);
-            } catch (NoSuchElementException e) {
+            if (lastReturned == null)
                 throw new IllegalStateException();
-            }
-            if (next==lastReturned)
+
+            Node<E> lastNext = lastReturned.next;
+            unlink(lastReturned);
+            if (next == lastReturned)
                 next = lastNext;
             else
                 nextIndex--;
-            lastReturned = header;
+            lastReturned = null;
             expectedModCount++;
         }
 
         public void set(E e) {
-            if (lastReturned == header)
+            if (lastReturned == null)
                 throw new IllegalStateException();
             checkForComodification();
-            lastReturned.element = e;
+            lastReturned.item = e;
         }
 
         public void add(E e) {
             checkForComodification();
-            lastReturned = header;
-            addBefore(e, next);
+            lastReturned = null;
+            if (next == null)
+                linkLast(e);
+            else
+                linkBefore(e, next);
             nextIndex++;
             expectedModCount++;
         }
@@ -779,41 +963,18 @@ public class LinkedList<E>
         }
     }
 
-    private static class Entry<E> {
-        E element;
-        Entry<E> next;
-        Entry<E> previous;
+    private static class Node<E> {
+        E item;
+        Node<E> next;
+        Node<E> prev;
 
-        Entry(E element, Entry<E> next, Entry<E> previous) {
-            this.element = element;
+        Node(Node<E> prev, E element, Node<E> next) {
+            this.item = element;
             this.next = next;
-            this.previous = previous;
+            this.prev = prev;
         }
     }
 
-    private Entry<E> addBefore(E e, Entry<E> entry) {
-        Entry<E> newEntry = new Entry<E>(e, entry, entry.previous);
-        newEntry.previous.next = newEntry;
-        newEntry.next.previous = newEntry;
-        size++;
-        modCount++;
-        return newEntry;
-    }
-
-    private E remove(Entry<E> e) {
-        if (e == header)
-            throw new NoSuchElementException();
-
-        E result = e.element;
-        e.previous.next = e.next;
-        e.next.previous = e.previous;
-        e.next = e.previous = null;
-        e.element = null;
-        size--;
-        modCount++;
-        return result;
-    }
-
     /**
      * @since 1.6
      */
@@ -821,9 +982,11 @@ public class LinkedList<E>
         return new DescendingIterator();
     }
 
-    /** Adapter to provide descending iterators via ListItr.previous */
-    private class DescendingIterator implements Iterator {
-        final ListItr itr = new ListItr(size());
+    /**
+     * Adapter to provide descending iterators via ListItr.previous
+     */
+    private class DescendingIterator implements Iterator<E> {
+        private final ListItr itr = new ListItr(size());
         public boolean hasNext() {
             return itr.hasPrevious();
         }
@@ -835,29 +998,32 @@ public class LinkedList<E>
         }
     }
 
+    @SuppressWarnings("unchecked")
+    private LinkedList<E> superClone() {
+        try {
+            return (LinkedList<E>) super.clone();
+        } catch (CloneNotSupportedException e) {
+            throw new InternalError();
+        }
+    }
+
     /**
-     * Returns a shallow copy of this <tt>LinkedList</tt>. (The elements
+     * Returns a shallow copy of this {@code LinkedList}. (The elements
      * themselves are not cloned.)
      *
-     * @return a shallow copy of this <tt>LinkedList</tt> instance
+     * @return a shallow copy of this {@code LinkedList} instance
      */
     public Object clone() {
-        LinkedList<E> clone = null;
-        try {
-            clone = (LinkedList<E>) super.clone();
-        } catch (CloneNotSupportedException e) {
-            throw new InternalError();
-        }
+        LinkedList<E> clone = superClone();
 
         // Put clone into "virgin" state
-        clone.header = new Entry<E>(null, null, null);
-        clone.header.next = clone.header.previous = clone.header;
+        clone.first = clone.last = null;
         clone.size = 0;
         clone.modCount = 0;
 
         // Initialize clone with our elements
-        for (Entry<E> e = header.next; e != header; e = e.next)
-            clone.add(e.element);
+        for (Node<E> x = first; x != null; x = x.next)
+            clone.add(x.item);
 
         return clone;
     }
@@ -879,8 +1045,8 @@ public class LinkedList<E>
     public Object[] toArray() {
         Object[] result = new Object[size];
         int i = 0;
-        for (Entry<E> e = header.next; e != header; e = e.next)
-            result[i++] = e.element;
+        for (Node<E> x = first; x != null; x = x.next)
+            result[i++] = x.item;
         return result;
     }
 
@@ -894,7 +1060,7 @@ public class LinkedList<E>
      *
      * <p>If the list fits in the specified array with room to spare (i.e.,
      * the array has more elements than the list), the element in the array
-     * immediately following the end of the list is set to <tt>null</tt>.
+     * immediately following the end of the list is set to {@code null}.
      * (This is useful in determining the length of the list <i>only</i> if
      * the caller knows that the list does not contain any null elements.)
      *
@@ -903,15 +1069,15 @@ public class LinkedList<E>
      * precise control over the runtime type of the output array, and may,
      * under certain circumstances, be used to save allocation costs.
      *
-     * <p>Suppose <tt>x</tt> is a list known to contain only strings.
+     * <p>Suppose {@code x} is a list known to contain only strings.
      * The following code can be used to dump the list into a newly
-     * allocated array of <tt>String</tt>:
+     * allocated array of {@code String}:
      *
      * <pre>
      *     String[] y = x.toArray(new String[0]);</pre>
      *
-     * Note that <tt>toArray(new Object[0])</tt> is identical in function to
-     * <tt>toArray()</tt>.
+     * Note that {@code toArray(new Object[0])} is identical in function to
+     * {@code toArray()}.
      *
      * @param a the array into which the elements of the list are to
      *          be stored, if it is big enough; otherwise, a new array of the
@@ -922,14 +1088,15 @@ public class LinkedList<E>
      *         this list
      * @throws NullPointerException if the specified array is null
      */
+    @SuppressWarnings("unchecked")
     public <T> T[] toArray(T[] a) {
         if (a.length < size)
             a = (T[])java.lang.reflect.Array.newInstance(
                                 a.getClass().getComponentType(), size);
         int i = 0;
         Object[] result = a;
-        for (Entry<E> e = header.next; e != header; e = e.next)
-            result[i++] = e.element;
+        for (Node<E> x = first; x != null; x = x.next)
+            result[i++] = x.item;
 
         if (a.length > size)
             a[size] = null;
@@ -940,8 +1107,8 @@ public class LinkedList<E>
     private static final long serialVersionUID = 876323262645176354L;
 
     /**
-     * Save the state of this <tt>LinkedList</tt> instance to a stream (that
-     * is, serialize it).
+     * Saves the state of this {@code LinkedList} instance to a stream
+     * (that is, serializes it).
      *
      * @serialData The size of the list (the number of elements it
      *             contains) is emitted (int), followed by all of its
@@ -956,14 +1123,15 @@ public class LinkedList<E>
         s.writeInt(size);
 
         // Write out all elements in the proper order.
-        for (Entry e = header.next; e != header; e = e.next)
-            s.writeObject(e.element);
+        for (Node<E> x = first; x != null; x = x.next)
+            s.writeObject(x.item);
     }
 
     /**
-     * Reconstitute this <tt>LinkedList</tt> instance from a stream (that is
-     * deserialize it).
+     * Reconstitutes this {@code LinkedList} instance from a stream
+     * (that is, deserializes it).
      */
+    @SuppressWarnings("unchecked")
     private void readObject(java.io.ObjectInputStream s)
         throws java.io.IOException, ClassNotFoundException {
         // Read in any hidden serialization magic
@@ -972,12 +1140,8 @@ public class LinkedList<E>
         // Read in size
         int size = s.readInt();
 
-        // Initialize header
-        header = new Entry<E>(null, null, null);
-        header.next = header.previous = header;
-
         // Read in all elements in the proper order.
-        for (int i=0; i<size; i++)
-            addBefore((E)s.readObject(), header);
+        for (int i = 0; i < size; i++)
+            linkLast((E)s.readObject());
     }
 }

test/java/util/Collection/MOAT.java

@@ -421,8 +421,11 @@ public class MOAT {
 
     private static void testQueue(Queue<Integer> q) {
         q.clear();
-        for (int i = 0; i < 5; i++)
+        for (int i = 0; i < 5; i++) {
+            testQueueAddRemove(q, null);
+            testQueueAddRemove(q, 537);
             q.add(i);
+        }
         equal(q.size(), 5);
         checkFunctionalInvariants(q);
         q.poll();
@@ -435,6 +438,216 @@ public class MOAT {
         }
     }
 
+    private static void testQueueAddRemove(final Queue<Integer> q,
+                                           final Integer e) {
+        final List<Integer> originalContents = new ArrayList<Integer>(q);
+        final boolean isEmpty = q.isEmpty();
+        final boolean isList = (q instanceof List);
+        final List asList = isList ? (List) q : null;
+        check(!q.contains(e));
+        try {
+            q.add(e);
+        } catch (NullPointerException npe) {
+            check(e == null);
+            return;                     // Null elements not supported
+        }
+        check(q.contains(e));
+        check(q.remove(e));
+        check(!q.contains(e));
+        equal(new ArrayList<Integer>(q), originalContents);
+
+        if (q instanceof Deque<?>) {
+            final Deque<Integer> deq = (Deque<Integer>) q;
+            final List<Integer> singleton = Collections.singletonList(e);
+
+            // insert, query, remove element at head
+            if (isEmpty) {
+                THROWS(NoSuchElementException.class,
+                       new Fun(){void f(){ deq.getFirst(); }},
+                       new Fun(){void f(){ deq.element(); }},
+                       new Fun(){void f(){ deq.iterator().next(); }});
+                check(deq.peekFirst() == null);
+                check(deq.peek() == null);
+            } else {
+                check(deq.getFirst() != e);
+                check(deq.element() != e);
+                check(deq.iterator().next() != e);
+                check(deq.peekFirst() != e);
+                check(deq.peek() != e);
+            }
+            check(!deq.contains(e));
+            check(!deq.removeFirstOccurrence(e));
+            check(!deq.removeLastOccurrence(e));
+            if (isList) {
+                check(asList.indexOf(e) == -1);
+                check(asList.lastIndexOf(e) == -1);
+            }
+            switch (rnd.nextInt(isList ? 4 : 3)) {
+            case 0: deq.addFirst(e); break;
+            case 1: check(deq.offerFirst(e)); break;
+            case 2: deq.push(e); break;
+            case 3: asList.add(0, e); break;
+            default: throw new AssertionError();
+            }
+            check(deq.peekFirst() == e);
+            check(deq.getFirst() == e);
+            check(deq.element() == e);
+            check(deq.peek() == e);
+            check(deq.iterator().next() == e);
+            check(deq.contains(e));
+            if (isList) {
+                check(asList.get(0) == e);
+                check(asList.indexOf(e) == 0);
+                check(asList.lastIndexOf(e) == 0);
+                check(asList.subList(0, 1).equals(singleton));
+            }
+            switch (rnd.nextInt(isList ? 11 : 9)) {
+            case 0: check(deq.pollFirst() == e); break;
+            case 1: check(deq.removeFirst() == e); break;
+            case 2: check(deq.remove() == e); break;
+            case 3: check(deq.pop() == e); break;
+            case 4: check(deq.removeFirstOccurrence(e)); break;
+            case 5: check(deq.removeLastOccurrence(e)); break;
+            case 6: check(deq.remove(e)); break;
+            case 7: check(deq.removeAll(singleton)); break;
+            case 8: Iterator it = deq.iterator(); it.next(); it.remove(); break;
+            case 9: asList.remove(0); break;
+            case 10: asList.subList(0, 1).clear(); break;
+            default: throw new AssertionError();
+            }
+            if (isEmpty) {
+                THROWS(NoSuchElementException.class,
+                       new Fun(){void f(){ deq.getFirst(); }},
+                       new Fun(){void f(){ deq.element(); }},
+                       new Fun(){void f(){ deq.iterator().next(); }});
+                check(deq.peekFirst() == null);
+                check(deq.peek() == null);
+            } else {
+                check(deq.getFirst() != e);
+                check(deq.element() != e);
+                check(deq.iterator().next() != e);
+                check(deq.peekFirst() != e);
+                check(deq.peek() != e);
+            }
+            check(!deq.contains(e));
+            check(!deq.removeFirstOccurrence(e));
+            check(!deq.removeLastOccurrence(e));
+            if (isList) {
+                check(isEmpty || asList.get(0) != e);
+                check(asList.indexOf(e) == -1);
+                check(asList.lastIndexOf(e) == -1);
+            }
+            equal(new ArrayList<Integer>(deq), originalContents);
+
+            // insert, query, remove element at tail
+            if (isEmpty) {
+                check(deq.peekLast() == null);
+                THROWS(NoSuchElementException.class,
+                       new Fun(){void f(){ deq.getLast(); }});
+            } else {
+                check(deq.peekLast() != e);
+                check(deq.getLast() != e);
+            }
+            switch (rnd.nextInt(isList ? 6 : 4)) {
+            case 0: deq.addLast(e); break;
+            case 1: check(deq.offerLast(e)); break;
+            case 2: check(deq.add(e)); break;
+            case 3: deq.addAll(singleton); break;
+            case 4: asList.addAll(deq.size(), singleton); break;
+            case 5: asList.add(deq.size(), e); break;
+            default: throw new AssertionError();
+            }
+            check(deq.peekLast() == e);
+            check(deq.getLast() == e);
+            check(deq.contains(e));
+            if (isList) {
+                ListIterator it = asList.listIterator(asList.size());
+                check(it.previous() == e);
+                check(asList.get(asList.size() - 1) == e);
+                check(asList.indexOf(e) == asList.size() - 1);
+                check(asList.lastIndexOf(e) == asList.size() - 1);
+                int size = asList.size();
+                check(asList.subList(size - 1, size).equals(singleton));
+            }
+            switch (rnd.nextInt(isList ? 8 : 6)) {
+            case 0: check(deq.pollLast() == e); break;
+            case 1: check(deq.removeLast() == e); break;
+            case 2: check(deq.removeFirstOccurrence(e)); break;
+            case 3: check(deq.removeLastOccurrence(e)); break;
+            case 4: check(deq.remove(e)); break;
+            case 5: check(deq.removeAll(singleton)); break;
+            case 6: asList.remove(asList.size() - 1); break;
+            case 7:
+                ListIterator it = asList.listIterator(asList.size());
+                it.previous();
+                it.remove();
+                break;
+            default: throw new AssertionError();
+            }
+            if (isEmpty) {
+                check(deq.peekLast() == null);
+                THROWS(NoSuchElementException.class,
+                       new Fun(){void f(){ deq.getLast(); }});
+            } else {
+                check(deq.peekLast() != e);
+                check(deq.getLast() != e);
+            }
+            check(!deq.contains(e));
+            equal(new ArrayList<Integer>(deq), originalContents);
+
+            // Test operations on empty deque
+            switch (rnd.nextInt(isList ? 4 : 2)) {
+            case 0: deq.clear(); break;
+            case 1:
+                Iterator it = deq.iterator();
+                while (it.hasNext()) {
+                    it.next();
+                    it.remove();
+                }
+                break;
+            case 2: asList.subList(0, asList.size()).clear(); break;
+            case 3:
+                ListIterator lit = asList.listIterator(asList.size());
+                while (lit.hasPrevious()) {
+                    lit.previous();
+                    lit.remove();
+                }
+                break;
+            default: throw new AssertionError();
+            }
+            testEmptyCollection(deq);
+            check(!deq.iterator().hasNext());
+            if (isList) {
+                check(!asList.listIterator().hasPrevious());
+                THROWS(NoSuchElementException.class,
+                       new Fun(){void f(){ asList.listIterator().previous(); }});
+            }
+            THROWS(NoSuchElementException.class,
+                   new Fun(){void f(){ deq.iterator().next(); }},
+                   new Fun(){void f(){ deq.element(); }},
+                   new Fun(){void f(){ deq.getFirst(); }},
+                   new Fun(){void f(){ deq.getLast(); }},
+                   new Fun(){void f(){ deq.pop(); }},
+                   new Fun(){void f(){ deq.remove(); }},
+                   new Fun(){void f(){ deq.removeFirst(); }},
+                   new Fun(){void f(){ deq.removeLast(); }});
+
+            check(deq.poll() == null);
+            check(deq.pollFirst() == null);
+            check(deq.pollLast() == null);
+            check(deq.peek() == null);
+            check(deq.peekFirst() == null);
+            check(deq.peekLast() == null);
+            check(!deq.removeFirstOccurrence(e));
+            check(!deq.removeLastOccurrence(e));
+
+            check(deq.addAll(originalContents) == !isEmpty);
+            equal(new ArrayList<Integer>(deq), originalContents);
+            check(!deq.addAll(Collections.<Integer>emptyList()));
+            equal(new ArrayList<Integer>(deq), originalContents);
+        }
+    }
+
     private static void testQueueIteratorRemove(Queue<Integer> q) {
         System.err.printf("testQueueIteratorRemove %s%n",
                           q.getClass().getSimpleName());