RFR: 8242009: Review setting test.java/vm.opts in jcmd/jhsdb and debugger in serviceability tests

[Chris Plummer]

Hi Daniil,

The changes look good.

thanks,

Chris

On 5/4/20 1:05 PM, Daniil Titov wrote:
> Hi Chris,
>
> Please review a new version of webrev [1] that addresses your comments.
>
> For the following 3 tests that showed the increase of the execution time with -Xcomp
> more than 5 minutes this version of the change  strips -Xcomp option when
> forwarding VM  arguments to  j*-tools  :
>
>     -- serviceability/sa/sadebugd/SADebugDTest.java,
>     -- serviceability/sa/sadebugd/DebugdConnectTest.java,
>     -- serviceability/sa/ClhsdbJstackXcompStress.java
>
> The execution time for the rest of the tests when running with -Xcomp was increased
> within 1 and half minute.
>
>
> [1] http://cr.openjdk.java.net/~dtitov/8242009/webrev.02/
> [2] https://bugs.openjdk.java.net/browse/JDK-8242009
>
> Thank you,
>   Daniil
>
>
> On 4/27/20, 12:55 PM, "Chris Plummer" <chris.plummer at oracle.com> wrote:
>
>      Hi Daniil,
>
>      Overall it looks good. A few comments below.
>
>      Can you add a comment to TestSysProps.java indicating the reason for
>      checking if the line starts with "[".
>
>      In JDKToolLauncher you have an extra empty line:
>
>        112      * Any platform specific arguments required for running the
>      tool are
>        113      * automatically added.
>        114      *
>        115      *
>        116      * @param args
>
>      In OutputAnalyzer.java, I wonder if all these matching APIs you updated
>      should by default just include the version output in their filtering.
>
>      As for the added time to execute, I would suggest possibly stripping
>      -Xcomp from the few outliers, and I would mostly focus on how much
>      longer it takes, not how many times longer. For example, increasing from
>      10 seconds to 40 seconds is not a big deal. Increasing from 10 minutes
>      to 20 minutes is.
>
>      SADebugDTest creates 8 tool processes so, that's probably the reason for
>      the big increase, although I'm not sure why it is kind of slow in the
>      first place. It does nothing more on each iteration than launch "jhsdb
>      debugd", which will connect to the debuggee, and then is killed off.
>      Maybe there is something slow with connecting, especial with RMI.
>
>      thanks,
>
>      Chris
>
>      On 4/27/20 12:07 PM, Daniil Titov wrote:
>      > Please review the change [1] that ensures that VM and test options are forwarded to
>      > j*-tools when they are launched from serviceability/sa tests.
>      >
>      > The tests that expect an empty output  were corrected to ignore the product version printed
>      > in the error stream since in some  tiers the tests are run with ' -showversion' VM option (tier3).
>      >
>      > In test serviceability/sa/TestSysProps.java the code that counts the system properties  was  corrected
>      > to ignore the debug output when the test is run with " -Xlog:cds=debug" option (tier4).
>      >
>      > Testing:  Mach5 tests for tier1 - tier7 passed.
>      >
>      > I also run the test with -XComp at Mach5 linux-x64-debug builds before and after the changes
>      > and for  the most of the tests the  overhead is about 2 times although for
>      > serviceability/sa/sadebugd/SADebugDTest.java it spikes up to 5 times.  Probably at least for some tests
>      > it makes to filter out some properties (e.g. -Xcomp) before forwarding them to j*-tools.
>      >
>      > serviceability/sa/sadebugd/SADebugDTest.java, before : 2m 23s ,  after:11m 07s
>      > serviceability/sa/sadebugd/TestJmapCore.java,  before : 42s ,  after:1m 09s
>      > serviceability/sa/TestSysProps.java, before : 36s ,  after: 1m 27s
>      >
>      >
>      > [1] http://cr.openjdk.java.net/~dtitov/8242009/webrev.01
>      > [2] https://bugs.openjdk.java.net/browse/JDK-8242009
>      >
>      > Thank you,
>      > Daniil
>      >
>      >
>
>
>
>