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