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

John Rose john.r.rose at oracle.com
Fri Feb 28 06:11:56 UTC 2020


On Feb 27, 2020, at 4: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.
> 
> 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.

I wanted to put the CL/PD argument pair at the end of the parameter
list, but the optional boolean interfered with it.  If we move the signature
parameter away from the boolean (to the front) we should do the same
for both constructors, right?

> 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.

Good catch; that’s a better name.  Thanks.

> 
> This change looks good to me apart from these minor requests.
> 
> 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