System.currentTimeMillis check in System class during startup.

[Jon VanAlten]

Hi David and Martin,

Thanks so much for your time (heh, punny) and thoughts, and for taking the ball and running with it.

cheers,

jon


----- "David Holmes" <David.Holmes at oracle.com> wrote:

> 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