[ping] RFR(L): 8221470: Print methods in exception messages in java-like Syntax.
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Fri Apr 5 05:41:13 UTC 2019
Hi David,
Thanks for pushing the change, and adding Coleen!
Best regards,
Goetz.
> -----Original Message-----
> From: David Holmes <david.holmes at oracle.com>
> Sent: Friday, April 5, 2019 2:23 AM
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; 'hotspot-runtime-
> dev at openjdk.java.net' <hotspot-runtime-dev at openjdk.java.net>
> Subject: Re: [ping] RFR(L): 8221470: Print methods in exception messages in
> java-like Syntax.
>
> Hi Goetz,
>
> On 4/04/2019 10:19 pm, Lindenmaier, Goetz wrote:
> > Hi David,
> >
> > I ran it through jdk-submit, no failures.
> > I now added reviewer information to the patch.
> > I updated the webrev in-place; nothing changed except
> > the reviewer information.
>
> You missed Coleen so I added her.
>
> > You said we need to sync on the push. Do you just
> > want to sponsor the change to make sure it works out?
> > Or do you want to announce when I should push?
> >
> > Feel free to just push it in case you want to...
>
> Changes pushed :)
>
> Thanks,
> David
>
> > Best regards,
> > Goetz.
> >
> >> -----Original Message-----
> >> From: David Holmes <david.holmes at oracle.com>
> >> Sent: Donnerstag, 4. April 2019 01:53
> >> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; 'hotspot-
> runtime-
> >> dev at openjdk.java.net' <hotspot-runtime-dev at openjdk.java.net>
> >> Subject: Re: [ping] RFR(L): 8221470: Print methods in exception messages
> in
> >> java-like Syntax.
> >>
> >> Looks good.
> >>
> >> I'm re-running through our test system.
> >>
> >> Thanks,
> >> David
> >>
> >> On 4/04/2019 1:18 am, Lindenmaier, Goetz wrote:
> >>> Hi,
> >>>
> >>> here a new webrev:
> >>> http://cr.openjdk.java.net/~goetz/wr19/8221470-exMsg-signature/03/
> >>>
> >>> I have removed the test with the bad class name.
> >>> I named the variables name_str/array_str now.
> >>>
> >>> I'll push it to jdk-submit for further testing.
> >>>
> >>> Best regards,
> >>> Goetz.
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: David Holmes <david.holmes at oracle.com>
> >>>> Sent: Mittwoch, 3. April 2019 00:42
> >>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; 'hotspot-
> runtime-
> >>>> dev at openjdk.java.net' <hotspot-runtime-dev at openjdk.java.net>
> >>>> Subject: Re: [ping] RFR(L): 8221470: Print methods in exception
> messages in
> >>>> java-like Syntax.
> >>>>
> >>>> Two follow ups ...
> >>>>
> >>>> On 2/04/2019 11:26 pm, Lindenmaier, Goetz wrote:
> >>>>> Hi David,
> >>>>>
> >>>>>> Overall this looks good to me - a few minor nits/comments below.
> >>>>> thanks!
> >>>>>
> >>>>>> I've applied the patch and am running it through our internal build
> and
> >>>>>> test system (tiers 1-3 initially).
> >>>>>>
> >>>>>> I have a suspicion there will be other tests that need to be updated -
> >>>>>> possibly even JCK tests. Discovering those a-priori will be difficult
> >>>>>> (simply running all the tests would take an extremely long time). Will
> >>>>>> have a discussion about how best to handle those internally.
> >>>>>
> >>>>> I ran most JCK test without problem. They usually don't check
> messages.
> >>>>> I ran all hotspot, jdk, langtools, nashorn and jaxp test (except
> >>>>> for headful tests).
> >>>>
> >>>> Thanks for the additional testing info. I duplicated some of that but
> >>>> found no issues, other than a couple of closed tests.
> >>>>
> >>>>>> src/hotspot/share/oops/method.cpp
> >>>>>> Please put a blank line after each new method.
> >>>>> Fixed.
> >>>>>
> >>>>>> src/hotspot/share/oops/symbol.cpp
> >>>>>>
> >>>>>> + os->print(".");
> >>>>>> + } else {
> >>>>>> + os->print("%c", start[i]);
> >>>>>>
> >>>>>> Please use os->put(char c) for individual characters.
> >>>>> Fixed.
> >>>>>
> >>>>>> The "start" name would seem better as "buf" to me.
> >>>>> Hmm, buf to me is a local chunk of memory used temporarily.
> >>>>> What about array_sig, class_sig?
> >>>>
> >>>> Not really "sigs".
> >>>>
> >>>> str? Else just leave it.
> >>>>
> >>>> Thanks,
> >>>> David
> >>>> -----
> >>>>
> >>>>>> + } else if (start[i] == 'L') {
> >>>>>> + print_class(os, start+i+1, len-i-2);
> >>>>>> Can you insert a comment that help explains the -2:
> >>>>> Done.
> >>>>>
> >>>>>> + for(SignatureStream ss(this); !ss.is_done(); ss.next()) {
> >>>>>> space after for (2 occurrences)
> >>>>> Fixed.
> >>>>>
> >>>>>>
> >>>>
> >>
> test/hotspot/jtreg/runtime/exceptionMsgs/methodPrinting/TestPrintingMe
> th
> >>>>>> ods.java
> >>>>>>
> >>>>>> Not sure the special characters can be used directly in the sources.
> Can
> >>>>>> they not be put in as unicode escapes at all places?
> >>>>> I'll try what Ioi proposed. I'll post a new webrev including that.
> >>>>>
> >>>>> Best regards,
> >>>>> Goetz.
> >>>>>
> >>>>>
> >>>>>>
> >>>>>> ---
> >>>>>>
> >>>>>> Thanks,
> >>>>>> David
> >>>>>> -------
> >>>>>>
> >>>>>>
> >>>>>> On 1/04/2019 12:32 pm, David Holmes wrote:
> >>>>>>> Hi Goetz,
> >>>>>>>
> >>>>>>> I'm looking at this ...
> >>>>>>>
> >>>>>>> On 29/03/2019 8:26 pm, Lindenmaier, Goetz wrote:
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> Any interest in this change?
> >>>>>>>
> >>>>>>> I'm personally of two minds here because these VM generated
> >> exceptions
> >>>>>>> are not only delivered to Java source code. I'd like to know how
> other
> >>>>>>> language developers using the JVM runtime would view this.
> >>>>>>>
> >>>>>>> That aside if you're going to make a change like this then I think the
> >>>>>>> full signature string has to be quoted in some way to delineate it
> >>>>>>> within the larger message.
> >>>>>>>
> >>>>>>>> Should I split it to adapt the exceptions separately one-by-one to
> >>>>>>>> make the change smaller and simplify the review?
> >>>>>>>
> >>>>>>> I don't think that is necessary.
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> David
> >>>>>>> -----
> >>>>>>>
> >>>>>>>> I would propose to start out with AbstractMethodError only.
> >>>>>>>>
> >>>>>>>> Best regards,
> >>>>>>>> Goetz.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> From: Lindenmaier, Goetz
> >>>>>>>> Sent: Tuesday, March 26, 2019 1:06 PM
> >>>>>>>> To: hotspot-runtime-dev at openjdk.java.net
> >>>>>>>> Subject: RFR(L): 8221470: Print methods in exception messages in
> >>>>>>>> java-like Syntax.
> >>>>>>>>
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> A row of exceptions are thrown from the hotspot runtime.
> >>>>>>>> They print methods with their JNI signatures. To increase
> >>>>>>>> readability and resemblance to source code, this change proposes
> >>>>>>>> to print them in a Java-like syntax.
> >>>>>>>>
> >>>>>>>> Some examples:
> >>>>>>>> current method printouts:
> >>>>>>>>
> >>>>>>>> test.TeMe3_B.ma()V
> >>>>>>>> test.TeMe3_B.ma(IZ[[BF)[[D
> >>>>>>>> test.TeMe3_B.ma([[[Ljava/lang/Object;)[[Ltest/TeMe3_B;
> >>>>>>>>
> >>>>>>>> improved format:
> >>>>>>>>
> >>>>>>>> void test.TeMe3_B.ma()
> >>>>>>>> double[][] test.TeMe3_B.ma(int, boolean, byte[][], float)
> >>>>>>>> test.TeMe3_B[][] test.TeMe3_B.ma(java.lang.Object[][][])
> >>>>>>>>
> >>>>>>>> So far, Method::name_and_sig_as_C_string() is used to print
> >>>>>>>> these messages.
> >>>>>>>>
> >>>>>>>> This change implements function Method::external_name() that
> prints
> >>>>>>>> the better
> >>>>>>>> format.
> >>>>>>>> external_name() is chosen according to Klass::external_name().
> >>>>>>>>
> >>>>>>>> Printing the better format requires parsing the signature
> >>>>>>>> Symbol. This is implemented in
> >>>>>>>> void
> Symbol::print_as_signature_external_return_type(outputStream
> >>>> *os);
> >>>>>>>> void
> Symbol::print_as_signature_external_parameters(outputStream
> >>>> *os);
> >>>>>>>> These method names are chosen according to
> >>>>>>>> Symbol::as_class_external_name().
> >>>>>>>>
> >>>>>>>> See this partial webrev for the new functions:
> >>>>>>>> http://cr.openjdk.java.net/~goetz/wr19/8221470-exMsg-
> signature/01-
> >>>>>> new_methods/
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Also, I changed a lot of exception messages to use the new
> format.
> >>>>>>>> This required to adapt a row of tests. I added a test to check
> >>>>>>>> the signature printing does not regress. For all these changes, see
> >>>>>>>> the full webrev:
> >>>>>>>> http://cr.openjdk.java.net/~goetz/wr19/8221470-exMsg-
> >> signature/01/
> >>>>>>>>
> >>>>>>>> I hope I detected all places where method signatures are printed
> to
> >>>>>>>> exception messages.
> >>>>>>>>
> >>>>>>>> Best regards,
> >>>>>>>> Goetz.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
More information about the hotspot-runtime-dev
mailing list