Hi Eirik,
Yes, its a long standing and inconsistent area of deferred maintenance.
Most of the individual cases are not used enough to warrant making the
(in many cases) incompatible change and breaking some
application/library somewhere.
Interesting, you did not mention Boolean.getBoolean(String name) whose
behavior isn't the most useful.
In particular, it returns true only if the value is "true" (case
insensitive).
From a usability point of view, it should be true if the Property is
present and either "true" or empty.
It should be possible to enable using just '-Dname'.
A related problem is the naming and semantics of the property, there a
number of places where to enable a function you have to set a
"disableXXX" property to false. That makes comprehension harder.
Common utility methods might be a small code cleanup but aligning all
uses might have a very small payback for the possible incompatibilties.
Regards, Roger
On 12/4/24 9:47 AM, Eirik Bjørsnøs wrote:
Hi,
The OpenJDK includes many boolean flags in the form of system
properties. These toggle different behavior such as debug logging,
verification, caching, compatibility and conditional features.
A common interpretation is to evaluate a property as true if it is set
and either blank or equal to "true" (ignoring case). This is a useful
interpretation when a feature should usually be disabled, but you want
users to enable it by setting a flag:
Let's call this the "ifEnabled" interpretation:
-Dflag=true => true
-Dflag=TRUE => true
-Dflag => true
-Dflag=false => false
-Dflag=FALSE => false
-Dflag=abc => false
-Dother => false
MacOSXFileSystem:
final String name = PROPERTY_NORMALIZE_FILE_PATHS;
String value = System.getProperty(name);
NORMALIZE_FILE_PATHS = (value != null)
&& ("".equals(value) || Boolean.parseBoolean(value));
The same logic is implemented in a number of different ways, see for
example:
IPAddressUtil:
var value = System.getProperty(DELAY_URL_PARSING_SP, "false");
DELAY_URL_PARSING_SP_VALUE = value.isEmpty()
|| Boolean.parseBoolean(value);
It can also be used to conditionally disable a feature:
ZipFile:
boolean result;
String value =
System.getProperty("jdk.util.zip.disableZip64ExtraFieldValidation");
if (value == null) {
result = false;
} else {
result = value.isEmpty() || value.equalsIgnoreCase("true");
}
return result;
However, sometimes the logic is inverted (what we really want below is
USE_FAST_PATH = !flagSet):
SystemModuleFinders:
String value =
System.getProperty("jdk.system.module.finder.disableFastPath");
if (value == null) {
USE_FAST_PATH = true;
} else {
USE_FAST_PATH = !value.isEmpty() && !Boolean.parseBoolean(value);
}
Another variant of interpretation flips the meaning of null and empty
values. Under this interpretation, a flag evaluates to true when the
flag is not set (value is null) and to false when the flag is set but
empty. Presumably, this is useful when you want a feature to be
enabled by default, but you need a way to disable it by setting the
flag to "false"
Let's call this the "unlessDisabled" interpretation:
-Dflag=true => true
-Dflag=TRUE => true
-Dflag => false
-Dflag=false => false
-Dflag=FALSE => false
-Dflag=abc => false
-DnotFlag => true
Switching the meaning of null seems useful, as it allows defining a
different default value when the flag is not set.
Switching the meaning of empty seems more questionable. Why should the
following evaluate to false?
-Djdk.preserveScopedValueCache
Likewise, why should the following evaluate to false given that the
default if not set is true?
-Djdk.preserveScopedValueCache=abc
I'm wondering if such use cases would have been better served by a
"not set to false" interpretation:
-Dflag=true => true
-Dflag=TRUE => true
-Dflag => true
-Dflag=false => false
-Dflag=FALSE => false
-Dflag=abc => true
-DnotFlag => true
Some examples of this logic:
Continuation.java:
String value = System.getProperty("jdk.preserveScopedValueCache");
PRESERVE_SCOPED_VALUE_CACHE = (value == null) ||
Boolean.parseBoolean(value);
HttpClient.java:
String keepAlive = props.getProperty("http.keepAlive");
if (keepAlive != null) {
keepAliveProp = Boolean.parseBoolean(keepAlive);
} else {
keepAliveProp = true;
}
Complicating the above is the fact that not all "true" or "false"
comparisons ignores case:
InetAddress.java:
PREFER_IPV4_STACK_VALUE =
System.getProperty("java.net.preferIPv4Stack");
..
if ("true".equals(PREFER_IPV4_STACK_VALUE) && ipv4Available) {
return LookupPolicy.of(IPV4);
}
The sum of all this "interpretation-of-flags" logic was a bit messy
and inconsistent before the JEP-486 cleanups. After the
SecurityManager cleanups, it's becoming increasingly evident that
there is a good amount of accidental complexity in this area.
The analysis required just to prepare this email felt surprisingly
difficult. One would think programmers can reason about simple boolean
arithmetic, but after looking at this aspect of the code base for a
while, I quickly felt the need for a banana break :-)
I guess there is also an underlying usability question here: Would
users prefer using the "ifEnabled" interpretation to configure a
"featureDisabled" flag, or would it be better to use an
"unlessDisabled" interpretation on a "featureEnabled" flag. Looking at
the OpenJDK code base, it's obvious that developers are not in
agreement about this question, leading to inconsistent treatment of
flags. (Personally, I much prefer "enabled=false" over "disabled=true")
Perhaps we should introduce some "Flags" utility with functions for a
small set of standard interpretations? Then new code could mostly use
that, and existing code could gradually move over to match standard
interpretations after carefully reviewing behavioral impact?
Any ideas, feedback, questions?
Thanks,
Eirik.
