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

Chris Plummer chris.plummer at oracle.com
Thu Apr 9 02:59:06 UTC 2020


Thanks Yasumasa!

Chris

On 4/8/20 7:37 PM, Yasumasa Suenaga wrote:
> Looks good!
>
> Yasumasa
>
> On 2020/04/09 11:08, Chris Plummer wrote:
>> 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