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.plum...@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.plum...@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 >> > >> > >> >> >> >> > >