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

Lois Foltan lois.foltan at oracle.com
Fri Feb 28 13:47:01 UTC 2020


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
>



More information about the hotspot-runtime-dev mailing list