[ping] RFR(L): 8221470: Print methods in exception messages in java-like Syntax.

David Holmes david.holmes at oracle.com
Fri Apr 5 00:23:15 UTC 2019 

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