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

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Wed Apr 8 05:54:04 UTC 2020


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