From 43d57ddd46f0cf5db92469219654c155bd982e05 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Fri, 10 Apr 2015 09:43:06 +0200
Subject: [PATCH] [#3623] CompositeByteBuf.iterator() should return optimized
 Iterable

Motivation:

CompositeByteBuf.iterator() currently creates a new ArrayList and fill it with the ByteBufs, which is more expensive then it needs to be.

Modifications:

- Use special Iterator implementation

Result:

Less overhead when calling iterator()
---
 .../io/netty/buffer/CompositeByteBuf.java     | 40 +++++++++-
 .../buffer/AbstractCompositeByteBufTest.java  | 73 +++++++++++++++++++
 2 files changed, 109 insertions(+), 4 deletions(-)

diff --git a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
index 095e534110..e18e028e47 100644
--- a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
@@ -28,9 +28,11 @@ import java.nio.channels.ScatteringByteChannel;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.ConcurrentModificationException;
 import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
+import java.util.NoSuchElementException;
 
 /**
  * A virtual buffer which shows multiple buffers as a single merged buffer.  It is recommended to use
@@ -40,6 +42,7 @@ import java.util.ListIterator;
 public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements Iterable<ByteBuf> {
 
     private static final ByteBuffer EMPTY_NIO_BUFFER = Unpooled.EMPTY_BUFFER.nioBuffer();
+    private static final Iterator<ByteBuf> EMPTY_ITERATOR = Collections.<ByteBuf>emptyList().iterator();
 
     private final ResourceLeak leak;
     private final ByteBufAllocator alloc;
@@ -374,11 +377,10 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
     @Override
     public Iterator<ByteBuf> iterator() {
         ensureAccessible();
-        List<ByteBuf> list = new ArrayList<ByteBuf>(components.size());
-        for (Component c: components) {
-            list.add(c.buf);
+        if (components.isEmpty()) {
+            return EMPTY_ITERATOR;
         }
-        return list.iterator();
+        return new CompositeByteBufIterator();
     }
 
     /**
@@ -1648,4 +1650,34 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
     public ByteBuf unwrap() {
         return null;
     }
+
+    private final class CompositeByteBufIterator implements Iterator<ByteBuf> {
+        private final int size = components.size();
+        private int index;
+
+        @Override
+        public boolean hasNext() {
+            return size > index;
+        }
+
+        @Override
+        public ByteBuf next() {
+            if (size != components.size()) {
+                throw new ConcurrentModificationException();
+            }
+            if (!hasNext()) {
+                throw new NoSuchElementException();
+            }
+            try {
+                return components.get(index++).buf;
+            } catch (IndexOutOfBoundsException e) {
+                throw new ConcurrentModificationException();
+            }
+        }
+
+        @Override
+        public void remove() {
+            throw new UnsupportedOperationException("Read-Only");
+        }
+    }
 }
diff --git a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
index 2812fd9eb1..b020512772 100644
--- a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
+++ b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
@@ -21,7 +21,10 @@ import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.ConcurrentModificationException;
+import java.util.Iterator;
 import java.util.List;
+import java.util.NoSuchElementException;
 
 import static io.netty.buffer.Unpooled.*;
 import static io.netty.util.ReferenceCountUtil.*;
@@ -865,4 +868,74 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
         assertNotSame(EMPTY_BUFFER, cbuf.internalComponentAtOffset(1));
         cbuf.release();
     }
+
+    @Test
+    public void testIterator() {
+        CompositeByteBuf cbuf = compositeBuffer();
+        cbuf.addComponent(EMPTY_BUFFER);
+        cbuf.addComponent(EMPTY_BUFFER);
+
+        Iterator<ByteBuf> it = cbuf.iterator();
+        assertTrue(it.hasNext());
+        assertSame(EMPTY_BUFFER, it.next());
+        assertTrue(it.hasNext());
+        assertSame(EMPTY_BUFFER, it.next());
+        assertFalse(it.hasNext());
+
+        try {
+            it.next();
+            fail();
+        } catch (NoSuchElementException e) {
+            //Expected
+        }
+        cbuf.release();
+    }
+
+    @Test
+    public void testEmptyIterator() {
+        CompositeByteBuf cbuf = compositeBuffer();
+
+        Iterator<ByteBuf> it = cbuf.iterator();
+        assertFalse(it.hasNext());
+
+        try {
+            it.next();
+            fail();
+        } catch (NoSuchElementException e) {
+            //Expected
+        }
+        cbuf.release();
+    }
+
+    @Test(expected = ConcurrentModificationException.class)
+    public void testIteratorConcurrentModificationAdd() {
+        CompositeByteBuf cbuf = compositeBuffer();
+        cbuf.addComponent(EMPTY_BUFFER);
+
+        Iterator<ByteBuf> it = cbuf.iterator();
+        cbuf.addComponent(EMPTY_BUFFER);
+
+        assertTrue(it.hasNext());
+        try {
+            it.next();
+        } finally {
+            cbuf.release();
+        }
+    }
+
+    @Test(expected = ConcurrentModificationException.class)
+    public void testIteratorConcurrentModificationRemove() {
+        CompositeByteBuf cbuf = compositeBuffer();
+        cbuf.addComponent(EMPTY_BUFFER);
+
+        Iterator<ByteBuf> it = cbuf.iterator();
+        cbuf.removeComponent(0);
+
+        assertTrue(it.hasNext());
+        try {
+            it.next();
+        } finally {
+            cbuf.release();
+        }
+    }
 }