vlsi commented on code in PR #6462:
URL: https://github.com/apache/jmeter/pull/6462#discussion_r2144775778


##########
bin/jmeter:
##########
@@ -169,20 +170,26 @@ esac
 # Default to en_EN
 : "${JMETER_LANGUAGE:="-Duser.language=en -Duser.region=EN"}"
 
-# Uncomment this to generate GC verbose file with Java prior to 9
-# VERBOSE_GC="-verbose:gc -Xloggc:gc_jmeter_%p.log -XX:+PrintGCDetails 
-XX:+PrintGCCause -XX:+PrintTenuringDistribution -XX:+PrintHeapAtGC 
-XX:+PrintGCApplicationConcurrentTime -XX:+PrintAdaptiveSizePolicy 
-XX:+PrintGCApplicationStoppedTime -XX:+PrintGCDateStamps"
+# Legacy GC verbose options removed (Java 8/9 support discontinued)
+
+# Optimized GC logging for Java 17 with structured output and performance 
analysis
+# Uncomment to enable comprehensive GC logging with rotation and detailed 
metrics
+# 
VERBOSE_GC="-Xlog:gc,gc+heap,gc+regions,gc+refine,gc+phases:gc_jmeter_%p_%t.log:time,level,tags:filecount=5,filesize=50M"
+
+# Alternative: Minimal GC logging for production (uncomment if needed)
+# VERBOSE_GC="-Xlog:gc:gc_jmeter_%p.log:time"
 
-# Uncomment this to generate GC verbose file with Java 9 and above
-# VERBOSE_GC="-Xlog:gc*,gc+age=trace,gc+heap=debug:file=gc_jmeter_%p.log"
+# Docker support for Java 17+
+# Modern container memory detection is automatic in Java 17+
+# RUN_IN_DOCKER="-XX:+UseContainerSupport"
 
-# Uncomment this if you run JMeter in DOCKER (need Java SE 8u131 or JDK 9)
-# see 
https://blogs.oracle.com/java-platform-group/java-se-support-for-docker-cpu-and-memory-limits
-# RUN_IN_DOCKER="-XX:+UnlockExperimentalVMOptions 
-XX:+UseCGroupMemoryLimitForHeap"
+# Java 17 specific performance optimizations
+# Modern JVM performance flags for better throughput and reduced latency
+JAVA17_PERFORMANCE="-XX:+AlwaysPreTouch -XX:+UseLargePages 
-XX:+OptimizeStringConcat"

Review Comment:
   Those are not "java 17" standard options, and they might be missing in some 
of the implementations.
   For instance, I doubt the options would work with OpenJ9.
   
   HotSpot has `OptimizeStringConcat` by default, so I'm not sure it makes a 
great difference to enable it explicitly.



##########
bin/jmeter:
##########
@@ -169,20 +170,26 @@ esac
 # Default to en_EN
 : "${JMETER_LANGUAGE:="-Duser.language=en -Duser.region=EN"}"
 
-# Uncomment this to generate GC verbose file with Java prior to 9
-# VERBOSE_GC="-verbose:gc -Xloggc:gc_jmeter_%p.log -XX:+PrintGCDetails 
-XX:+PrintGCCause -XX:+PrintTenuringDistribution -XX:+PrintHeapAtGC 
-XX:+PrintGCApplicationConcurrentTime -XX:+PrintAdaptiveSizePolicy 
-XX:+PrintGCApplicationStoppedTime -XX:+PrintGCDateStamps"
+# Legacy GC verbose options removed (Java 8/9 support discontinued)
+
+# Optimized GC logging for Java 17 with structured output and performance 
analysis
+# Uncomment to enable comprehensive GC logging with rotation and detailed 
metrics
+# 
VERBOSE_GC="-Xlog:gc,gc+heap,gc+regions,gc+refine,gc+phases:gc_jmeter_%p_%t.log:time,level,tags:filecount=5,filesize=50M"
+
+# Alternative: Minimal GC logging for production (uncomment if needed)
+# VERBOSE_GC="-Xlog:gc:gc_jmeter_%p.log:time"
 
-# Uncomment this to generate GC verbose file with Java 9 and above
-# VERBOSE_GC="-Xlog:gc*,gc+age=trace,gc+heap=debug:file=gc_jmeter_%p.log"
+# Docker support for Java 17+
+# Modern container memory detection is automatic in Java 17+
+# RUN_IN_DOCKER="-XX:+UseContainerSupport"
 
