Re: RFR (M) 8223040: Add a AGCT test

Daniel D. Daugherty Fri, 03 May 2019 08:01:35 -0700

On 5/2/19 8:28 PM, Jean Christophe Beyler wrote:

:)


Sounds good to me and here is the new webrev with that naming scheme:
Webrev: http://cr.openjdk.java.net/~jcbeyler/8223040/webrev.01/


make/test/JtregNativeHotspot.gmk
    No comments.

test/hotspot/jtreg/serviceability/AsyncGetCallTrace/MyPackage/AsyncGetCallTraceTest.java
    L38:       System.loadLibrary("AsyncGetCallTraceTest");
    L40:       System.err.println("Could not load Agct library");
        The name in the error message should match the actual library name.

test/hotspot/jtreg/serviceability/AsyncGetCallTrace/libAsyncGetCallTraceTest.cpp
    L79:   // OnClassPrepare callback to prime the jmethods for ASGCT.
        ASGCT used here, but never spelled out before.
        Also, you've been using AGCT elsewhere...

    L107:   if (jvmti->AddCapabilities(&caps) != JVMTI_ERROR_NONE) {
        Missing an error message:

          fprintf(stderr, "Problem adding the capabilities\n");

    L118:     fprintf(stderr, "Problem adding the capabilities\n");
        typo: s/capabilities/callbacks/

L125: fprintf(stderr, "Problem setting the class loadingevent.\n");

        typo: s/loading/load/

L132: fprintf(stderr, "Problem setting the class loadingevent.\n");

        typo: s/loading/prepare/

    L161: // A copy of the ASGCT data structures.
        I thought I put a copy of the header file into the repo...

    L165: } ASGCT_CallFrame;
        I screwed up when I used "ASGCT_" years ago... Can't fix it now.


Your call on whether to fix the minor issues above. I don't need to
see a new webrev if you do.

Thumbs up.

Dan

Bug: https://bugs.openjdk.java.net/browse/JDK-8223040

Thanks for your help!
Jc

On Thu, May 2, 2019 at 5:16 PM <serguei.spit...@oracle.com<mailto:serguei.spit...@oracle.com>> wrote:




    On 5/2/19 5:13 PM, serguei.spit...@oracle.com
    <mailto:serguei.spit...@oracle.com> wrote:

    God suggestion!


    Above is a typo, I wanted to say "Good suggestion".
    But the typo is funny. :)

    Thanks,
    Serguei


    Thanks,
    Serguei

    On 5/2/19 4:55 PM, Daniel D. Daugherty wrote:

    I would use "AsyncGetCallTrace" for the top level directory name.
    That would make it easier for someone searching the test space...

    Dan


    On 5/2/19 7:03 PM, Jean Christophe Beyler wrote:

    Hi Serguei,

    Thanks for the review, I fixed the bug name but have not yet
    changed the webrev. Does anyone else have an opinion of the
    naming of the tests?

    Thanks all!
    Jc

    On Tue, Apr 30, 2019 at 5:10 PM <serguei.spit...@oracle.com
    <mailto:serguei.spit...@oracle.com>> wrote:

        Hi Jc,

        I'd suggest to change the bug title to be:
           Add a AsyncGetCallTrace test

        I'm not sure about the test names.
        Maybe, it is Okay to keep the AGCT abbreviation.
        But I'd like to hear other opinions.

        Thanks,
        Serguei

        On 4/30/19 3:47 PM, Jean Christophe Beyler wrote:

        Hi all,

        As I start looking at working on the AGCT bugs, I wanted
        to at least start creating a baseline of tests for AGCT.
        This is an attempt to just have a "base" test (and
        infrastructure) that tries to call AGCT and get back some
        sane information.

        Next step will be to add a few more tests that will be
        exposing the limitations of
        https://bugs.openjdk.java.net/browse/JDK-8178287 for example.

        Webrev:
        http://cr.openjdk.java.net/~jcbeyler/8223040/webrev.00/
        <http://cr.openjdk.java.net/%7Ejcbeyler/8223040/webrev.00/>
        Bug: https://bugs.openjdk.java.net/browse/JDK-8223040

        This passed the test on my linux machine (the test is only
        for linux due to the dlsym) and the submit-repo.

        Thanks,
        Jc

--

    Thanks,
    Jc




--

Thanks,
Jc

Re: RFR (M) 8223040: Add a AGCT test

Reply via email to