Hi Alan,
On 09/27/10 08:33, Alan Bateman wrote:
I don't know if you are still looking for a reviewer for this one. I
agree with David's comment that it is rather complicated.
Thanks for reviewing it. Yes, I still need a reviewer for this fix.
I wonder if we better to add VM.saveSystemProperties rather than
having VM.booted do it as a side effect. The saveSystemProperties
method could throw IllegalStateException if booted, to catch any
mis-use after VM initialization. It could also filter out and remove
properties that shouldn't be visible to applications, like
sun.nio.MaxDirectMemorySize (on that property, you could change the
maxDirectMemory method so that it doesn't need to use setProperties
and could get you back the cost of saving the properties).
I agree that it's better to add a method to save the system properties
during initialization and make it explicit. I'll update the fix and
send out the new webrev.
Also, on this, VM.getSystemProperty probably needs a better name,
maybe getSavedSystemProperty, to make it clear that it's not the same
as System.getProperty. On that, do we have any cases where it will be
invoked after initialization has completed? I'm just wondering if it
needs a privileged block as these have been removed from the callers.
Properties.getProperty doesn't have permission check and thus there is
no need for a privileged block for this method. Will think about the
method name getSavedSystemProperty will work, or maybe
getSystemPropertyWithNoLock?
In passing, I see that make/java/java/FILES_java.gmk has sun.net.www
classes listed in the middle of the sun.misc list. It might be good to
re-order these while you are there.
Not wishing to add to your load, but I see XMLUtils.save is using raw
types and maybe it could be updated while you are there (if you have
time of course).
Typo in DownloadManager at L667 -> "How" instead of "Now".
I can clean up the above you mention.
Mandy
