RFR(S) 8242162: convert clhsdb "sysprops" command from javascript to java

Chris Plummer chris.plummer at oracle.com
Thu Apr 9 02:08:05 UTC 2020


Thanks Serguei!

Can I get one more review please?

thanks,

Chris

On 4/7/20 10:54 PM, serguei.spitsyn at oracle.com wrote:
> Chris,
>
> Refactored version looks nice.
> I was thinking to suggest the same in previous review but decided 
> there is not a big profit for small blocks of code.
> But it is obvious now that it is clearly better. :)
>
> Thanks,
> Serguei
>
>
> On 4/7/20 22:15, Chris Plummer wrote:
>> Hello,
>>
>> Please review the following:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8242162
>> http://cr.openjdk.java.net/~cjplummer/8242162/webrev.00
>> http://cr.openjdk.java.net/~cjplummer/8242162/webrev.01
>>
>> Note there are two webrevs. You can skip the first if you want. It's 
>> the initial straight forward implementation. After that I did 
>> refactoring, so it might be easier if you looked at webrev.00 first 
>> without the refactoring, but I will be pushing webrev.01.
>>
>> [Serguei, regarding the refactoring you suggested, I didn't find it 
>> was worth doing for the execution of the commands since there are 4 
>> of them, and they all have subtle differences that makes it hard to 
>> refactor in a productive way. I did refactor comparing the lists and 
>> counting the lists.]
>>
>> Regarding the change in LingeredAppSysProps.java, I ran into an issue 
>> when I added the clhsdb test. I was suddenly seeing an extra 
>> property. It was user.timezone. I did some research and found that 
>> user.timezone is kind of special. If not set on the command line it 
>> will not be created until someone calls TimeZone.getDefault(). What’s 
>> interesting is that when using the Attach API to get the sysprops 
>> (using jinfo or jcmd), the first time you do this you don’t get back 
>> user.timezone, but repeat the command and you do. Somehow the first 
>> time the command is executed it triggers the initialization of 
>> user.timezone, but only after the sysprops result was generated. So 
>> in the test LingeredAppSysProps.main() was printing the properties 
>> without use.name, and so was "jhsdb jinfo" and "jinfo". But the last 
>> "jinfo" command triggered the initialization of user.timezone, so it 
>> turned up in the subsequent run of "clhsdb sysprops", and thus the 
>> count in that list was one higher than the rest. By forcing the 
>> initialization of user.timezone at the start of 
>> LingeredAppSysProps.main(), now user.timezone shows up in all 4 lists.
>>
>> thanks,
>>
>> Chris
>>
>




More information about the serviceability-dev mailing list