-# Uncomment this if you run JMeter in DOCKER (need Java SE 8u131 or JDK 9)
-# see 
https://blogs.oracle.com/java-platform-group/java-se-support-for-docker-cpu-and-memory-limits
-# RUN_IN_DOCKER="-XX:+UnlockExperimentalVMOptions 
-XX:+UseCGroupMemoryLimitForHeap"
+# Java 17 specific performance optimizations
+# Modern JVM performance flags for better throughput and reduced latency
+JAVA17_PERFORMANCE="-XX:+AlwaysPreTouch -XX:+UseLargePages 
-XX:+OptimizeStringConcat"
 
-# Finally, some tracing to help in case things go astray:
-# You may want to add those settings:
-# -XX:+ParallelRefProcEnabled -XX:+PerfDisableSharedMem
-: "${GC_ALGO:="-XX:+UseG1GC -XX:MaxGCPauseMillis=100 -XX:G1ReservePercent=20"}"
+# Enhanced G1GC tuning for Java 17
+# Improved G1 performance with better defaults and new features
+: "${GC_ALGO:="-XX:+UseG1GC -XX:MaxGCPauseMillis=100 -XX:G1ReservePercent=20 
-XX:+ParallelRefProcEnabled -XX:+UseStringDeduplication 
-XX:G1HeapRegionSize=16m"}"

Review Comment:
   G1 is Hotspot-specific GC :-/



##########
bin/jmeter.bat:
##########
@@ -78,12 +78,22 @@ if not defined JMETER_LANGUAGE (
 )
 
 rem Minimal version to run JMeter
-set MINIMAL_VERSION=1.8.0
+set MINIMAL_VERSION=17.0.0
 
+rem Java 17 specific performance optimizations
+rem Modern JVM performance flags for better throughput and reduced latency
+set JAVA17_PERFORMANCE=-XX:+AlwaysPreTouch -XX:+UseLargePages 
-XX:+OptimizeStringConcat
 
-rem --add-opens if JAVA 9
-set JAVA9_OPTS=
+rem Optimized GC logging for Java 17 with structured output and performance 
analysis
+rem Uncomment to enable comprehensive GC logging with rotation and detailed 
metrics
+rem set 
VERBOSE_GC=-Xlog:gc,gc+heap,gc+regions,gc+refine,gc+phases:gc_jmeter_%%p_%%t.log:time,level,tags:filecount=5,filesize=50M
 
+rem Alternative: Minimal GC logging for production (uncomment if needed)
+rem set VERBOSE_GC=-Xlog:gc:gc_jmeter_%%p.log:time
+
+
+rem Module access for modern Java versions (required for JMeter components)
+set JAVA_OPTS=--add-opens java.desktop/sun.awt=ALL-UNNAMED --add-opens 
java.desktop/sun.swing=ALL-UNNAMED --add-opens 
java.desktop/javax.swing.text.html=ALL-UNNAMED --add-opens 
java.desktop/java.awt=ALL-UNNAMED --add-opens 
java.desktop/java.awt.font=ALL-UNNAMED 
--add-opens=java.base/java.lang=ALL-UNNAMED 
--add-opens=java.base/java.lang.invoke=ALL-UNNAMED 
--add-opens=java.base/java.lang.reflect=ALL-UNNAMED 
--add-opens=java.base/java.util=ALL-UNNAMED 
--add-opens=java.base/java.text=ALL-UNNAMED 
--add-opens=java.desktop/sun.awt.shell=ALL-UNNAMED

Review Comment:
   I assume you just moved the code, however, it would be nice to reduce the 
list of those options one day



##########
bin/jmeter.bat:
##########
@@ -151,25 +149,21 @@ if not defined HEAP (
     set HEAP=-Xms1g -Xmx1g -XX:MaxMetaspaceSize=256m
 )
 
