RFR (M) JDK-8237766: Enhance signature API to include ResolvingSignatureStream

[Lois Foltan]

On 2/28/2020 9:56 AM, Frederic Parain wrote:
> Changes look good.
> I concur with Harold, adding blank lines would make signature.cpp easier to read.

Thank you for the review Fred.  Will add the blank lines before pushing.
Lois

>
> Fred
>
>
>> On Feb 28, 2020, at 09:27, Harold Seigel <harold.seigel at oracle.com> wrote:
>>
>> Hi Lois,
>>
>> This looks good!  One very minor nit, could you put blank lines between the different ResolvingSignatureStream constructors in signature.cpp, before committing?  It would make it a little more readable.
>>
>> Thanks, Harold
>>
>> On 2/28/2020 8:47 AM, Lois Foltan wrote:
>>> On 2/27/2020 7:37 PM, coleen.phillimore at oracle.com wrote:
>>>> http://cr.openjdk.java.net/~lfoltan/bug_jdk8237766.0/webrev/src/hotspot/share/oops/method.cpp.frames.html
>>>>
>>>> 1683 // These two methods are static since a GC may move the Method
>>>>
>>>> Not in this change but can you remove this obsolete comment.
>>> Done.
>>>
>>>> http://cr.openjdk.java.net/~lfoltan/bug_jdk8237766.0/webrev/src/hotspot/share/runtime/signature.cpp.frames.html
>>>>
>>>> Can you make line 422 shorter?  And I think the signature should be the first parameter since that's the main parameter.
>>> Done. Changed both ResolvingSignatureStream constructors that have a signature parameter.  It is now the first parameter.
>>>
>>>> 452 Klass* ResolvingSignatureStream::as_loaded_klass(TRAPS) {
>>>>
>>>>
>>>> I think as_loaded_klass as a name doesn't really say that you're not going to load the class.  Can you rename klass_if_loaded()? There's a klass_at_if_loaded which does the same sort of thing in the constant pool.
>>> Done.
>>>
>>>> This change looks good to me apart from these minor requests.
>>> Thank you for the review!  Updated webrev at: http://cr.openjdk.java.net/~lfoltan/bug_jdk8237766.1/webrev/
>>> Lois
>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 2/25/20 11:41 AM, Lois Foltan wrote:
>>>>> Please review the following enhancement to introduce a ResolvingSignatureStream class into the signature API support. This consolidates numerous places in the code that manually walk through the differing parts of a signature and request to resolve or find the respective underlying types. ResolvingSignatureStream now provides this capability in order to eliminate the code duplication and redundancy.
>>>>>
>>>>> open webrev at: http://cr.openjdk.java.net/~lfoltan/bug_jdk8237766.0/webrev/
>>>>> bug link: https://bugs.openjdk.java.net/browse/JDK-8237766
>>>>> contributed-by: Lois Foltan, John Rose
>>>>>
>>>>> Testing: hs-tier1-3 (all platforms), hs-tier4-6 (linux only)
>>>>>
>>>>> Thanks,
>>>>> Lois