[GitHub] [activemq-nms-amqp] Havret edited a comment on issue #34: NO-JIRA: Enable interop tests for Travis
Havret edited a comment on issue #34: NO-JIRA: Enable interop tests for Travis URL: https://github.com/apache/activemq-nms-amqp/pull/34#issuecomment-533919613 @michaelandrepearce Is it available on docker hub? 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-nms-amqp] Havret commented on issue #28: AMQNET-614: Extend logging
Havret commented on issue #28: AMQNET-614: Extend logging URL: https://github.com/apache/activemq-nms-amqp/pull/28#issuecomment-533964508 Done. 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-nms-amqp] michaelandrepearce commented on issue #34: NO-JIRA: Enable interop tests for Travis
michaelandrepearce commented on issue #34: NO-JIRA: Enable interop tests for Travis URL: https://github.com/apache/activemq-nms-amqp/pull/34#issuecomment-533963429 No but you can build on fly or work with artemis for next release to get it so 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] gaohoward opened a new pull request #2846: ARTEMIS-2500 CoreMessage doesn't make a ful copy of its props
gaohoward opened a new pull request #2846: ARTEMIS-2500 CoreMessage doesn't make a ful copy of its props URL: https://github.com/apache/activemq-artemis/pull/2846 When CoreMessage is doing copyHeadersAndProperties() it doesn't make a full copy of its properties (a TypedProperties object). It will cause problem when multiple threads/parties are modifying the properties of the copied messages from the same message. 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 edited a comment on issue #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers
franz1981 edited a comment on issue #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers URL: https://github.com/apache/activemq-artemis/pull/2844#issuecomment-533906809 I'm going to "tune" differently the optimization while reading/writing RandonAccessFile in chunks of 8192, because on https://github.com/netty/netty/pull/9591#issuecomment-533904873 I've measured that the allocator isn't a bottleneck when chunk size > 8192 and assuming single threaded usage (malloc -> free on the same thread). ATM AMQP and OpenWire could cause big buffers allocation on native space (on JNI), so I would reduce the impact of this by tuning chunk size just to reduce the foot print overhead of 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
[GitHub] [activemq-nms-amqp] Havret commented on issue #34: NO-JIRA: Enable interop tests for Travis
Havret commented on issue #34: NO-JIRA: Enable interop tests for Travis URL: https://github.com/apache/activemq-nms-amqp/pull/34#issuecomment-533919613 It it available on docker hub? 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-nms-amqp] michaelandrepearce commented on issue #28: NO-JIRA: Extend logging
michaelandrepearce commented on issue #28: NO-JIRA: Extend logging URL: https://github.com/apache/activemq-nms-amqp/pull/28#issuecomment-533918205 Jira item needed. Also squash commits pls 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-nms-amqp] michaelandrepearce commented on issue #34: NO-JIRA: Enable interop tests for Travis
michaelandrepearce commented on issue #34: NO-JIRA: Enable interop tests for Travis URL: https://github.com/apache/activemq-nms-amqp/pull/34#issuecomment-533918027 https://github.com/apache/activemq-artemis/tree/master/artemis-docker 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-nms-amqp] michaelandrepearce merged pull request #33: NO-JIRA: Update docs
michaelandrepearce merged pull request #33: NO-JIRA: Update docs URL: https://github.com/apache/activemq-nms-amqp/pull/33 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-nms-amqp] michaelandrepearce commented on issue #33: NO-JIRA: Update docs
michaelandrepearce commented on issue #33: NO-JIRA: Update docs URL: https://github.com/apache/activemq-nms-amqp/pull/33#issuecomment-533917913 Ill merge this but anything more than a few lines change does typically need a jira 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-nms-amqp] michaelandrepearce edited a comment on issue #34: NO-JIRA: Enable interop tests for Travis
michaelandrepearce edited a comment on issue #34: NO-JIRA: Enable interop tests for Travis URL: https://github.com/apache/activemq-nms-amqp/pull/34#issuecomment-533917673 @havret we should use the apache artemis docker build. Not an external one. If features or parts are missing they should be contributed to the artemis code base 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-nms-amqp] michaelandrepearce commented on issue #34: NO-JIRA: Enable interop tests for Travis
michaelandrepearce commented on issue #34: NO-JIRA: Enable interop tests for Travis URL: https://github.com/apache/activemq-nms-amqp/pull/34#issuecomment-533917673 @havret we should use the apache artemis docker build. Not an external one 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 edited a comment on issue #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers
franz1981 edited a comment on issue #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers URL: https://github.com/apache/activemq-artemis/pull/2844#issuecomment-533906809 I'm going to "tune" the optimization while reading/writing RandonAccessFile in chunks of 8192, because on https://github.com/netty/netty/pull/9591#issuecomment-533904873 I've measured that the allocator isn't a bottleneck when chunk size > 8192 and assuming single threaded usage (malloc -> free on the same thread). ATM AMQP and OpenWire could cause big buffers allocation on native space (on JNI), so I would reduce the impact of this by tuning chunk size just to reduce the foot print overhead of 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
[GitHub] [activemq-artemis] franz1981 commented on issue #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers
franz1981 commented on issue #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers URL: https://github.com/apache/activemq-artemis/pull/2844#issuecomment-533906809 I'm going to "tune" the optimization while reading/writing RandonAccessFile in chunks of 8192, because on https://github.com/netty/netty/pull/9591#issuecomment-533904873 I've measured that the allocator isn't a bottleneck when chunk size > 8192 and assuming single threaded usage (malloc -> free on the same thread). ATM AMQP and OpenWire could cause big buffers allocation on native space (on JNI), so I would reduce the impact of this by tuning chunk size just to reduce the foot print over head of 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
[GitHub] [activemq-nms-amqp] Havret opened a new pull request #36: [WIP] Transaction Integration Tests
Havret opened a new pull request #36: [WIP] Transaction Integration Tests URL: https://github.com/apache/activemq-nms-amqp/pull/36 As we have testing framework finally available (new TestAmqpPeer implementation), I started adding missing integration tests for transactions. 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 #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 commented on a change in pull request #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#discussion_r326903252 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ChannelImpl.java ## @@ -151,6 +151,11 @@ public ChannelImpl(final CoreRemotingConnection connection, } this.interceptors = interceptors; + //zero copy transfer is initialized only for replication channels Review comment: Right now is not and I've added an assert to make the test suite to fail when is not properly configured, but we can always configure Netty pipeline upfront (right after SSL handler) to handle it without affecting normal usage, If is more clear.. 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-nms-amqp] Havret edited a comment on issue #28: NO-JIRA: Extend logging
Havret edited a comment on issue #28: NO-JIRA: Extend logging URL: https://github.com/apache/activemq-nms-amqp/pull/28#issuecomment-533896432 @cjwmorgan-sol Thank you very much for your feedback. I've added message tracing support to our backlog https://issues.apache.org/jira/browse/AMQNET-613 @michaelandrepearce It would be great if we have this merged any time soon. 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-nms-amqp] Havret commented on issue #28: NO-JIRA: Extend logging
Havret commented on issue #28: NO-JIRA: Extend logging URL: https://github.com/apache/activemq-nms-amqp/pull/28#issuecomment-533896432 @cjwmorgan-sol Thank you very much for your feedback. I've added message tracing support to our backlog https://issues.apache.org/jira/browse/AMQNET-613 @michaelandrepearce It would be great if we have this merged any time soon. 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 issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-533885763 @clebertsuconic yes, agree and I would like to run a soak test + wait @wy96f results as well :+1 The wildfly tests seems pretty stable with this, but we have many of them to be tried first 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 edited a comment on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 edited a comment on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-533885763 @clebertsuconic yes, agree and I would like to run a soak test + wait @wy96f results as well +1 The wildfly tests seems pretty stable with this, but we have many of them to be tried first 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 #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 commented on a change in pull request #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#discussion_r326903252 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ChannelImpl.java ## @@ -151,6 +151,11 @@ public ChannelImpl(final CoreRemotingConnection connection, } this.interceptors = interceptors; + //zero copy transfer is initialized only for replication channels Review comment: Right now is not and I've added an assert to make the test suite to fail on it, but we can always configure the Netty pipeline to handle them with no harm, If is more clear.. 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] clebertsuconic commented on a change in pull request #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
clebertsuconic commented on a change in pull request #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#discussion_r326903004 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ChannelImpl.java ## @@ -151,6 +151,11 @@ public ChannelImpl(final CoreRemotingConnection connection, } this.interceptors = interceptors; + //zero copy transfer is initialized only for replication channels Review comment: Yeah.. this looks like a Hack to me! when is sendFiles used without being the replica channel? 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] clebertsuconic commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
clebertsuconic commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-533884923 lets wait the release I'm doing on monday before we can start considering this? We will need to test it within Wildfly to make sure it won't cause an issue in there. 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 edited a comment on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 edited a comment on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-533874887 @ehsavoie @clebertsuconic FYI this version is not failing anymore the wildfly tests on https://issues.apache.org/jira/browse/ARTEMIS-2496 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 issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-533881207 @wy96f I remember you've provided some numbers for ARTEMIS-2336 using one of yours load generator: it would be nice to compare the results with this pr using `-Dio.netty.file.region=true|false` and vs master to verify that `-Dio.netty.file.region=false` is not worst then master. 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 edited a comment on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 edited a comment on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-533881207 @wy96f I remember you've provided some numbers for ARTEMIS-2336 using one of your load generator: it would be nice to compare the results with this pr using `-Dio.netty.file.region=true|false` and vs master to verify that `-Dio.netty.file.region=false` is not worst then master. 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 #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 commented on a change in pull request #2845: ARTEMIS-2336 Use zero
copy to replicate journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#discussion_r326897522
##
File path:
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnection.java
##
@@ -355,12 +373,24 @@ private boolean canWrite(final int requiredCapacity) {
return canWrite;
}
- private Object getFileObject(RandomAccessFile raf, FileChannel fileChannel,
long offset, int dataSize) {
- if (channel.pipeline().get(SslHandler.class) == null) {
- return new NonClosingDefaultFileRegion(fileChannel, offset, dataSize);
+ private Object getFileObject(FileChannel fileChannel, long offset, int
dataSize) {
+ if (USE_FILE_REGION && channel.pipeline().get(SslHandler.class) == null)
{
Review comment:
`channel.pipeline().get(SslHandler.class)` could be costy: @wy96f we don't
have any chance to evaluate this before?
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 #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 commented on a change in pull request #2845: ARTEMIS-2336 Use zero
copy to replicate journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#discussion_r326896991
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/ReplicationSyncFileMessage.java
##
@@ -258,7 +258,19 @@ public void decodeRest(final ActiveMQBuffer buffer) {
private void readFile(ByteBuffer buffer) {
try {
- fileChannel.read(buffer, offset);
+ if (buffer.hasArray()) {
+raf.seek(offset);
+final int position = buffer.position();
+final int read = raf.read(buffer.array(), buffer.arrayOffset() +
position, buffer.remaining());
Review comment:
This one should use the same optimization on
https://github.com/apache/activemq-artemis/pull/2844
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 #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 commented on a change in pull request #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#discussion_r326896884 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ChannelImpl.java ## @@ -151,6 +151,11 @@ public ChannelImpl(final CoreRemotingConnection connection, } this.interceptors = interceptors; + //zero copy transfer is initialized only for replication channels Review comment: @clebertsuconic This one is ugly I know :) We can find a better way together ;) 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 issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-533874887 @ehsavoie @clebertsuconic This version is not failing anymore the wildfly tests on https://issues.apache.org/jira/browse/ARTEMIS-2496 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 opened a new pull request #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
franz1981 opened a new pull request #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845 I've opened this PR for discussion: I would like to re-introduce ARTEMIS-2336, but I've allowed wildfly or any user that doesn't want/can to use zero copy to be able to use the existing artemis code. I've opened https://github.com/netty/netty/pull/9592 too to "enhance" `ChunkedNioFile` in order to solve a bug on our implementation: in the meantime I've "shadowed" my solution directly into `AbsoluteChunkedNioFile` to not rely on any specific Netty version. This PR could make use for InVM connection the same optimization sent on https://github.com/apache/activemq-artemis/pull/2844 while reading file (RandomAccessFile) on `ReplicationSyncFileMessage`. 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-nms-amqp] Havret commented on issue #34: NO-JIRA: Enable interop tests for Travis
Havret commented on issue #34: NO-JIRA: Enable interop tests for Travis URL: https://github.com/apache/activemq-nms-amqp/pull/34#issuecomment-533868994 I ignored failing test. It should be restored after https://github.com/vromero/activemq-artemis-docker/pull/124 is merged and we can use Artemis 2.10.0 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 edited a comment on issue #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers
franz1981 edited a comment on issue #2844: ARTEMIS-1811 NIO Seq File should use RandomAccessFile with heap buffers URL: https://github.com/apache/activemq-artemis/pull/2844#issuecomment-533606931 @wy96f @clebertsuconic Just for completeness that's what would change: - [Java_java_io_RandomAccessFile_writeBytes->writeBytes](https://github.com/frohoff/jdk8u-jdk/blob/da0da73ab82ed714dc5be94acd2f0d00fbdfe2e9/src/share/native/java/io/RandomAccessFile.c#L85) - [writeBytes->IO_Write](https://github.com/frohoff/jdk8u-jdk/blob/da0da73ab82ed714dc5be94acd2f0d00fbdfe2e9/src/share/native/java/io/io_util.c#L189) by using a stack buffer if write/read length is <= `BUF_SIZE` (ie 8192 bytes) or with a fresh new buffer allocated using malloc/free: in both cases GetByteArrayRegion/SetByteArrayRegion are used to perform a copy from/to the provided java `byte[]` - [IO_Write === handleWrite](https://github.com/frohoff/jdk8u-jdk/blob/da0da73ab82ed714dc5be94acd2f0d00fbdfe2e9/src/solaris/native/java/io/io_util_md.h#L71) - [handleWrite->write](https://github.com/frohoff/jdk8u-jdk/blob/da0da73ab82ed714dc5be94acd2f0d00fbdfe2e9/src/solaris/native/java/io/io_util_md.c#L164) I see that this PR has few advantages vs https://github.com/apache/activemq-artemis/pull/2832: - it is simpler/less impactfull on artemis code base - although copies always happen, no leaks can happen I see that for small writes/reads the perf hit is not very high because copy java byte[]<->stack allocated native byte[] copy is cheap if compared to the cost of the write/read syscall and won't impact scalability of the native allocator, because malloc/free isn't used. I'm worried, because this change would enable a non-transparent handling of large writes/reads on JNI that could silently kill performance due to `malloc/free` + the large copy: that's exactly our use case for OpenWire and AMQP (until we'll get streaming of large messages in) and sadly it would affect CORE too, given that CORE writes/reads in 100KB sized chunks, that's > `BUF_SIZE` (ie 8192 bytes). PLEASE DO NOT MERGE: I would like to discuss with you about it :) I've added https://github.com/apache/activemq-artemis/pull/2844/commits/453c9796a486e6b9289e8cc8aa61e1ebcf41fd9f trying to mitigate the effects of 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
