Hi Martin,
Martin Buchholz said the following on 07/08/10 15:38:
On Wed, Jul 7, 2010 at 18:08, David Holmes <david.hol...@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
