[GitHub] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251200554
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/nio/NIOSequentialFile.java
##
@@ -51,6 +55,34 @@ public NIOSequentialFile(final SequentialFileFactory
factory,
this.maxIO = maxIO;
}
+ @Override
+ protected TimedBufferObserver createTimedBufferObserver() {
+ return new LocalBufferObserver() {
Review comment:
Good point about stacks trace bud, will do it immediately!
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251192884
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/nio/NIOSequentialFile.java
##
@@ -51,6 +55,34 @@ public NIOSequentialFile(final SequentialFileFactory
factory,
this.maxIO = maxIO;
}
+ @Override
+ protected TimedBufferObserver createTimedBufferObserver() {
+ return new LocalBufferObserver() {
Review comment:
I can do it, but (for this specific class) I can't see the benefits.
In a future implementation I would like to abstract away things like
`AbstractSequentialFile::fileSize` and create 2 separate classes:
`AsyncLocalBufferObserver` and `SyncLocalBufferObserver`.
The former that perform the copy because it assumes asynchrounous writes,
the latter not.
But at this point, given that the classes are packed into their owner seq
file class probably I could just let them be where/as they are to reduce the
code changes.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251192884
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/nio/NIOSequentialFile.java
##
@@ -51,6 +55,34 @@ public NIOSequentialFile(final SequentialFileFactory
factory,
this.maxIO = maxIO;
}
+ @Override
+ protected TimedBufferObserver createTimedBufferObserver() {
+ return new LocalBufferObserver() {
Review comment:
I can do it, but (for this specific class) I can't see the benefits.
In a future implementation I would like to abstract away things like
`AbstractSequentialFile::fileSize` and create 2 separate classes:
`AsyncLocalBufferObserver` and `SyncLocalBufferObserver`.
The former that perform the copy because it assymes asynchrounous writes,
the latter not.
But at this point, given that the classes are packed into their owner seq
file class probably I could just let them be where/as they are to reduce the
code changes.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251192771
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/AbstractSequentialFile.java
##
@@ -301,21 +269,19 @@ protected ByteBuffer newBuffer(int size, int limit) {
protected class LocalBufferObserver implements TimedBufferObserver {
Review comment:
It is not easy: it's `Local` to the owner class in the worst way: it use
directly private files as `fileSize` that has a very tricky semantic around. I
prefer to left it to stay where it is to reduce the amount of changes.
I could put it as package private class, but that means to expose (on
package level) some internals of `AbstractSequentialFile` that's not a good
idea IMHO.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251192771
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/AbstractSequentialFile.java
##
@@ -301,21 +269,19 @@ protected ByteBuffer newBuffer(int size, int limit) {
protected class LocalBufferObserver implements TimedBufferObserver {
Review comment:
It is not easy: it's `Local` to the owner class in the worst way: it use
directly private files as `fileSize` that has a very tricky semantic around. I
prefer to left it to stay where it is to reduce the amount of changes.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251192720
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/DelegateCallback.java
##
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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 org.apache.activemq.artemis.core.io;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.activemq.artemis.journal.ActiveMQJournalLogger;
+
+/**
+ * It is a utility class to allow several {@link IOCallback}s to be used as
one.
+ */
+public final class DelegateCallback implements IOCallback {
+
+ private final List delegates;
+
+ /**
+* It doesn't copy defensively the given {@code delegates}.
+*
+* @throws NullPointerException if {@code delegates} is {@code null}
+*/
+ public DelegateCallback(final List delegates) {
+ Objects.requireNonNull(delegates, "delegates cannot be null!");
+ this.delegates = delegates;
+ }
+
+ @Override
+ public void done() {
+ done(delegates);
+ }
+
+ @Override
+ public void onError(final int errorCode, final String errorMessage) {
+ onError(delegates, errorCode, errorMessage);
+ }
+
+ public static void done(Collection delegates) {
Review comment:
:+1: Yep, good idea! And I could move `DelegateCallback` too to be near
`IOCallback` too
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251009673
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
It has been solved on the `AbstractSequentialFile`: ASYNCIO is doing it
exactly like before (with the copy), while NIO/MAPPED avoid the copy by using a
specialized class.
For ASYNCIO:
https://github.com/apache/activemq-artemis/blob/e57de09c6fb49783301a84bff68cd4618473f476/artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/AbstractSequentialFile.java#L275
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251014507
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
> What happens to Mapped
It will use another wrapper I've written some time ago, because
`MappedSequentialFile` isn't a child of `AbstractSequentialFile`, here:
https://github.com/apache/activemq-artemis/pull/2522/files#diff-6707e3363162061f97af51ef7f8d13a4R256
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251014507
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
> What happens to Mapped
It will use another wrapper I've written some time ago, because
`MappedSequentialFile` isn't a child of 'AbstractSequentialFile`, here:
https://github.com/apache/activemq-artemis/pull/2522/files#diff-6707e3363162061f97af51ef7f8d13a4R256
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251014507
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
> What happens to Mapped
It will use another wrapper I've written some time ago, because
`MappedSequentialFile` isn't a child of 'AbstractSequentialFile`, here:
https://github.com/apache/activemq-artemis/pull/2522/files#diff-6707e3363162061f97af51ef7f8d13a4R256
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251011676
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
By default `AbstractSequentialFile` cannot say if the underline file is
performing async or sync writes, hence it will copy it, like it was done in the
past on `TimedBuffer`, while NIO/MAPPED can assume that writes are synchronous
and they will avoid the copy (if possilble; there are some contraints I've
written on the code).
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251009673
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
It has been solved on the `AbstractSequentialFile`: ASYNCIO is doing is
exactly like before (with the copy), while NIO/MAPPED avoid the copy by using a
specialized class.
For ASYNCIO:
https://github.com/apache/activemq-artemis/blob/e57de09c6fb49783301a84bff68cd4618473f476/artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/AbstractSequentialFile.java#L275
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251011676
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
By default `AbstractSequentialFile` cannot say if the underline file is
performing async or sync writes, hence it just copy it, like it was done in the
past on `TimedBuffer`, while NIO/MAPPED can assume that writes are synchronous
and they will avoid the copy (if possilble, there are some contraints I've
written on the code).
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251011676
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
By default `AbstractSequentialFile` cannot say if the underline file is
performing async or sync writes, hence it just copy it, like it was done in the
past on `TimedBuffer`, while NIO/MAPPED can assume that writes are synchronous
and they will avoid the copy (if possilble; there are some contraints I've
written on the code).
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251009673
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
It has been solved on the abstract seq file Local: ASYNCIO is doing is
exactly like before (with the copy), while NIO/MAPPED avoid the copy by using a
specialized class.
For ASYNCIO:
https://github.com/apache/activemq-artemis/blob/e57de09c6fb49783301a84bff68cd4618473f476/artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/AbstractSequentialFile.java#L275
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251009673
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
It has been solved on the abstract seq file Local: ASYNCIO is doing is
exactly like before (with the copy), while NIO/MAPPED avoid the copy by using a
specialized class.
For ASYNCIO:
```java
@Override
public void flushBuffer(final ByteBuffer buffer, final boolean
requestedSync, final List callbacks) {public void
flushBuffer(final ByteBuf byteBuf, final boolean requestedSync, final
List callbacks) {
buffer.flip(); final int bytes =
byteBuf.readableBytes();
if (bytes > 0) {
if (buffer.limit() == 0) { final ByteBuffer buffer =
newBuffer(byteBuf.capacity(), bytes);
factory.releaseBuffer(buffer); buffer.limit(bytes);
} else {byteBuf.getBytes(byteBuf.readerIndex(),
buffer);
buffer.flip();
writeDirect(buffer, requestedSync, new
DelegateCallback(callbacks)); writeDirect(buffer,
requestedSync, new DelegateCallback(callbacks));
} else {
DelegateCallback.done(callbacks);
}}
}}
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251009673
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
It has been solved on the abstract seq file Local: ASYNCIO is doing is
exactly like before (with the copy), while NIO/MAPPED avoid the copy by using a
specialized class.
For ASYNCIO:
https://github.com/apache/activemq-artemis/pull/2522/files#diff-e39fc5f44901194cbd6d29fb1b08e0ddR275
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on a change in pull request #2522: ARTEMIS-2239 Zero-copy
NIO/MAPPED TimedBuffer
URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251009673
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/buffer/TimedBuffer.java
##
@@ -358,13 +358,7 @@ public boolean flushBatch() {
bytesFlushed.addAndGet(pos);
}
- final ByteBuffer bufferToFlush =
bufferObserver.newBuffer(bufferSize, pos);
- //bufferObserver::newBuffer doesn't necessary return a buffer
with limit == pos or limit == bufferSize!!
- bufferToFlush.limit(pos);
- //perform memcpy under the hood due to the off heap buffer
- buffer.getBytes(0, bufferToFlush);
-
- bufferObserver.flushBuffer(bufferToFlush, pendingSync,
callbacks);
+ bufferObserver.flushBuffer(buffer.byteBuf(), pendingSync,
callbacks);
Review comment:
It has been solved on the abstract seq file: ASYNCIO is doing is exactly
like before (with the copy), while NIO/MAPPED avoid the copy by using a
specialized class
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
