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