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

[Chris Plummer]

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
>>      >
>>      >
>>
>>
>>
>>
>
>