[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers
franz1981 commented on a change in pull request #2844: ARTEMIS-1811 NIO Seq
File should use RandomAccessFile with heap buffers
URL: https://github.com/apache/activemq-artemis/pull/2844#discussion_r327938663
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/nio/NIOSequentialFile.java
##
@@ -160,14 +173,63 @@ public int read(final ByteBuffer bytes) throws Exception
{
return read(bytes, null);
}
+ //This value has been tuned just to reduce the memory footprint
+ //of read/write of the whole file size: given that this value
+ //is > 8192, RandomAccessFile JNI code will use malloc/free instead
+ //of using a copy on the stack, but it has been proven to NOT be
+ //a bottleneck
+ private static final int BUF_SIZE = 2 * 1024 * 1024;
+
+ private static int readRaf(RandomAccessFile raf, byte[] b, int off, int
len, int ioSize) throws IOException {
+ int remaining = len;
+ int offset = off;
+ while (remaining > 0) {
+ final int chunkSize = Math.min(ioSize, remaining);
+ final int read = raf.read(b, offset, chunkSize);
+ assert read != 0;
+ if (read == -1) {
+if (len == remaining) {
+ return -1;
+}
+break;
+ }
+ offset += read;
+ remaining -= read;
+ }
+ return len - remaining;
+ }
+
+ private static void writeRaf(RandomAccessFile raf, byte[] b, int off, int
len, int ioSize) throws IOException {
+ int remaining = len;
+ int offset = off;
+ while (remaining > 0) {
+ final int chunkSize = Math.min(ioSize, remaining);
+ raf.write(b, offset, chunkSize);
+ offset += chunkSize;
+ remaining -= chunkSize;
+ }
+ }
+
@Override
public synchronized int read(final ByteBuffer bytes,
final IOCallback callback) throws IOException,
ActiveMQIllegalStateException {
try {
if (channel == null) {
throw new ActiveMQIllegalStateException("File " +
this.getFileName() + " has a null channel");
}
- int bytesRead = channel.read(bytes);
+ final int bytesRead;
+ if (bytes.hasArray()) {
+if (bytes.remaining() > BUF_SIZE) {
+ bytesRead = readRaf(rfile, bytes.array(), bytes.arrayOffset() +
bytes.position(), bytes.remaining(), BUF_SIZE);
+} else {
+ bytesRead = rfile.read(bytes.array(), bytes.arrayOffset() +
bytes.position(), bytes.remaining());
Review comment:
to save inlining budget: for most of the cases where the first check will
save the call on the happy path
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers
franz1981 commented on a change in pull request #2844: ARTEMIS-1811 NIO Seq
File should use RandomAccessFile with heap buffers
URL: https://github.com/apache/activemq-artemis/pull/2844#discussion_r327938663
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/nio/NIOSequentialFile.java
##
@@ -160,14 +173,63 @@ public int read(final ByteBuffer bytes) throws Exception
{
return read(bytes, null);
}
+ //This value has been tuned just to reduce the memory footprint
+ //of read/write of the whole file size: given that this value
+ //is > 8192, RandomAccessFile JNI code will use malloc/free instead
+ //of using a copy on the stack, but it has been proven to NOT be
+ //a bottleneck
+ private static final int BUF_SIZE = 2 * 1024 * 1024;
+
+ private static int readRaf(RandomAccessFile raf, byte[] b, int off, int
len, int ioSize) throws IOException {
+ int remaining = len;
+ int offset = off;
+ while (remaining > 0) {
+ final int chunkSize = Math.min(ioSize, remaining);
+ final int read = raf.read(b, offset, chunkSize);
+ assert read != 0;
+ if (read == -1) {
+if (len == remaining) {
+ return -1;
+}
+break;
+ }
+ offset += read;
+ remaining -= read;
+ }
+ return len - remaining;
+ }
+
+ private static void writeRaf(RandomAccessFile raf, byte[] b, int off, int
len, int ioSize) throws IOException {
+ int remaining = len;
+ int offset = off;
+ while (remaining > 0) {
+ final int chunkSize = Math.min(ioSize, remaining);
+ raf.write(b, offset, chunkSize);
+ offset += chunkSize;
+ remaining -= chunkSize;
+ }
+ }
+
@Override
public synchronized int read(final ByteBuffer bytes,
final IOCallback callback) throws IOException,
ActiveMQIllegalStateException {
try {
if (channel == null) {
throw new ActiveMQIllegalStateException("File " +
this.getFileName() + " has a null channel");
}
- int bytesRead = channel.read(bytes);
+ final int bytesRead;
+ if (bytes.hasArray()) {
+if (bytes.remaining() > BUF_SIZE) {
+ bytesRead = readRaf(rfile, bytes.array(), bytes.arrayOffset() +
bytes.position(), bytes.remaining(), BUF_SIZE);
+} else {
+ bytesRead = rfile.read(bytes.array(), bytes.arrayOffset() +
bytes.position(), bytes.remaining());
Review comment:
to save inlining budget: for most of the cases the first check will save the
call on the happy path
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers
franz1981 commented on a change in pull request #2844: ARTEMIS-1811 NIO Seq
File should use RandomAccessFile with heap buffers
URL: https://github.com/apache/activemq-artemis/pull/2844#discussion_r327938663
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/nio/NIOSequentialFile.java
##
@@ -160,14 +173,63 @@ public int read(final ByteBuffer bytes) throws Exception
{
return read(bytes, null);
}
+ //This value has been tuned just to reduce the memory footprint
+ //of read/write of the whole file size: given that this value
+ //is > 8192, RandomAccessFile JNI code will use malloc/free instead
+ //of using a copy on the stack, but it has been proven to NOT be
+ //a bottleneck
+ private static final int BUF_SIZE = 2 * 1024 * 1024;
+
+ private static int readRaf(RandomAccessFile raf, byte[] b, int off, int
len, int ioSize) throws IOException {
+ int remaining = len;
+ int offset = off;
+ while (remaining > 0) {
+ final int chunkSize = Math.min(ioSize, remaining);
+ final int read = raf.read(b, offset, chunkSize);
+ assert read != 0;
+ if (read == -1) {
+if (len == remaining) {
+ return -1;
+}
+break;
+ }
+ offset += read;
+ remaining -= read;
+ }
+ return len - remaining;
+ }
+
+ private static void writeRaf(RandomAccessFile raf, byte[] b, int off, int
len, int ioSize) throws IOException {
+ int remaining = len;
+ int offset = off;
+ while (remaining > 0) {
+ final int chunkSize = Math.min(ioSize, remaining);
+ raf.write(b, offset, chunkSize);
+ offset += chunkSize;
+ remaining -= chunkSize;
+ }
+ }
+
@Override
public synchronized int read(final ByteBuffer bytes,
final IOCallback callback) throws IOException,
ActiveMQIllegalStateException {
try {
if (channel == null) {
throw new ActiveMQIllegalStateException("File " +
this.getFileName() + " has a null channel");
}
- int bytesRead = channel.read(bytes);
+ final int bytesRead;
+ if (bytes.hasArray()) {
+if (bytes.remaining() > BUF_SIZE) {
+ bytesRead = readRaf(rfile, bytes.array(), bytes.arrayOffset() +
bytes.position(), bytes.remaining(), BUF_SIZE);
+} else {
+ bytesRead = rfile.read(bytes.array(), bytes.arrayOffset() +
bytes.position(), bytes.remaining());
Review comment:
to save inlining budget: for most of the cases the first check will safe the
call on the happy path
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers
franz1981 commented on a change in pull request #2844: ARTEMIS-1811 NIO Seq
File should use RandomAccessFile with heap buffers
URL: https://github.com/apache/activemq-artemis/pull/2844#discussion_r326859418
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/nio/NIOSequentialFile.java
##
@@ -173,6 +173,40 @@ public int read(final ByteBuffer bytes) throws Exception {
return read(bytes, null);
}
+ //On *nix platform is the size by which RandomAccessFile write/read are
performed using a
+ //stack-allocated copy of the java byte[]
Review comment:
I've verified that is a the same behaviour for Windows: it is in the shared
JNI code by any OSs
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers
franz1981 commented on a change in pull request #2844: ARTEMIS-1811 NIO Seq
File should use RandomAccessFile with heap buffers
URL: https://github.com/apache/activemq-artemis/pull/2844#discussion_r326849697
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/nio/NIOSequentialFile.java
##
@@ -173,6 +173,40 @@ public int read(final ByteBuffer bytes) throws Exception {
return read(bytes, null);
}
+ //On *nix platform is the size by which RandomAccessFile write/read are
performed using a
+ //stack-allocated copy of the java byte[]
Review comment:
This was just a POC commit to mitigate the issue , but we can discuss on the
comments about it :)
Thanks to looking at it
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
