Use a single array in SelectedSelectionKeySet

Motivation:
SelectedSelectionKeySet currently uses 2 arrays internally and users are expected to call flip() to access the underlying array and switch the active array. However we do not concurrently use 2 arrays at the same time and we can get away with using a single array if we are careful about when we reset the elements of the array.

Modifications:
- Introduce SelectedSelectionKeySetSelector which wraps a Selector and ensures we reset the underlying SelectedSelectionKeySet data structures before we select
- The loop bounds in NioEventLoop#processSelectedKeysOptimized can be defined more precisely because we know the real size of the underlying array

Result:
Fixes https://github.com/netty/netty/issues/6058

transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java

@@ -384,7 +384,7 @@ public abstract class AbstractNioChannel extends AbstractChannel {
         boolean selected = false;
         for (;;) {
             try {
-                selectionKey = javaChannel().register(eventLoop().selector, 0, this);
+                selectionKey = javaChannel().register(eventLoop().unwrappedSelector(), 0, this);
                 return;
             } catch (CancelledKeyException e) {
                 if (!selected) {

transport/src/main/java/io/netty/channel/nio/NioEventLoop.java

@@ -116,7 +116,8 @@ public final class NioEventLoop extends SingleThreadEventLoop {
     /**
      * The NIO {@link Selector}.
      */
-    Selector selector;
+    private Selector selector;
+    private Selector unwrappedSelector;
     private SelectedSelectionKeySet selectedKeys;
 
     private final SelectorProvider provider;
@@ -150,15 +151,14 @@ public final class NioEventLoop extends SingleThreadEventLoop {
     }
 
     private Selector openSelector() {
-        final Selector selector;
         try {
-            selector = provider.openSelector();
+            unwrappedSelector = provider.openSelector();
         } catch (IOException e) {
             throw new ChannelException("failed to open a new selector", e);
         }
 
         if (DISABLE_KEYSET_OPTIMIZATION) {
-            return selector;
+            return unwrappedSelector;
         }
 
         final SelectedSelectionKeySet selectedKeySet = new SelectedSelectionKeySet();
@@ -179,12 +179,12 @@ public final class NioEventLoop extends SingleThreadEventLoop {
 
         if (!(maybeSelectorImplClass instanceof Class) ||
                 // ensure the current selector implementation is what we can instrument.
-                !((Class<?>) maybeSelectorImplClass).isAssignableFrom(selector.getClass())) {
+                !((Class<?>) maybeSelectorImplClass).isAssignableFrom(unwrappedSelector.getClass())) {
             if (maybeSelectorImplClass instanceof Throwable) {
                 Throwable t = (Throwable) maybeSelectorImplClass;
-                logger.trace("failed to instrument a special java.util.Set into: {}", selector, t);
+                logger.trace("failed to instrument a special java.util.Set into: {}", unwrappedSelector, t);
             }
-            return selector;
+            return unwrappedSelector;
         }
 
         final Class<?> selectorImplClass = (Class<?>) maybeSelectorImplClass;
@@ -205,8 +205,8 @@ public final class NioEventLoop extends SingleThreadEventLoop {
                         return cause;
                     }
 
-                    selectedKeysField.set(selector, selectedKeySet);
+                    selectedKeysField.set(unwrappedSelector, selectedKeySet);
-                    publicSelectedKeysField.set(selector, selectedKeySet);
+                    publicSelectedKeysField.set(unwrappedSelector, selectedKeySet);
                     return null;
                 } catch (NoSuchFieldException e) {
                     return e;
@@ -219,13 +219,12 @@ public final class NioEventLoop extends SingleThreadEventLoop {
         if (maybeException instanceof Exception) {
             selectedKeys = null;
             Exception e = (Exception) maybeException;
-            logger.trace("failed to instrument a special java.util.Set into: {}", selector, e);
+            logger.trace("failed to instrument a special java.util.Set into: {}", unwrappedSelector, e);
-        } else {
+            return unwrappedSelector;
-            selectedKeys = selectedKeySet;
-            logger.trace("instrumented a special java.util.Set into: {}", selector);
         }
-
+        selectedKeys = selectedKeySet;
-        return selector;
+        logger.trace("instrumented a special java.util.Set into: {}", unwrappedSelector);
+        return new SelectedSelectionKeySetSelector(unwrappedSelector, selectedKeySet);
     }
 
     /**
@@ -474,7 +473,7 @@ public final class NioEventLoop extends SingleThreadEventLoop {
 
     private void processSelectedKeys() {
         if (selectedKeys != null) {
-            processSelectedKeysOptimized(selectedKeys.flip());
+            processSelectedKeysOptimized();
         } else {
             processSelectedKeysPlain(selector.selectedKeys());
         }
@@ -547,15 +546,12 @@ public final class NioEventLoop extends SingleThreadEventLoop {
         }
     }
 
-    private void processSelectedKeysOptimized(SelectionKey[] selectedKeys) {
+    private void processSelectedKeysOptimized() {
-        for (int i = 0;; i ++) {
+        for (int i = 0; i < selectedKeys.size; ++i) {
-            final SelectionKey k = selectedKeys[i];
+            final SelectionKey k = selectedKeys.keys[i];
-            if (k == null) {
-                break;
-            }
             // null out entry in the array to allow to have it GC'ed once the Channel close
             // See https://github.com/netty/netty/issues/2363
-            selectedKeys[i] = null;
+            selectedKeys.keys[i] = null;
 
             final Object a = k.attachment();
 
@@ -570,21 +566,9 @@ public final class NioEventLoop extends SingleThreadEventLoop {
             if (needsToSelectAgain) {
                 // null out entries in the array to allow to have it GC'ed once the Channel close
                 // See https://github.com/netty/netty/issues/2363
-                for (;;) {
+                selectedKeys.reset(i + 1);
-                    i++;
-                    if (selectedKeys[i] == null) {
-                        break;
-                    }
-                    selectedKeys[i] = null;
-                }
 
                 selectAgain();
-                // Need to flip the optimized selectedKeys to get the right reference to the array
-                // and reset the index to -1 which will then set to 0 on the for loop
-                // to start over again.
-                //
-                // See https://github.com/netty/netty/issues/1523
-                selectedKeys = this.selectedKeys.flip();
                 i = -1;
             }
         }
@@ -704,6 +688,10 @@ public final class NioEventLoop extends SingleThreadEventLoop {
         }
     }
 
+    Selector unwrappedSelector() {
+        return unwrappedSelector;
+    }
+
     int selectNow() throws IOException {
         try {
             return selector.selectNow();

transport/src/main/java/io/netty/channel/nio/SelectedSelectionKeySet.java

@@ -13,24 +13,20 @@
  * License for the specific language governing permissions and limitations
  * under the License.
  */
-
 package io.netty.channel.nio;
 
 import java.nio.channels.SelectionKey;
 import java.util.AbstractSet;
+import java.util.Arrays;
 import java.util.Iterator;
 
 final class SelectedSelectionKeySet extends AbstractSet<SelectionKey> {
 
-    private SelectionKey[] keysA;
+    SelectionKey[] keys;
-    private int keysASize;
+    int size;
-    private SelectionKey[] keysB;
-    private int keysBSize;
-    private boolean isA = true;
 
     SelectedSelectionKeySet() {
-        keysA = new SelectionKey[1024];
+        keys = new SelectionKey[1024];
-        keysB = keysA.clone();
     }
 
     @Override
@@ -39,58 +35,17 @@ final class SelectedSelectionKeySet extends AbstractSet<SelectionKey> {
             return false;
         }
 
-        if (isA) {
+        keys[size++] = o;
-            int size = keysASize;
+        if (size == keys.length) {
-            keysA[size ++] = o;
+            increaseCapacity();
-            keysASize = size;
-            if (size == keysA.length) {
-                doubleCapacityA();
-            }
-        } else {
-            int size = keysBSize;
-            keysB[size ++] = o;
-            keysBSize = size;
-            if (size == keysB.length) {
-                doubleCapacityB();
-            }
         }
 
         return true;
     }
 
-    private void doubleCapacityA() {
-        SelectionKey[] newKeysA = new SelectionKey[keysA.length << 1];
-        System.arraycopy(keysA, 0, newKeysA, 0, keysASize);
-        keysA = newKeysA;
-    }
-
-    private void doubleCapacityB() {
-        SelectionKey[] newKeysB = new SelectionKey[keysB.length << 1];
-        System.arraycopy(keysB, 0, newKeysB, 0, keysBSize);
-        keysB = newKeysB;
-    }
-
-    SelectionKey[] flip() {
-        if (isA) {
-            isA = false;
-            keysA[keysASize] = null;
-            keysBSize = 0;
-            return keysA;
-        } else {
-            isA = true;
-            keysB[keysBSize] = null;
-            keysASize = 0;
-            return keysB;
-        }
-    }
-
     @Override
     public int size() {
-        if (isA) {
+        return size;
-            return keysASize;
-        } else {
-            return keysBSize;
-        }
     }
 
     @Override
@@ -107,4 +62,19 @@ final class SelectedSelectionKeySet extends AbstractSet<SelectionKey> {
     public Iterator<SelectionKey> iterator() {
         throw new UnsupportedOperationException();
     }
+
+    void reset() {
+        reset(0);
+    }
+
+    void reset(int start) {
+        Arrays.fill(keys, start, size, null);
+        size = 0;
+    }
+
+    private void increaseCapacity() {
+        SelectionKey[] newKeys = new SelectionKey[keys.length << 1];
+        System.arraycopy(keys, 0, newKeys, 0, size);
+        keys = newKeys;
+    }
 }

transport/src/main/java/io/netty/channel/nio/SelectedSelectionKeySetSelector.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright 2017 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.channel.nio;
+
+import java.io.IOException;
+import java.nio.channels.SelectionKey;
+import java.nio.channels.Selector;
+import java.nio.channels.spi.SelectorProvider;
+import java.util.Set;
+
+final class SelectedSelectionKeySetSelector extends Selector {
+    private final SelectedSelectionKeySet selectionKeys;
+    private final Selector delegate;
+
+    SelectedSelectionKeySetSelector(Selector delegate, SelectedSelectionKeySet selectionKeys) {
+        this.delegate = delegate;
+        this.selectionKeys = selectionKeys;
+    }
+
+    @Override
+    public boolean isOpen() {
+        return delegate.isOpen();
+    }
+
+    @Override
+    public SelectorProvider provider() {
+        return delegate.provider();
+    }
+
+    @Override
+    public Set<SelectionKey> keys() {
+        return delegate.keys();
+    }
+
+    @Override
+    public Set<SelectionKey> selectedKeys() {
+        return delegate.selectedKeys();
+    }
+
+    @Override
+    public int selectNow() throws IOException {
+        selectionKeys.reset();
+        return delegate.selectNow();
+    }
+
+    @Override
+    public int select(long timeout) throws IOException {
+        selectionKeys.reset();
+        return delegate.select(timeout);
+    }
+
+    @Override
+    public int select() throws IOException {
+        selectionKeys.reset();
+        return delegate.select();
+    }
+
+    @Override
+    public Selector wakeup() {
+        return delegate.wakeup();
+    }
+
+    @Override
+    public void close() throws IOException {
+        delegate.close();
+    }
+}

transport/src/test/java/io/netty/channel/nio/SelectedSelectionKeySetTest.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2017 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.channel.nio;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+import java.nio.channels.SelectionKey;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+public class SelectedSelectionKeySetTest {
+    @Mock
+    private SelectionKey mockKey;
+    @Mock
+    private SelectionKey mockKey2;
+
+    @Before
+    public void setup() {
+        MockitoAnnotations.initMocks(this);
+    }
+
+    @Test
+    public void addElements() {
+        SelectedSelectionKeySet set = new SelectedSelectionKeySet();
+        final int expectedSize = 1000000;
+        for (int i = 0; i < expectedSize; ++i) {
+            assertTrue(set.add(mockKey));
+        }
+
+        assertEquals(expectedSize, set.size());
+        assertFalse(set.isEmpty());
+    }
+
+    @Test
+    public void resetSet() {
+        SelectedSelectionKeySet set = new SelectedSelectionKeySet();
+        assertTrue(set.add(mockKey));
+        assertTrue(set.add(mockKey2));
+        set.reset(1);
+
+        assertSame(mockKey, set.keys[0]);
+        assertNull(set.keys[1]);
+        assertEquals(0, set.size());
+        assertTrue(set.isEmpty());
+    }
+}