Hi Chris,
Thank you for reviewing this change.
Best regards,
Daniil
On 5/11/20, 10:55 AM, "Chris Plummer" <chris.plum...@oracle.com> wrote:
Hi Daniil,
Looks good.
thanks,
Chris
On 5/11/20 9:43 AM, Daniil Titov wrote:
> 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
> >> >
> >> >
> >>
> >>
> >>
> >>
> >
> >
>
>
>
