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
