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

[Daniil Titov]

Hi Chris,

Please review a new version of the fix[1] that also updates jdk/sun/tools  tests.

Testing: Mach5 tier1-tier7 tests successfully passed.


[1] http://cr.openjdk.java.net/~dtitov/8242009/webrev.03/
[2] ] https://bugs.openjdk.java.net/browse/JDK-8242009

Thank you,
Daniil

On 5/8/20, 12:21 AM, "Chris Plummer" <chris.plummer at oracle.com> wrote:

    Hi Daniil,

    I just noticed you didn't update the tests in jdk/sun/tools/jhsdb. Do 
    you think these should be done also?

    Chris

    On 5/7/20 11:44 PM, Chris Plummer wrote:
    > 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
    >>      >
    >>      >
    >>
    >>
    >>
    >>
    >
    >