From d933a9dd566fb3ddc47e8982df131f0cccf58b17 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Wed, 9 Sep 2020 20:30:48 +0200 Subject: [PATCH] Move IovArray handling code in an extra class to better seperate it and (#10559) easier to test. Motivation: We should move the IovArray related code to an extra class so its easier to test Modifications: - Move into extra class - Add dedicated test Result: Cleanup --- .../netty/channel/uring/IOUringEventLoop.java | 38 +++-------- .../io/netty/channel/uring/IovArrays.java | 63 +++++++++++++++++++ .../io/netty/channel/uring/IovArraysTest.java | 62 ++++++++++++++++++ 3 files changed, 134 insertions(+), 29 deletions(-) create mode 100644 transport-native-io_uring/src/main/java/io/netty/channel/uring/IovArrays.java create mode 100644 transport-native-io_uring/src/test/java/io/netty/channel/uring/IovArraysTest.java diff --git a/transport-native-io_uring/src/main/java/io/netty/channel/uring/IOUringEventLoop.java b/transport-native-io_uring/src/main/java/io/netty/channel/uring/IOUringEventLoop.java index 5c49734ced..577a2a733d 100644 --- a/transport-native-io_uring/src/main/java/io/netty/channel/uring/IOUringEventLoop.java +++ b/transport-native-io_uring/src/main/java/io/netty/channel/uring/IOUringEventLoop.java @@ -48,8 +48,7 @@ final class IOUringEventLoop extends SingleThreadEventLoop implements private final AtomicLong nextWakeupNanos = new AtomicLong(AWAKE); private final FileDescriptor eventfd; - private final IovArray[] iovArrays; - private int iovArrayIdx; + private final IovArrays iovArrays; private long prevDeadlineNanos = NONE; private boolean pendingWakeup; @@ -61,15 +60,12 @@ final class IOUringEventLoop extends SingleThreadEventLoop implements // TODO: Let's hard code this to 8 IovArrays to keep the memory overhead kind of small. We may want to consider // allow to change this in the future. - iovArrays = new IovArray[8]; - for (int i = 0; i < iovArrays.length; i++) { - iovArrays[i] = new IovArray(); - } + iovArrays = new IovArrays(8); ringBuffer = Native.createRingBuffer(new Runnable() { @Override public void run() { // Once we submitted its safe to clear the IovArrays and so be able to re-use these. - clearUsedIovArrays(); + iovArrays.clear(); } }); @@ -183,13 +179,6 @@ final class IOUringEventLoop extends SingleThreadEventLoop implements } } - private void clearUsedIovArrays() { - for (int i = 0; i <= iovArrayIdx; i++) { - iovArrays[i].clear(); - } - iovArrayIdx = 0; - } - @Override public boolean handle(int fd, int res, long flags, int op, int pollMask) { IOUringSubmissionQueue submissionQueue = ringBuffer.getIoUringSubmissionQueue(); @@ -294,9 +283,7 @@ final class IOUringEventLoop extends SingleThreadEventLoop implements logger.warn("Failed to close the event fd.", e); } ringBuffer.close(); - for (IovArray array: iovArrays) { - array.release(); - } + iovArrays.release(); } public RingBuffer getRingBuffer() { @@ -312,18 +299,11 @@ final class IOUringEventLoop extends SingleThreadEventLoop implements } public IovArray iovArray() { - IovArray iovArray = iovArrays[iovArrayIdx]; - if (iovArray.isFull()) { - if (iovArrayIdx < iovArrays.length - 1) { - // There is another array left that we can use, increment the index and use it. - iovArrayIdx++; - iovArray = iovArrays[iovArrayIdx]; - } else { - // No array left to use. Submit so we can reuse all of the arrays. - ringBuffer.getIoUringSubmissionQueue().submit(); - iovArray = iovArrays[iovArrayIdx]; - } - assert !iovArray.isFull(); + IovArray iovArray = iovArrays.next(); + if (iovArray == null) { + ringBuffer.getIoUringSubmissionQueue().submit(); + iovArray = iovArrays.next(); + assert iovArray != null; } return iovArray; } diff --git a/transport-native-io_uring/src/main/java/io/netty/channel/uring/IovArrays.java b/transport-native-io_uring/src/main/java/io/netty/channel/uring/IovArrays.java new file mode 100644 index 0000000000..45aee77943 --- /dev/null +++ b/transport-native-io_uring/src/main/java/io/netty/channel/uring/IovArrays.java @@ -0,0 +1,63 @@ +/* + * Copyright 2020 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.uring; + +import io.netty.channel.unix.IovArray; + +final class IovArrays { + + private final IovArray[] iovArrays; + private int iovArrayIdx; + + IovArrays(int numArrays) { + iovArrays = new IovArray[numArrays]; + for (int i = 0 ; i < iovArrays.length; i++) { + iovArrays[i] = new IovArray(); + } + } + + /** + * Return the next {@link IovArray} to use which has space left in it. Otherwise returns {@code null} if there + * is no {@link IovArray} which has some space left. + */ + IovArray next() { + IovArray iovArray = iovArrays[iovArrayIdx]; + if (iovArray.isFull()) { + if (iovArrayIdx < iovArrays.length - 1) { + // There is another array left that we can use, increment the index and use it. + iovArrayIdx++; + iovArray = iovArrays[iovArrayIdx]; + } + } + return iovArray.isFull() ? null : iovArray; + } + + /** + * Clear all {@link IovArray}s that were used since the last time this method was called. + */ + void clear() { + for (int i = 0; i <= iovArrayIdx; i++) { + iovArrays[i].clear(); + } + iovArrayIdx = 0; + } + + void release() { + for (IovArray array: iovArrays) { + array.release(); + } + } +} diff --git a/transport-native-io_uring/src/test/java/io/netty/channel/uring/IovArraysTest.java b/transport-native-io_uring/src/test/java/io/netty/channel/uring/IovArraysTest.java new file mode 100644 index 0000000000..649c6f4269 --- /dev/null +++ b/transport-native-io_uring/src/test/java/io/netty/channel/uring/IovArraysTest.java @@ -0,0 +1,62 @@ +/* + * Copyright 2020 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.uring; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import io.netty.channel.unix.IovArray; +import org.junit.Test; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; + +public class IovArraysTest { + + @Test + public void test() { + IOUring.ensureAvailability(); + ByteBuf buf = Unpooled.directBuffer(1).writeZero(1); + IovArrays arrays = new IovArrays(2); + try { + IovArray next = arrays.next(); + assertNotNull(next); + assertSame(next, arrays.next()); + + while (next.add(buf, 0, buf.readableBytes())) { + // loop until we filled it. + } + + IovArray next2 = arrays.next(); + assertNotSame(next, next2); + + while (next2.add(buf, 0, buf.readableBytes())) { + // loop until we filled it. + } + + assertNull(arrays.next()); + + arrays.clear(); + + // We should start again from idx 0 + assertSame(next, arrays.next()); + } finally { + arrays.release(); + buf.release(); + } + } +}