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/03/26 08:30:15 UTC
[GitHub] [activemq-artemis] franz1981 opened a new pull request #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
franz1981 opened a new pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521
https://issues.apache.org/jira/browse/ARTEMIS-3211
--
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 #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
franz1981 commented on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808077196
I'm fine with the suggestion of @gemmellr and the comments of @gtully and @michaelandrepearce ; just would like to warn users to set it, given that's a free lunch :)
--
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 #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
franz1981 commented on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808046339
@michaelandrepearce In the comments of https://issues.apache.org/jira/browse/ARTEMIS-3211 I've explained why the change on Inline level -> it's the new default on the JDK, so we've the coverage of the JDK team that bumping up won't produce any regression (they run the old JMS spec too while testing changes like this)
--
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 #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808043968
I would question playing around too much with default jvm settings out the box. Unless we have some stats where these settings improve things for most users. E.g. small brokers on virtualised envs and large brokers on physical
--
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] gtully commented on pull request #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
gtully commented on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808105990
perfect, that section could do with some love :-)
--
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] gtully commented on pull request #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
gtully commented on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808087563
maybe add a little section at:
https://activemq.apache.org/components/artemis/documentation/latest/perf-tuning.html#tuning-the-vm
--
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 #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
franz1981 commented on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808067361
@michaelandrepearce
> Yes but you now introduce an issue what happens if next version of jvm changes maxinline again, yourve basically forced a default which would ignore the jvms recommendation
Not ignore, just leaving on the ground some free optimization: inlining should be already tuned to account for the type of application and with Netty-like application that perform many buffer manipulation, I've seen in other contexts (eg Vert-x) that raising it would just produce better optimized code (see https://github.com/netty/netty/pull/10368#issuecomment-650029428 for an horrible journey trying to improve the situation for default jdk < 14 users due to lack of this setting in many envs).
This is a type of setting that won't change over time so often (last time was >10 years ago!), really, but I understand your point here.
> I would leave it out personally and rather recommend users to latest jvm for performance improvements
Sadly some users cannot move from jdk 8 (yet) for many reasons, I would rather recommend to do it if they can, is really a free lunch here.
@gtully
> Folks always have to ask their own computer when it comes to performance tradeoffs.
AFAIK there's no trade off here, but just a conservative setting on jdk 8, I can come with some numbers if we need, but isn't easy, I suggest to look at the Netty PR I've referenced that better explain how painful is to workaround on critical path code lack of this setting...
> btw: what is the string deduplication bit, I imagine that can have a large impact depending on the use case. There are loads of potentially duplicate strings and there is some pooling, but surely de-dup would need to work in concert with that.
Sadly string dedup work mostly for old gen Strings and tends to increase pause time: that's a fine grain optimization tuning that each user can decide after profiling (given that is a trade-off).
--
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] gemmellr commented on pull request #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808068526
Franz linked to the JDK14 JIRA for the MaxInlineLevel change, but I'd note it was also backported to JDK 11.0.10 via https://bugs.openjdk.java.net/browse/JDK-8256164 already. Given that, and discussion of dropping JDK8 support, I would probably just leave it unset at this point, perhaps document it for JDK8 usage, and tell people to use current JVMs for best perf.
--
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 #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
franz1981 edited a comment on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808046339
@michaelandrepearce In the comments of https://issues.apache.org/jira/browse/ARTEMIS-3211 I've explained why the change on Inline level -> it's the new default on the JDK, so we've the coverage of the JDK team that bumping up won't produce any regression (they run the old JMS spec too while testing changes like this)
Just as a reference from another known broker: https://github.com/apache/kafka/blob/581344673047c126f2e5f2c00c00221d12a9b7d5/bin/kafka-run-class.sh#L269
--
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] gtully commented on pull request #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
gtully commented on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808060261
I agree with leaving the defaults. referencing this as something that can be relevant such that tweaks to this should be considered when doing performance tuning may be sufficient.
Folks always have to ask their own computer when it comes to performance tradeoffs.
btw: what is the string deduplication bit, I imagine that can have a large impact depending on the use case. There are loads of potentially duplicate strings and there is some pooling, but surely de-dup would need to work in concert with that.
--
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 #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
franz1981 edited a comment on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808100219
Good idea @gtully that seems the appropriate place, going to close this and maybe refresh that perf tuning doc to help users on perf suggestions related other parts of the brokers ie thread pools, GC, Netty configs, JVM etc etc wdyt?
--
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 #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
franz1981 commented on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808100219
Good idea @gtully that seems the appropriate place, going to close this and maybe refresh that doc part to help users on perf suggestions related other parts of the brokers ie thread pools, GC, Netty configs, JVM etc etc wdyt?
--
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 closed pull request #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
franz1981 closed pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521
--
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 edited a comment on pull request #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808052681
Yes but you now introduce an issue what happens if next version of jvm changes maxinline again, yourve basically forced a default which would ignore the jvms recommendation. As well yes the jvm teams have done regressions on latest versions but I would question if done on earlier else they would have simply changed the default in their tick tock minor release changes. I would leave it out personally and rather recommend users to latest jvm for performance improvements. And we simply have docs around how to tune, as anyhow most tunings will be setup dependent and for specific usecase
--
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 #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808052681
Yes but you now introduce an issue what happens if next version of jvm changes maxinline again, yourve basically forced a default which would ignore the jvms recommendation. I would leave it out personally and rather recommend users to latest jvm for performance improvements. And we simply have docs around how to tune, as anyhow most tunings will be setup dependent and for specific usecase
--
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 #3521: ARTEMIS-3211 Increase MaxInlineLevel to 15 on JVM configuration and remove String deduplication
Posted by GitBox <gi...@apache.org>.
franz1981 edited a comment on pull request #3521:
URL: https://github.com/apache/activemq-artemis/pull/3521#issuecomment-808067361
@michaelandrepearce
> Yes but you now introduce an issue what happens if next version of jvm changes maxinline again, yourve basically forced a default which would ignore the jvms recommendation
Not ignore, just leaving on the ground some free optimization: inlining should be already tuned to account for the type of application and with Netty-like application that perform many buffer manipulation, I've seen in other contexts (eg Vert-x) that raising it would just produce better optimized code (see https://github.com/netty/netty/pull/10368#issuecomment-650029428 for an horrible journey trying to improve the situation for default jdk < 14 users due to lack of this setting in many envs).
This is a type of setting that won't change over time so often (last time was >10 years ago!), really, but I understand your point here.
> I would leave it out personally and rather recommend users to latest jvm for performance improvements
Sadly some users cannot move from jdk 8 (yet) for many reasons, I would rather recommend to do it if they can, is really a free lunch here.
@gtully
> Folks always have to ask their own computer when it comes to performance tradeoffs.
AFAIK there's no trade off here, but just a conservative setting on jdk 8, I can come with some numbers if we need, but isn't easy, I suggest to look at the Netty PR I've referenced that better explain how painful is to workaround on critical path code lack of this setting...
> btw: what is the string deduplication bit, I imagine that can have a large impact depending on the use case. There are loads of potentially duplicate strings and there is some pooling, but surely de-dup would need to work in concert with that.
Sadly string dedup work mostly for old gen Strings and tends to increase pause time: that's a fine grain optimization tuning that each user can decide after profiling (given that is a trade-off).
--
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