-rem Uncomment this to generate GC verbose file with Java prior to 9
-rem set VERBOSE_GC=-verbose:gc -Xloggc:gc_jmeter_%%p.log -XX:+PrintGCDetails 
-XX:+PrintGCCause -XX:+PrintTenuringDistribution -XX:+PrintHeapAtGC 
-XX:+PrintGCApplicationConcurrentTime -XX:+PrintGCApplicationStoppedTime 
-XX:+PrintGCDateStamps -XX:+PrintAdaptiveSizePolicy
-
-rem Uncomment this to generate GC verbose file with Java 9 and above
-rem set VERBOSE_GC=-Xlog:gc*,gc+age=trace,gc+heap=debug:file=gc_jmeter_%%p.log
-rem You may want to add those settings
-rem -XX:+ParallelRefProcEnabled -XX:+PerfDisableSharedMem
+rem Legacy GC verbose options removed (Java 8/9 support discontinued)
+rem Enhanced G1GC tuning for Java 17
+rem Improved G1 performance with better defaults and new features
 if not defined GC_ALGO (
-    set GC_ALGO=-XX:+UseG1GC -XX:MaxGCPauseMillis=100 -XX:G1ReservePercent=20
+    set GC_ALGO=-XX:+UseG1GC -XX:MaxGCPauseMillis=100 -XX:G1ReservePercent=20 
-XX:+ParallelRefProcEnabled -XX:+UseStringDeduplication -XX:G1HeapRegionSize=16m

Review Comment:
   `ParallelRefProcEnabled` seems to be enabled by default as well



##########
src/components/src/main/java/org/apache/jmeter/assertions/SizeAssertion.java:
##########
@@ -174,42 +174,41 @@ public void setAllowedSize(long size) {
      * than equal, less than equal.
      *
      */
+    private record ComparisonResult(boolean result, String errorMessage) {}
+
     private String compareSize(long resultSize) {
-        String comparatorErrorMessage;
         long allowedSize = Long.parseLong(getAllowedSize());
-        boolean result;
         int comp = getCompOper();
-        switch (comp) {
-        case EQUAL:
-            result = resultSize == allowedSize;
-            comparatorErrorMessage = 
JMeterUtils.getResString("size_assertion_comparator_error_equal"); //$NON-NLS-1$
-            break;
-        case NOTEQUAL:
-            result = resultSize != allowedSize;
-            comparatorErrorMessage = 
JMeterUtils.getResString("size_assertion_comparator_error_notequal"); 
//$NON-NLS-1$
-            break;
-        case GREATERTHAN:
-            result = resultSize > allowedSize;
-            comparatorErrorMessage = 
JMeterUtils.getResString("size_assertion_comparator_error_greater"); 
//$NON-NLS-1$
-            break;
-        case LESSTHAN:
-            result = resultSize < allowedSize;
-            comparatorErrorMessage = 
JMeterUtils.getResString("size_assertion_comparator_error_less"); //$NON-NLS-1$
-            break;
-        case GREATERTHANEQUAL:
-            result = resultSize >= allowedSize;
-            comparatorErrorMessage = 
JMeterUtils.getResString("size_assertion_comparator_error_greaterequal"); 
//$NON-NLS-1$
-            break;
-        case LESSTHANEQUAL:
-            result = resultSize <= allowedSize;
-            comparatorErrorMessage = 
JMeterUtils.getResString("size_assertion_comparator_error_lessequal"); 
//$NON-NLS-1$
-            break;
-        default:
-            result = false;
-            comparatorErrorMessage = "ERROR - invalid condition";
-            break;
-        }
-        return result ? "" : comparatorErrorMessage;
+        
+        ComparisonResult comparison = switch (comp) {
+            case EQUAL -> new ComparisonResult(
+                resultSize == allowedSize,
+                
JMeterUtils.getResString("size_assertion_comparator_error_equal") //$NON-NLS-1$
+            );
+            case NOTEQUAL -> new ComparisonResult(

Review Comment:
   We might rather refactor `CompOper` to an enum. It works both ways though



##########
src/components/src/main/java/org/apache/jmeter/config/CSVDataSet.java:
##########
@@ -222,20 +222,21 @@ private void initVars(FileServer server, final 
JMeterContext context, String del
     private void setAlias(final JMeterContext context, String alias) {
         String mode = getShareMode();
         int modeInt = CSVDataSetBeanInfo.getShareModeAsInt(mode);
-        switch(modeInt){
-            case CSVDataSetBeanInfo.SHARE_ALL:
-                this.alias = alias;
-                break;
-            case CSVDataSetBeanInfo.SHARE_GROUP:
-                this.alias = alias + "@" + 
System.identityHashCode(context.getThreadGroup());
-                break;
-            case CSVDataSetBeanInfo.SHARE_THREAD:
-                this.alias = alias + "@" + 
System.identityHashCode(context.getThread());
-                break;
-            default:
-                this.alias = alias + "@" + mode; // user-specified key
-                break;
-        }
+        this.alias = switch(modeInt){
+            case CSVDataSetBeanInfo.SHARE_ALL -> alias;
+            case CSVDataSetBeanInfo.SHARE_GROUP -> {
+                // Complex logic for thread group sharing
+                yield alias + "@" + 
System.identityHashCode(context.getThreadGroup());

Review Comment:
   just wondering, is `yield` needed here?



-- 
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-unsubscr...@jmeter.apache.org

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

Reply via email to