From 12c9ce7f679cc593ea7e902302cef756cd2bef05 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 28 Oct 2014 19:42:07 +0100 Subject: [PATCH] Allow to lazy create a DefaultFileRegion from a File Motivation: We only provided a constructor in DefaultFileRegion that takes a FileChannel which means the File itself needs to get opened on construction. This has the problem that if you want to write a lot of Files very fast you may end up with may open FD's even if they are not needed yet. This can lead to hit the open FD limit of the OS. Modifications: Add a new constructor to DefaultFileRegion which allows to construct it from a File. The FileChannel will only be obtained when transferTo(...) is called or the DefaultFileRegion is explicit open'ed via open() (this is needed for the native epoll transport) Result: Less resource usage when writing a lot of DefaultFileRegion. --- .../main/c/io_netty_channel_epoll_Native.c | 2 +- .../main/c/io_netty_channel_epoll_Native.h | 2 +- .../java/io/netty/channel/epoll/Native.java | 11 +++- .../io/netty/channel/DefaultFileRegion.java | 62 ++++++++++++++++++- 4 files changed, 71 insertions(+), 6 deletions(-) diff --git a/transport-native-epoll/src/main/c/io_netty_channel_epoll_Native.c b/transport-native-epoll/src/main/c/io_netty_channel_epoll_Native.c index 2a06c2267c..24092d03c9 100644 --- a/transport-native-epoll/src/main/c/io_netty_channel_epoll_Native.c +++ b/transport-native-epoll/src/main/c/io_netty_channel_epoll_Native.c @@ -1101,7 +1101,7 @@ JNIEXPORT jint JNICALL Java_io_netty_channel_epoll_Native_accept(JNIEnv * env, j return socketFd; } -JNIEXPORT jlong JNICALL Java_io_netty_channel_epoll_Native_sendfile(JNIEnv *env, jclass clazz, jint fd, jobject fileRegion, jlong base_off, jlong off, jlong len) { +JNIEXPORT jlong JNICALL Java_io_netty_channel_epoll_Native_sendfile0(JNIEnv *env, jclass clazz, jint fd, jobject fileRegion, jlong base_off, jlong off, jlong len) { jobject fileChannel = (*env)->GetObjectField(env, fileRegion, fileChannelFieldId); if (fileChannel == NULL) { throwRuntimeException(env, "Unable to obtain FileChannel from FileRegion"); diff --git a/transport-native-epoll/src/main/c/io_netty_channel_epoll_Native.h b/transport-native-epoll/src/main/c/io_netty_channel_epoll_Native.h index a6e9fb04aa..92f6ee627b 100644 --- a/transport-native-epoll/src/main/c/io_netty_channel_epoll_Native.h +++ b/transport-native-epoll/src/main/c/io_netty_channel_epoll_Native.h @@ -69,7 +69,7 @@ void Java_io_netty_channel_epoll_Native_listen(JNIEnv * env, jclass clazz, jint jboolean Java_io_netty_channel_epoll_Native_connect(JNIEnv * env, jclass clazz, jint fd, jbyteArray address, jint scopeId, jint port); jboolean Java_io_netty_channel_epoll_Native_finishConnect(JNIEnv * env, jclass clazz, jint fd); jint Java_io_netty_channel_epoll_Native_accept(JNIEnv * env, jclass clazz, jint fd); -jlong Java_io_netty_channel_epoll_Native_sendfile(JNIEnv *env, jclass clazz, jint fd, jobject fileRegion, jlong base_off, jlong off, jlong len); +jlong Java_io_netty_channel_epoll_Native_sendfile0(JNIEnv *env, jclass clazz, jint fd, jobject fileRegion, jlong base_off, jlong off, jlong len); jobject Java_io_netty_channel_epoll_Native_remoteAddress(JNIEnv * env, jclass clazz, jint fd); jobject Java_io_netty_channel_epoll_Native_localAddress(JNIEnv * env, jclass clazz, jint fd); void Java_io_netty_channel_epoll_Native_setReuseAddress(JNIEnv * env, jclass clazz, jint fd, jint optval); diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java index 5bbd890692..9b6894ee4f 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java @@ -77,7 +77,16 @@ final class Native { public static native int read(int fd, ByteBuffer buf, int pos, int limit) throws IOException; public static native int readAddress(int fd, long address, int pos, int limit) throws IOException; - public static native long sendfile( + public static long sendfile( + int dest, DefaultFileRegion src, long baseOffset, long offset, long length) throws IOException { + // Open the file-region as it may be created via the lazy constructor. This is needed as we directly access + // the FileChannel field directly via JNI + src.open(); + + return sendfile0(dest, src, baseOffset, offset, length); + } + + private static native long sendfile0( int dest, DefaultFileRegion src, long baseOffset, long offset, long length) throws IOException; public static int sendTo( diff --git a/transport/src/main/java/io/netty/channel/DefaultFileRegion.java b/transport/src/main/java/io/netty/channel/DefaultFileRegion.java index 75ef56a9f0..d9a40c3369 100644 --- a/transport/src/main/java/io/netty/channel/DefaultFileRegion.java +++ b/transport/src/main/java/io/netty/channel/DefaultFileRegion.java @@ -16,15 +16,18 @@ package io.netty.channel; import io.netty.util.AbstractReferenceCounted; +import io.netty.util.IllegalReferenceCountException; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; +import java.io.File; import java.io.IOException; +import java.io.RandomAccessFile; import java.nio.channels.FileChannel; import java.nio.channels.WritableByteChannel; /** - * Default {@link FileRegion} implementation which transfer data from a {@link FileChannel}. + * Default {@link FileRegion} implementation which transfer data from a {@link FileChannel} or {@link File}. * * Be aware that the {@link FileChannel} will be automatically closed once {@link #refCnt()} returns * {@code 0}. @@ -32,11 +35,11 @@ import java.nio.channels.WritableByteChannel; public class DefaultFileRegion extends AbstractReferenceCounted implements FileRegion { private static final InternalLogger logger = InternalLoggerFactory.getInstance(DefaultFileRegion.class); - - private final FileChannel file; + private final File f; private final long position; private final long count; private long transfered; + private FileChannel file; /** * Create a new instance @@ -58,6 +61,47 @@ public class DefaultFileRegion extends AbstractReferenceCounted implements FileR this.file = file; this.position = position; this.count = count; + f = null; + } + + /** + * Create a new instance using the given {@link File}. The {@link File} will be opened lazily or + * explicitly via {@link #open()}. + * + * @param f the {@link File} which should be transfered + * @param position the position from which the transfer should start + * @param count the number of bytes to transfer + */ + public DefaultFileRegion(File f, long position, long count) { + if (f == null) { + throw new NullPointerException("f"); + } + if (position < 0) { + throw new IllegalArgumentException("position must be >= 0 but was " + position); + } + if (count < 0) { + throw new IllegalArgumentException("count must be >= 0 but was " + count); + } + this.position = position; + this.count = count; + this.f = f; + } + + /** + * Returns {@code true} if the {@link FileRegion} has a open file-descriptor + */ + public boolean isOpen() { + return file != null; + } + + /** + * Explicitly open the underlying file-descriptor if not done yet. + */ + public void open() throws IOException { + if (!isOpen() && refCnt() > 0) { + // Only open if this DefaultFileRegion was not released yet. + file = new RandomAccessFile(f, "r").getChannel(); + } } @Override @@ -86,6 +130,11 @@ public class DefaultFileRegion extends AbstractReferenceCounted implements FileR if (count == 0) { return 0L; } + if (refCnt() == 0) { + throw new IllegalReferenceCountException(0); + } + // Call open to make sure fc is initialized. This is a no-oop if we called it before. + open(); long written = file.transferTo(this.position + position, count, target); if (written > 0) { @@ -96,6 +145,13 @@ public class DefaultFileRegion extends AbstractReferenceCounted implements FileR @Override protected void deallocate() { + FileChannel file = this.file; + + if (file == null) { + return; + } + this.file = null; + try { file.close(); } catch (IOException e) {