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

[Yasumasa Suenaga]

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
>>>
>>
> 
>