[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=198413&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-198413 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 14/Feb/19 00:23 Start Date: 14/Feb/19 00:23 Worklog Time Spent: 10m Work Description: asfgit commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522 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 Issue Time Tracking --- Worklog Id: (was: 198413) Time Spent: 5h 50m (was: 5h 40m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 5h 50m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=197538&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197538 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 12/Feb/19 12:31 Start Date: 12/Feb/19 12:31 Worklog Time Spent: 10m Work Description: franz1981 commented on issue #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#issuecomment-462741716 This one should be kinda independent from the rest so can be merged without worrying about any conflicts, thanks!!! 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 Issue Time Tracking --- Worklog Id: (was: 197538) Time Spent: 5h 40m (was: 5.5h) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 5h 40m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=197513&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197513 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 12/Feb/19 12:00 Start Date: 12/Feb/19 12:00 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#issuecomment-462732912 Looking promising. @franz1981 with all the changes youve been working on for journals, im not sure what the merge order is. Can this just be merged OR do we need merge another PR first? 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 Issue Time Tracking --- Worklog Id: (was: 197513) Time Spent: 5.5h (was: 5h 20m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 5.5h > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=194977&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194977 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 06/Feb/19 08:25 Start Date: 06/Feb/19 08:25 Worklog Time Spent: 10m Work Description: franz1981 commented on issue #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#issuecomment-460938230 @michaelandrepearce @clebertsuconic I've manged to obtain the CPU flamgegraphs of the time spent on `TimedBuffer::flushBatch` and I see that the changes of this PR save a visible amount of CPU dependent by the accumulated written bytes into `TimedBuffer`. This is the flames of `master`: ![image](https://user-images.githubusercontent.com/13125299/52328387-ba3d1a80-29ef-11e9-81d9-7cda890a832b.png) While with this PR: ![image](https://user-images.githubusercontent.com/13125299/52328364-a396c380-29ef-11e9-91b7-3a626c53621a.png) The violet part represent the CPU cycles spent zeroing the buffer and it's ~20% of the total time spent on `TimedBuffer::flushBuffer`. As a note: I've mentioned that a further (important) improvement on NIO/MAPPED would be to allow `fdatasync` and the callbacks to be called on an external thread (dedicated, like ASYNCIO on blockedPoll) and it's correct: with a very fast disk `fdatasync` is not that costy, but waking up threads while executing `OperationContextImpl::done` is one of the dominant costs (the 2 bars on the left). 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 Issue Time Tracking --- Worklog Id: (was: 194977) Time Spent: 5h 20m (was: 5h 10m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 5h 20m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190416&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190416 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 14:15 Start Date: 26/Jan/19 14:15 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251200460 ## 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: At least make it a nested class. Anonymous classes whilst not impossible to debug, just having it named makes stacks trace, heap dumps etc that little bit easier to quickly read and understand. 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 Issue Time Tracking --- Worklog Id: (was: 190416) Time Spent: 4h 40m (was: 4.5h) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 4h 40m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190420&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190420 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 14:23 Start Date: 26/Jan/19 14:23 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251200661 ## 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: Fair enough. Im won over on this one 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 Issue Time Tracking --- Worklog Id: (was: 190420) Time Spent: 5h 10m (was: 5h) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 5h 10m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190419&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190419 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 14:19 Start Date: 26/Jan/19 14:19 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190419) Time Spent: 5h (was: 4h 50m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 5h > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190418&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190418 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 14:17 Start Date: 26/Jan/19 14:17 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251200460 ## 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: At least make it a nested named class. Anonymous classes whilst not impossible to debug, just having it named makes stacks trace, heap dumps etc that little bit easier to quickly read and understand. 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 Issue Time Tracking --- Worklog Id: (was: 190418) Time Spent: 4h 50m (was: 4h 40m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 4h 50m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190378&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190378 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 10:13 Start Date: 26/Jan/19 10:13 Worklog Time Spent: 10m Work Description: franz1981 commented on issue #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#issuecomment-457819129 @michaelandrepearce > Have you got any stats to share on this, especially on running on standardish spec server? Im assuming that its quite a good improvement based on it removes copies :) , but be good for the record like Not yet: I can just say that on my box `fsync` is such heavy-weight that is difficult to measure the gain of this improvement. If I would use a `tmpfs` folder for the journal instead it would just show that zeroing (and copying) is just the major cost and the speedup is so dramatical that is just unrealistic. @clebertsuconic @michaelandrepearce I will try to get a proper benchmark box to find out this, but nonetheless I'm expecting *at least* a better ability from NIO/MAPPED to maintain the buffer timeout: the next big improvement would be to run `fsync` on a separate "coalescing" thread (similarly to what libaio does on the blocked poll). 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 Issue Time Tracking --- Worklog Id: (was: 190378) Time Spent: 4.5h (was: 4h 20m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 4.5h > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190358&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190358 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 08:46 Start Date: 26/Jan/19 08:46 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190358) Time Spent: 4h 20m (was: 4h 10m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 4h 20m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190357&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190357 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 08:45 Start Date: 26/Jan/19 08:45 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190357) Time Spent: 4h 10m (was: 4h) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 4h 10m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190356&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190356 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 08:41 Start Date: 26/Jan/19 08:41 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190356) Time Spent: 4h (was: 3h 50m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 4h > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190355&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190355 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 08:40 Start Date: 26/Jan/19 08:40 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190355) Time Spent: 3h 50m (was: 3h 40m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 3h 50m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190354&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190354 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 08:38 Start Date: 26/Jan/19 08:38 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190354) Time Spent: 3h 40m (was: 3.5h) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 3h 40m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190292&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190292 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 02:08 Start Date: 26/Jan/19 02:08 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251183788 ## 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) { + delegates.forEach(callback -> { + try { +callback.done(); + } catch (Throwable e) { +ActiveMQJournalLogger.LOGGER.errorCompletingCallback(e); + } + }); + } + + public static void onError(Collection delegates, int errorCode, final String errorMessage) { Review comment: ditto as per other static method, thats takes IOCallback 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 Issue Time Tracking --- Worklog Id: (was: 190292) Time Spent: 3h 10m (was: 3h) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 3h 10m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190291&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190291 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 02:07 Start Date: 26/Jan/19 02:07 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251183735 ## 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: This is not specific to DelegateCallback, should this move to IOCallback, e.g. this would be usable for anything that extends IOCallback 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 Issue Time Tracking --- Worklog Id: (was: 190291) Time Spent: 3h (was: 2h 50m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 3h > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190295&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190295 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 02:15 Start Date: 26/Jan/19 02:15 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251184069 ## 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: Make this its own class, so its not anonymous. 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 Issue Time Tracking --- Worklog Id: (was: 190295) Time Spent: 3.5h (was: 3h 20m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 3.5h > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190293&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190293 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 02:13 Start Date: 26/Jan/19 02:13 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251184000 ## 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: Is it worth extracting this to its own class file. 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 Issue Time Tracking --- Worklog Id: (was: 190293) Time Spent: 3h 20m (was: 3h 10m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 3h 20m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190290&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190290 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 26/Jan/19 02:07 Start Date: 26/Jan/19 02:07 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251183735 ## 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: This is not specific to DelegateCallback, should this move to IOCallback 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 Issue Time Tracking --- Worklog Id: (was: 190290) Time Spent: 2h 50m (was: 2h 40m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 2h 50m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190047&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190047 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 15:03 Start Date: 25/Jan/19 15:03 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190047) Time Spent: 2h 40m (was: 2.5h) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 2h 40m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190033&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190033 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 14:48 Start Date: 25/Jan/19 14:48 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190033) Time Spent: 0.5h (was: 20m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 0.5h > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190045&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190045 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 15:01 Start Date: 25/Jan/19 15:01 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190045) Time Spent: 2h 20m (was: 2h 10m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 2h 20m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190046&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190046 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 15:01 Start Date: 25/Jan/19 15:01 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190046) Time Spent: 2.5h (was: 2h 20m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 2.5h > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190044&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190044 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 15:00 Start Date: 25/Jan/19 15:00 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190044) Time Spent: 2h 10m (was: 2h) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 2h 10m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190036&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190036 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 14:50 Start Date: 25/Jan/19 14:50 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190036) Time Spent: 1h (was: 50m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 1h > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190042&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190042 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 14:55 Start Date: 25/Jan/19 14:55 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190042) Time Spent: 2h (was: 1h 50m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190041&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190041 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 14:54 Start Date: 25/Jan/19 14:54 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190041) Time Spent: 1h 50m (was: 1h 40m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 1h 50m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190039&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190039 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 14:53 Start Date: 25/Jan/19 14:53 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190039) Time Spent: 1.5h (was: 1h 20m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 1.5h > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190038&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190038 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 14:53 Start Date: 25/Jan/19 14:53 Worklog Time Spent: 10m Work Description: clebertsuconic commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251011598 ## 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: I see.. you moved the previous logic to the SequentialFile.. nice one!! What happens to Mapped though, from what I looked it still using a new buffer. is that what you wanted? 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 Issue Time Tracking --- Worklog Id: (was: 190038) Time Spent: 1h 20m (was: 1h 10m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 1h 20m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190040&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190040 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 14:53 Start Date: 25/Jan/19 14:53 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190040) Time Spent: 1h 40m (was: 1.5h) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 1h 40m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190037&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190037 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 14:51 Start Date: 25/Jan/19 14:51 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190037) Time Spent: 1h 10m (was: 1h) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 1h 10m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190035&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190035 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 14:50 Start Date: 25/Jan/19 14:50 Worklog Time Spent: 10m Work Description: clebertsuconic commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251010514 ## 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: Hmmm.. but you moved the copy to LocalBufferObserver::flushBuffer. What's the difference then, you're still making the copy. 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 Issue Time Tracking --- Worklog Id: (was: 190035) Time Spent: 50m (was: 40m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 50m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190034&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190034 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 14:50 Start Date: 25/Jan/19 14:50 Worklog Time Spent: 10m Work Description: franz1981 commented on 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 Issue Time Tracking --- Worklog Id: (was: 190034) Time Spent: 40m (was: 0.5h) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 40m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190026&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190026 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 14:39 Start Date: 25/Jan/19 14:39 Worklog Time Spent: 10m Work Description: clebertsuconic commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#discussion_r251006945 ## 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: I wish it was that simple. that's only viable if your write operation is synchronous. That's probably ok with NIO or Mapped, but it's definitely not ok with libaio. If you allow the inner buffer to be reused like that, TimedBuffer will be making changes to a buffer that's been submitted to the kernel, and you may endup with wrong data. And that kind of race is hard to catch. You can only that do this kind of thing on NIO or Mapped, assuming their buffers will be copied to the file right away. 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 Issue Time Tracking --- Worklog Id: (was: 190026) Time Spent: 20m (was: 10m) > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 20m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2239) Zero-copy NIO/MAPPED TimedBuffer
[ https://issues.apache.org/jira/browse/ARTEMIS-2239?focusedWorklogId=190002&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-190002 ] ASF GitHub Bot logged work on ARTEMIS-2239: --- Author: ASF GitHub Bot Created on: 25/Jan/19 13:41 Start Date: 25/Jan/19 13:41 Worklog Time Spent: 10m Work Description: franz1981 commented on pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522 NIO/MAPPED journal types can use directly the buffer of TimedBuffer to perform file writes, avoiding an expensive copy + zeroing. 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 Issue Time Tracking --- Worklog Id: (was: 190002) Time Spent: 10m Remaining Estimate: 0h > Zero-copy NIO/MAPPED TimedBuffer > > > Key: ARTEMIS-2239 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2239 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.6.4 >Reporter: Francesco Nigro >Assignee: Francesco Nigro >Priority: Major > Time Spent: 10m > Remaining Estimate: 0h > > NIO/MAPPED journal types are currently performing the copy of the buffer used > on TimedBuffer to accumulate writes, but this copy (and the zeroing of the > new buffer) could be avoided, reducing dramatically the CPU usage and > latencies during a burst of writes. > An added benefit of this change is that isn't necessary anymore to stress the > thread pool of the file factory, because there is no need to pool the buffer > used to perform the write. -- This message was sent by Atlassian JIRA (v7.6.3#76005)