You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/01/05 13:55:02 UTC
[GitHub] [activemq-artemis] franz1981 opened a new pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
franz1981 opened a new pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397
https://issues.apache.org/jira/browse/ARTEMIS-3050
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#issuecomment-772633185
lets postpone this PR.. and think about the WeakREference on PagedReference
perhaps this will be ok, as the messages will be sent through the LInkedList on Queue in certain occasions. I guess if we make the WeakReference on PagedReference a strong reference, it willl probably be ok.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [activemq-artemis] franz1981 edited a comment on pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
franz1981 edited a comment on pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#issuecomment-913535518
@michaelandrepearce I was waiting the comment of @clebertsuconic
```
postponing after the next release... as discussed.
```
So don't know yet what are the replacement/proposal to replace it yet
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] franz1981 commented on pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
franz1981 commented on pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#issuecomment-766614126
This one should be safe to be merged
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [activemq-artemis] franz1981 commented on pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
franz1981 commented on pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#issuecomment-913535518
@michaelandrepearce I was waiting the comment of @clebertsuconic
```
postponing after the next release... as discussed.
```
So don't know yet what are the replacement/proposal to this yet
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#issuecomment-1036534232
I'm converting this to a draft and removing the `DO-NOT-MERGE-YET` label. Using the draft functionality is a bit more elegant. That said, it would be great to either get this merged or just close it. What do you think @clebertsuconic, @franz1981?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#discussion_r555588661
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java
##########
@@ -30,10 +30,9 @@
import org.apache.activemq.artemis.core.server.ServerConsumer;
import org.apache.activemq.artemis.core.server.impl.AckReason;
import org.apache.activemq.artemis.core.transaction.Transaction;
-import org.apache.activemq.artemis.utils.collections.LinkedListImpl;
import org.jboss.logging.Logger;
-public class PagedReferenceImpl extends LinkedListImpl.Node<PagedReferenceImpl> implements PagedReference, Runnable {
Review comment:
<= He had long hair back then and care free:P
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#discussion_r555588661
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java
##########
@@ -30,10 +30,9 @@
import org.apache.activemq.artemis.core.server.ServerConsumer;
import org.apache.activemq.artemis.core.server.impl.AckReason;
import org.apache.activemq.artemis.core.transaction.Transaction;
-import org.apache.activemq.artemis.utils.collections.LinkedListImpl;
import org.jboss.logging.Logger;
-public class PagedReferenceImpl extends LinkedListImpl.Node<PagedReferenceImpl> implements PagedReference, Runnable {
Review comment:
<= He had hair back then and care free:P
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#issuecomment-913532991
@clebertsuconic / @franz1981 whats occuring with this, wanting to start clearing down old / stagnant PR's a bit of spring cleaning so to say - so we focus on stuff thats actively working on / relevant .
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#issuecomment-773588200
postponing after the next release... as discussed.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#discussion_r555475824
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java
##########
@@ -30,10 +30,9 @@
import org.apache.activemq.artemis.core.server.ServerConsumer;
import org.apache.activemq.artemis.core.server.impl.AckReason;
import org.apache.activemq.artemis.core.transaction.Transaction;
-import org.apache.activemq.artemis.utils.collections.LinkedListImpl;
import org.jboss.logging.Logger;
-public class PagedReferenceImpl extends LinkedListImpl.Node<PagedReferenceImpl> implements PagedReference, Runnable {
Review comment:
This was an optimization you performed some time ago, to speed up things on the delivering... Queue.references...
Are you sure about this change? Did you validate perf on 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:
users@infra.apache.org
[GitHub] [activemq-artemis] franz1981 commented on pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
franz1981 commented on pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#issuecomment-766614126
This one should be safe to be merged
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#discussion_r555475824
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java
##########
@@ -30,10 +30,9 @@
import org.apache.activemq.artemis.core.server.ServerConsumer;
import org.apache.activemq.artemis.core.server.impl.AckReason;
import org.apache.activemq.artemis.core.transaction.Transaction;
-import org.apache.activemq.artemis.utils.collections.LinkedListImpl;
import org.jboss.logging.Logger;
-public class PagedReferenceImpl extends LinkedListImpl.Node<PagedReferenceImpl> implements PagedReference, Runnable {
Review comment:
This was an optimization you (@franz1981 ) performed some time ago, to speed up things on the delivering... Queue.references...
Are you sure about this change? Did you validate perf on 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:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#issuecomment-773588200
postponing after the next release... as discussed.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #3397: ARTEMIS-3050 Reduce PagedReferenceImpl memory footprint
Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #3397:
URL: https://github.com/apache/activemq-artemis/pull/3397#discussion_r555562814
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java
##########
@@ -30,10 +30,9 @@
import org.apache.activemq.artemis.core.server.ServerConsumer;
import org.apache.activemq.artemis.core.server.impl.AckReason;
import org.apache.activemq.artemis.core.transaction.Transaction;
-import org.apache.activemq.artemis.utils.collections.LinkedListImpl;
import org.jboss.logging.Logger;
-public class PagedReferenceImpl extends LinkedListImpl.Node<PagedReferenceImpl> implements PagedReference, Runnable {
Review comment:
Yes but i was young :)
I didn't know that pages references are never placed on the queue, hence they don't need to extends node
----------------------------------------------------------------
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:
users@infra.apache.org