Code Review Request for 6977738
Mandy Chung
mandy.chung at oracle.com
Thu Sep 30 17:46:27 UTC 2010
On 09/30/10 07:18, Alan Bateman wrote:
> Mandy Chung wrote:
>> Alan, David,
>>
>> I revise the fix including the following:
>> 1. Use the lazy initialization holder class idiom to initialize the bcp
>> variable to simplify the synchronization.
>>
>> 2. Group the initialization of system properties in a new private method
>> System.initializeSystemProperties so that it can save a copy for
>> internal
>> implementation use to address the deadlock issue and remove the
>> properties that are intended for private internal use from public access
>> before initializing the System.props. This removes the 2 calls to
>> System.setProperties.
>>
>> 3. Reorder sun.net.* in FILES_java.gmk
>>
>> The new webrev is at:
>> http://cr.openjdk.java.net/~mchung/6977738/webrev.01/
> This looks much better (and thanks for doing the various
> drive-by-clean-ups).
>
> I assume BootClassPathHolder should be private, and that the bcp field
> can be final.
>
> Minor nit but but the IllegalStateExceptions thrown by
> VM.getSavedProperty capitalizes the first letter of the message
> whereas VM.saveAndRemoveProperties doesn't.
>
> I wonder if it would be worth seeing if saveAndRemoveProperties could
> grab the value of the java.lang.Integer.IntegerCache.high property.
> That would allow us to be handle it more consistently with these other
> properties that we save during initialization. The cache
> initialization could then use VM.getSavedProperty to get the value.
> Might be worth thinking about but you would need to be careful to
> ensure that Integer.valueOf isn't called as otherwise isn't not going
> to work.
>
Yes, I also thought of that. I was trying to keep the change as
minimal. I agree that I should take this chance to clean up and handle
private system properties consistently. It's good not to leak the
private internal interfaces for public access.
Joe, Sherman,
I change IntegerCache and ZipFile initialization to get the property
value from the private system properties object (not calling
System.getProperty). I also remove the "sun.zip.disableMemoryMapping"
property from the public system properties. Can you take a look?
http://cr.openjdk.java.net/~mchung/6977738/webrev.02
Thanks
Mandy
More information about the core-libs-dev
mailing list