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
This commit is contained in:
Scott Mitchell 2016-11-23 13:50:42 -08:00
parent 4d7d478a3d
commit 795f318c3c
5 changed files with 194 additions and 90 deletions

View File

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

View File

@ -116,7 +116,8 @@ public final class NioEventLoop extends SingleThreadEventLoop {
/** /**
* The NIO {@link Selector}. * The NIO {@link Selector}.
*/ */
Selector selector; private Selector selector;
private Selector unwrappedSelector;
private SelectedSelectionKeySet selectedKeys; private SelectedSelectionKeySet selectedKeys;
private final SelectorProvider provider; private final SelectorProvider provider;
@ -150,15 +151,14 @@ public final class NioEventLoop extends SingleThreadEventLoop {
} }
private Selector openSelector() { private Selector openSelector() {
final Selector selector;
try { try {
selector = provider.openSelector(); unwrappedSelector = provider.openSelector();
} catch (IOException e) { } catch (IOException e) {
throw new ChannelException("failed to open a new selector", e); throw new ChannelException("failed to open a new selector", e);
} }
if (DISABLE_KEYSET_OPTIMIZATION) { if (DISABLE_KEYSET_OPTIMIZATION) {
return selector; return unwrappedSelector;
} }
final SelectedSelectionKeySet selectedKeySet = new SelectedSelectionKeySet(); final SelectedSelectionKeySet selectedKeySet = new SelectedSelectionKeySet();
@ -179,12 +179,12 @@ public final class NioEventLoop extends SingleThreadEventLoop {
if (!(maybeSelectorImplClass instanceof Class) || if (!(maybeSelectorImplClass instanceof Class) ||
// ensure the current selector implementation is what we can instrument. // ensure the current selector implementation is what we can instrument.
!((Class<?>) maybeSelectorImplClass).isAssignableFrom(selector.getClass())) { !((Class<?>) maybeSelectorImplClass).isAssignableFrom(unwrappedSelector.getClass())) {
if (maybeSelectorImplClass instanceof Throwable) { if (maybeSelectorImplClass instanceof Throwable) {
Throwable t = (Throwable) maybeSelectorImplClass; 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; final Class<?> selectorImplClass = (Class<?>) maybeSelectorImplClass;
@ -205,8 +205,8 @@ public final class NioEventLoop extends SingleThreadEventLoop {
return cause; return cause;
} }
selectedKeysField.set(selector, selectedKeySet); selectedKeysField.set(unwrappedSelector, selectedKeySet);
publicSelectedKeysField.set(selector, selectedKeySet); publicSelectedKeysField.set(unwrappedSelector, selectedKeySet);
return null; return null;
} catch (NoSuchFieldException e) { } catch (NoSuchFieldException e) {
return e; return e;
@ -219,13 +219,12 @@ public final class NioEventLoop extends SingleThreadEventLoop {
if (maybeException instanceof Exception) { if (maybeException instanceof Exception) {
selectedKeys = null; selectedKeys = null;
Exception e = (Exception) maybeException; 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() { private void processSelectedKeys() {
if (selectedKeys != null) { if (selectedKeys != null) {
processSelectedKeysOptimized(selectedKeys.flip()); processSelectedKeysOptimized();
} else { } else {
processSelectedKeysPlain(selector.selectedKeys()); 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 // 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 // See https://github.com/netty/netty/issues/2363
selectedKeys[i] = null; selectedKeys.keys[i] = null;
final Object a = k.attachment(); final Object a = k.attachment();
@ -570,21 +566,9 @@ public final class NioEventLoop extends SingleThreadEventLoop {
if (needsToSelectAgain) { if (needsToSelectAgain) {
// null out entries in the array to allow to have it GC'ed once the Channel close // 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 // See https://github.com/netty/netty/issues/2363
for (;;) { selectedKeys.reset(i + 1);
i++;
if (selectedKeys[i] == null) {
break;
}
selectedKeys[i] = null;
}
selectAgain(); 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; i = -1;
} }
} }
@ -704,6 +688,10 @@ public final class NioEventLoop extends SingleThreadEventLoop {
} }
} }
Selector unwrappedSelector() {
return unwrappedSelector;
}
int selectNow() throws IOException { int selectNow() throws IOException {
try { try {
return selector.selectNow(); return selector.selectNow();

View File

@ -13,24 +13,20 @@
* License for the specific language governing permissions and limitations * License for the specific language governing permissions and limitations
* under the License. * under the License.
*/ */
package io.netty.channel.nio; package io.netty.channel.nio;
import java.nio.channels.SelectionKey; import java.nio.channels.SelectionKey;
import java.util.AbstractSet; import java.util.AbstractSet;
import java.util.Arrays;
import java.util.Iterator; import java.util.Iterator;
final class SelectedSelectionKeySet extends AbstractSet<SelectionKey> { 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() { SelectedSelectionKeySet() {
keysA = new SelectionKey[1024]; keys = new SelectionKey[1024];
keysB = keysA.clone();
} }
@Override @Override
@ -39,58 +35,17 @@ final class SelectedSelectionKeySet extends AbstractSet<SelectionKey> {
return false; 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; 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 @Override
public int size() { public int size() {
if (isA) { return size;
return keysASize;
} else {
return keysBSize;
}
} }
@Override @Override
@ -107,4 +62,19 @@ final class SelectedSelectionKeySet extends AbstractSet<SelectionKey> {
public Iterator<SelectionKey> iterator() { public Iterator<SelectionKey> iterator() {
throw new UnsupportedOperationException(); 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;
}
} }

View File

@ -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();
}
}

View File

@ -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());
}
}