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.hol...@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.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
