System.currentTimeMillis check in System class during startup.

[David Holmes]

Hi Martin,

Martin Buchholz said the following on 07/08/10 15:38:
> On Wed, Jul 7, 2010 at 18:08, David Holmes <David.Holmes at oracle.com> wrote:
>> I think there has been some confusion here. It seems to me that the
>> anti-inlining that is being referred to is javac's "inlining" of
>> compile-time constants - BUT that doesn't apply to reference types other
>> than String, so it seems to me that the current code contortions serve no
>> useful purpose and direct initialization to null could have been used.
> 
> I was actually afraid of what the VM would do, not javac.

I don't see how this impacts VM compilation - even if such compilation 
were to occur this early in the VM initialization process - but we 
should be able to test this easily enough - perhaps with -Xcomp

> I freely admit to a bit of superstition.
> Surely there must have been a time when the
> anti-inlining was effective?

This was very early code from late 1996 so perhaps then the rules for 
compile-time constants were not clear. The comment for this change 
simply states:

"Initialize properties and streams in an explicit method in order to 
avoid doing I/O before the thread system is initialized."

which seems unrelated to the coding changes in question. That said, 
given this is pre-hotspot perhaps early VMs did have an issue compiling 
this code during initialization. Certainly the code seems to be geared 
towards avoiding the inlining of the nullXXXStream methods themselves - 
with the currentTimeMillis() check being a non-trivial equivalent of 
"true" ? Though again I can't see how not-inlining, or inlining, those 
methods, would affect anything.

> Anyways, with two votes for making the code sane,
> I'll go along and simply initialize to null.
> http://cr.openjdk.java.net/~martin/webrevs/openjdk7/TimeTravel/

Looks okay to me, but this:

Summary: Accept negative values of currentTimeMillis as valid

should be more like:

Summary: remove unnecessary check of currentTimeMillis

I've filed 6967533

> If there's a problem with setOut, we'll surely find out before this jdk
> goes into production.

Let us hope so. As the old saying goes "never take down a fence without 
knowing why it was put up in the first place" ;-)

Cheers,
David

> Martin
> 
>> There is a second memory-model issue concerning these fields but basically
>> that requires that no code gets compiled and caches the initial null values
>> of these fields, prior to the setXX methods being called to initialize them
>> correctly. I don't see how the use of the current code would change that one
>> way or another though, so again the current code seems to serve no purpose.
>>
>> Did I miss something?
>>
>> David Holmes