matthiasblaesing commented on code in PR #6596:
URL: https://github.com/apache/netbeans/pull/6596#discussion_r1367845962
##########
enterprise/glassfish.common/src/org/netbeans/modules/glassfish/common/GlassfishInstanceProvider.java:
##########
@@ -71,17 +83,17 @@ public final class GlassfishInstanceProvider implements
ServerInstanceProvider,
public static final String JAKARTAEE10_DEPLOYER_FRAGMENT =
"deployer:gfv700ee10";
public static final String EE6WC_DEPLOYER_FRAGMENT = "deployer:gfv3ee6wc";
// NOI18N
public static final String PRELUDE_DEPLOYER_FRAGMENT = "deployer:gfv3"; //
NOI18N
- private static String EE6_INSTANCES_PATH = "/GlassFishEE6/Instances"; //
NOI18N
- private static String EE7_INSTANCES_PATH = "/GlassFishEE7/Instances"; //
NOI18N
- private static String EE8_INSTANCES_PATH = "/GlassFishEE8/Instances"; //
NOI18N
- private static String JAKARTAEE8_INSTANCES_PATH =
"/GlassFishJakartaEE8/Instances"; // NOI18N
- private static String JAKARTAEE9_INSTANCES_PATH =
"/GlassFishJakartaEE9/Instances"; // NOI18N
- private static String JAKARTAEE91_INSTANCES_PATH =
"/GlassFishJakartaEE91/Instances"; // NOI18N
- private static String JAKARTAEE10_INSTANCES_PATH =
"/GlassFishJakartaEE10/Instances"; // NOI18N
- private static String EE6WC_INSTANCES_PATH = "/GlassFishEE6WC/Instances";
// NOI18N
-
- public static String PRELUDE_DEFAULT_NAME = "GlassFish_v3_Prelude";
//NOI18N
- public static String EE6WC_DEFAULT_NAME = "GlassFish_Server_3.1"; // NOI18N
+ private static final String EE6_INSTANCES_PATH =
"/GlassFishEE6/Instances"; // NOI18N
+ private static final String EE7_INSTANCES_PATH =
"/GlassFishEE7/Instances"; // NOI18N
+ private static final String EE8_INSTANCES_PATH =
"/GlassFishEE8/Instances"; // NOI18N
+ private static final String JAKARTAEE8_INSTANCES_PATH =
"/GlassFishJakartaEE8/Instances"; // NOI18N
+ private static final String JAKARTAEE9_INSTANCES_PATH =
"/GlassFishJakartaEE9/Instances"; // NOI18N
+ private static final String JAKARTAEE91_INSTANCES_PATH =
"/GlassFishJakartaEE91/Instances"; // NOI18N
+ private static final String JAKARTAEE10_INSTANCES_PATH =
"/GlassFishJakartaEE10/Instances"; // NOI18N
+ private static final String EE6WC_INSTANCES_PATH =
"/GlassFishEE6WC/Instances"; // NOI18N
+
+ public static final String PRELUDE_DEFAULT_NAME = "GlassFish_v3_Prelude";
//NOI18N
+ public static final String EE6WC_DEFAULT_NAME = "GlassFish_Server_3.1"; //
NOI18N
Review Comment:
The problem is the "final static". It took me some digging to find the
specificiation basis and not just some random note, but this builds it:
https://docs.oracle.com/javase/specs/jls/se8/html/jls-4.html#jls-4.12.4:
> A constant variable is a final variable of primitive type or type String
that is initialized with a constant expression
([§15.28](https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.28)).
Whether a variable is a constant variable or not may have implications with
respect to class initialization
([§12.4.1](https://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.4.1)),
binary compatibility
([§13.1](https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html#jls-13.1),
[§13.4.9](https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html#jls-13.4.9)),
and definite assignment ([§16 (Definite
Assignment)](https://docs.oracle.com/javase/specs/jls/se8/html/jls-16.html)).
https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html#jls-13.1
(section 3)
> A reference to a field that is a constant variable
([§4.12.4](https://docs.oracle.com/javase/specs/jls/se8/html/jls-4.html#jls-4.12.4))
must be resolved at compile time to the value V denoted by the constant
variable's initializer.
>
> If such a field is static, then no reference to the field should be
present in the code in a binary file, including the class or interface which
declared the field. Such a field must always appear to have been initialized
([§12.4.2](https://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.4.2));
the default initial value for the field (if different than V) must never be
observed.
You can observe this with this project:
[constant_variable.zip](https://github.com/apache/netbeans/files/13062491/constant_variable.zip)
Unzip and run from the base directory:
```
mvn clean process-classes; javap -v -p
target/classes/eu/doppelhelix/dev/constantvariable/UseSite.class
```
You see, that the string is evaluated at compile time. The string is loaded
from the constant table (ldc call) an directly passed to `println` from
`System.out`. The output was created by OpenJDK 17 as shipped with Ubuntu, it
might be different in your case.
```
public static void main(java.lang.String[]);
descriptor: ([Ljava/lang/String;)V
flags: (0x0009) ACC_PUBLIC, ACC_STATIC
Code:
stack=2, locals=1, args_size=1
0: getstatic #7 // Field
java/lang/System.out:Ljava/io/PrintStream;
3: ldc #15 // String Hello World!
5: invokevirtual #17 // Method
java/io/PrintStream.println:(Ljava/lang/String;)V
8: return
LineNumberTable:
line 7: 0
line 8: 8
LocalVariableTable:
Start Length Slot Name Signature
0 9 0 args [Ljava/lang/String;
```
Now open `ConstantProvider.java`, comment out "Variant 1" and choose one of
the other variants, repeat.
This yields a much more complex result:
```
public static void main(java.lang.String[]);
descriptor: ([Ljava/lang/String;)V
flags: (0x0009) ACC_PUBLIC, ACC_STATIC
Code:
stack=2, locals=1, args_size=1
0: getstatic #7 // Field
java/lang/System.out:Ljava/io/PrintStream;
3: getstatic #13 // Field
eu/doppelhelix/dev/constantvariable/ConstantProvider.WORLD:Ljava/lang/String;
6: invokedynamic #19, 0 // InvokeDynamic
#0:makeConcatWithConstants:(Ljava/lang/String;)Ljava/lang/String;
11: invokevirtual #23 // Method
java/io/PrintStream.println:(Ljava/lang/String;)V
14: return
LineNumberTable:
line 7: 0
line 8: 14
LocalVariableTable:
Start Length Slot Name Signature
0 15 0 args [Ljava/lang/String;
}
SourceFile: "UseSite.java"
BootstrapMethods:
0: #43 REF_invokeStatic
java/lang/invoke/StringConcatFactory.makeConcatWithConstants:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/String;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;
Method arguments:
#49 Hello \u0001!
InnerClasses:
public static final #56= #52 of #54; // Lookup=class
java/lang/invoke/MethodHandles$Lookup of class java/lang/invoke/MethodHandles
```
Now the value of `ConstantProvider.WORLD` is loaded first (`get static #13`)
and then passed to the invoke dynamic, that then builds the string at runtime.
TL;DR version: Move initialization into a static initializer block and the
variables can become final, which being inlined.
##########
enterprise/glassfish.common/src/org/netbeans/modules/glassfish/common/GlassfishInstanceProvider.java:
##########
@@ -71,17 +83,17 @@ public final class GlassfishInstanceProvider implements
ServerInstanceProvider,
public static final String JAKARTAEE10_DEPLOYER_FRAGMENT =
"deployer:gfv700ee10";
public static final String EE6WC_DEPLOYER_FRAGMENT = "deployer:gfv3ee6wc";
// NOI18N
public static final String PRELUDE_DEPLOYER_FRAGMENT = "deployer:gfv3"; //
NOI18N
- private static String EE6_INSTANCES_PATH = "/GlassFishEE6/Instances"; //
NOI18N
- private static String EE7_INSTANCES_PATH = "/GlassFishEE7/Instances"; //
NOI18N
- private static String EE8_INSTANCES_PATH = "/GlassFishEE8/Instances"; //
NOI18N
- private static String JAKARTAEE8_INSTANCES_PATH =
"/GlassFishJakartaEE8/Instances"; // NOI18N
- private static String JAKARTAEE9_INSTANCES_PATH =
"/GlassFishJakartaEE9/Instances"; // NOI18N
- private static String JAKARTAEE91_INSTANCES_PATH =
"/GlassFishJakartaEE91/Instances"; // NOI18N
- private static String JAKARTAEE10_INSTANCES_PATH =
"/GlassFishJakartaEE10/Instances"; // NOI18N
- private static String EE6WC_INSTANCES_PATH = "/GlassFishEE6WC/Instances";
// NOI18N
-
- public static String PRELUDE_DEFAULT_NAME = "GlassFish_v3_Prelude";
//NOI18N
- public static String EE6WC_DEFAULT_NAME = "GlassFish_Server_3.1"; // NOI18N
+ private static final String EE6_INSTANCES_PATH =
"/GlassFishEE6/Instances"; // NOI18N
+ private static final String EE7_INSTANCES_PATH =
"/GlassFishEE7/Instances"; // NOI18N
+ private static final String EE8_INSTANCES_PATH =
"/GlassFishEE8/Instances"; // NOI18N
+ private static final String JAKARTAEE8_INSTANCES_PATH =
"/GlassFishJakartaEE8/Instances"; // NOI18N
+ private static final String JAKARTAEE9_INSTANCES_PATH =
"/GlassFishJakartaEE9/Instances"; // NOI18N
+ private static final String JAKARTAEE91_INSTANCES_PATH =
"/GlassFishJakartaEE91/Instances"; // NOI18N
+ private static final String JAKARTAEE10_INSTANCES_PATH =
"/GlassFishJakartaEE10/Instances"; // NOI18N
+ private static final String EE6WC_INSTANCES_PATH =
"/GlassFishEE6WC/Instances"; // NOI18N
+
+ public static final String PRELUDE_DEFAULT_NAME = "GlassFish_v3_Prelude";
//NOI18N
+ public static final String EE6WC_DEFAULT_NAME = "GlassFish_Server_3.1"; //
NOI18N
Review Comment:
The problem is the "final static". It took me some digging to find the
specificiation basis and not just some random note, but this builds it:
https://docs.oracle.com/javase/specs/jls/se8/html/jls-4.html#jls-4.12.4:
> A constant variable is a final variable of primitive type or type String
that is initialized with a constant expression
([§15.28](https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.28)).
Whether a variable is a constant variable or not may have implications with
respect to class initialization
([§12.4.1](https://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.4.1)),
binary compatibility
([§13.1](https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html#jls-13.1),
[§13.4.9](https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html#jls-13.4.9)),
and definite assignment ([§16 (Definite
Assignment)](https://docs.oracle.com/javase/specs/jls/se8/html/jls-16.html)).
https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html#jls-13.1
(section 3)
> A reference to a field that is a constant variable
([§4.12.4](https://docs.oracle.com/javase/specs/jls/se8/html/jls-4.html#jls-4.12.4))
must be resolved at compile time to the value V denoted by the constant
variable's initializer.
>
> If such a field is static, then no reference to the field should be
present in the code in a binary file, including the class or interface which
declared the field. Such a field must always appear to have been initialized
([§12.4.2](https://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.4.2));
the default initial value for the field (if different than V) must never be
observed.
You can observe this with this project:
[constant_variable.zip](https://github.com/apache/netbeans/files/13062491/constant_variable.zip)
Unzip and run from the base directory:
```
mvn clean process-classes; javap -v -p
target/classes/eu/doppelhelix/dev/constantvariable/UseSite.class
```
You see, that the string is evaluated at compile time. The string is loaded
from the constant table (ldc call) an directly passed to `println` from
`System.out`. The output was created by OpenJDK 17 as shipped with Ubuntu, it
might be different in your case.
```
public static void main(java.lang.String[]);
descriptor: ([Ljava/lang/String;)V
flags: (0x0009) ACC_PUBLIC, ACC_STATIC
Code:
stack=2, locals=1, args_size=1
0: getstatic #7 // Field
java/lang/System.out:Ljava/io/PrintStream;
3: ldc #15 // String Hello World!
5: invokevirtual #17 // Method
java/io/PrintStream.println:(Ljava/lang/String;)V
8: return
LineNumberTable:
line 7: 0
line 8: 8
LocalVariableTable:
Start Length Slot Name Signature
0 9 0 args [Ljava/lang/String;
```
Now open `ConstantProvider.java`, comment out "Variant 1" and choose one of
the other variants, repeat.
This yields a much more complex result:
```
public static void main(java.lang.String[]);
descriptor: ([Ljava/lang/String;)V
flags: (0x0009) ACC_PUBLIC, ACC_STATIC
Code:
stack=2, locals=1, args_size=1
0: getstatic #7 // Field
java/lang/System.out:Ljava/io/PrintStream;
3: getstatic #13 // Field
eu/doppelhelix/dev/constantvariable/ConstantProvider.WORLD:Ljava/lang/String;
6: invokedynamic #19, 0 // InvokeDynamic
#0:makeConcatWithConstants:(Ljava/lang/String;)Ljava/lang/String;
11: invokevirtual #23 // Method
java/io/PrintStream.println:(Ljava/lang/String;)V
14: return
LineNumberTable:
line 7: 0
line 8: 14
LocalVariableTable:
Start Length Slot Name Signature
0 15 0 args [Ljava/lang/String;
}
SourceFile: "UseSite.java"
BootstrapMethods:
0: #43 REF_invokeStatic
java/lang/invoke/StringConcatFactory.makeConcatWithConstants:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/String;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;
Method arguments:
#49 Hello \u0001!
InnerClasses:
public static final #56= #52 of #54; // Lookup=class
java/lang/invoke/MethodHandles$Lookup of class java/lang/invoke/MethodHandles
```
Now the value of `ConstantProvider.WORLD` is loaded first (`get static #13`)
and then passed to the invoke dynamic, that then builds the string at runtime.
TL;DR version: Move initialization into a static initializer block and the
variables can become final, which being inlined.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
