You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by GitBox <gi...@apache.org> on 2022/08/02 10:54:03 UTC

[GitHub] [tomcat] nobeh opened a new pull request, #534: Issue 66195 observe mr jar system property

nobeh opened a new pull request, #534:
URL: https://github.com/apache/tomcat/pull/534

   Observe if system property for MR JAR is available
       
       - Check if the JVM system property for MR-JAR is set
       - If set, parse the value as a boolean
       - If set, and when `#isMultiRelease()` is invoked, return the configured
         value
       
   Refs 66195


-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] [tomcat] markt-asf closed pull request #534: Issue 66195 observe mr jar system property

Posted by GitBox <gi...@apache.org>.
markt-asf closed pull request #534: Issue 66195 observe mr jar system property
URL: https://github.com/apache/tomcat/pull/534


-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] [tomcat] nobeh commented on pull request #534: Issue 66195 observe mr jar system property

Posted by GitBox <gi...@apache.org>.
nobeh commented on PR #534:
URL: https://github.com/apache/tomcat/pull/534#issuecomment-1203780743

   > already implements the relevant logic, so we don't need to do anything extra with this property
   
   Yes, that's correct. However, Tomcat uses a "MethodHandle" invocation to call that method. When a system is under load, through profiling, we've observed that invoking the method handle to evaluate to `false` could actually be an expensive operation. This is one of the main motivations for this change. 
   
   > but may also be force
   
   Yes. Thanks for the catch. I will update 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.

To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] [tomcat] nobeh commented on pull request #534: Issue 66195 observe mr jar system property

Posted by GitBox <gi...@apache.org>.
nobeh commented on PR #534:
URL: https://github.com/apache/tomcat/pull/534#issuecomment-1202330120

   https://bz.apache.org/bugzilla/show_bug.cgi?id=66195


-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] [tomcat] nobeh commented on pull request #534: Issue 66195 observe mr jar system property

Posted by GitBox <gi...@apache.org>.
nobeh commented on PR #534:
URL: https://github.com/apache/tomcat/pull/534#issuecomment-1203795589

   > to evaluate to `false` could actually be an expensive operation. This is one of the main motivations for this change.
   
   And the root cause for that is because of a `synchronized` block:
   
   https://github.com/openjdk/jdk/blob/jdk-11%2B28/src/java.base/share/classes/java/util/jar/JarFile.java#L999
   
   


-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] [tomcat] aooohan commented on pull request #534: Issue 66195 observe mr jar system property

Posted by GitBox <gi...@apache.org>.
aooohan commented on PR #534:
URL: https://github.com/apache/tomcat/pull/534#issuecomment-1202708385

   Hi @nobeh ,
   
   The `JarFile#isMultiRelease` method itself already implements the relevant logic, so we don't need to do anything extra with this property. In addition, we know from the documentation that the value of the `jdk.util.jar.enableMultiRelease` property is not just of the boolean type, but may also be `force`. So your implementation is also faulty.
   
   


-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] [tomcat] markt-asf commented on pull request #534: Issue 66195 observe mr jar system property

Posted by GitBox <gi...@apache.org>.
markt-asf commented on PR #534:
URL: https://github.com/apache/tomcat/pull/534#issuecomment-1214871231

   The sync block won't be reached:
   https://github.com/openjdk/jdk/blob/jdk-11%2B28/src/java.base/share/classes/java/util/jar/JarFile.java#L382
   
   The case hasn't been made that this change is necessary nor has any evidence been provided of a genuine performance improvement.


-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org