Hello Roger,
On 10/09/21 12:32 am, Roger Riggs wrote:
src/java.base/share/classes/java/util/Properties.java line 187:
185: ?
System.getenv("SOURCE_DATE_EPOCH")
186: :
AccessController.doPrivileged((PrivilegedAction<String>)
187: () ->
System.getenv("SOURCE_DATE_EPOCH"));
The format of SOURCE_DATE_EPOCH is fine, but referring to an external document
as part of the OpenJDK spec is undesirable in the long run. Previous comments
gave support to using the environment variable directly but
it very unlike similar cases that use system properties when the behavior of an
API needs to be overridden or modified.
As a system property, specified on the command line, it would be visible when
the program is invoked,
explicitly intended to change behavior and not having a context sensitive
effect. Named perhaps, java.util.Properties.storeDate.
Would this system property have a value that represents what
SOURCE_DATE_EPOCH environment variable would have contained? i.e. it is
the "A UNIX timestamp, defined as the number of seconds, excluding leap
seconds, since 01 Jan 1970 00:00:00 UTC."? So users can potentially do
-Djava.util.Properties.storeDate=${SOURCE_DATE_EPOCH}?
Or would this system property be just some kind flag, which when having
a value of "true" would expect the java.util.Properties code to lookup
the SOURCE_DATE_EPOCH environment variable and use that value from the
environment variable?
I'm guessing it's the former where the value is the number of epoch
seconds, but just wanted to be sure before I do this change.
(BTW, there is no need to special case doPriv calls, they are pretty well
optimized when the security manager is not set and it keeps the code simpler.)
From what I see in the implementation of
AccessController.doPriveleged(), I see that it does a bunch of
uncoditional work (like Reflection.getCallerClass() and then
Reference.reachabilityFence() calls). Do you mean that additional work
is neglible in the absence of a security manager, that this special
casing of the doPriv call can be removed?
Given the low frequency of calls to store(), even caching the parsed date
doesn't save much.
And the cost is the cost of the size and loading an extra class.
Sounds fine. I'll remove the caching part in my subsequent update.
-Jaikiran