RFR (L) JDK-8230199: consolidate signature parsing code in HotSpot sources

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Mon Jan 27 21:36:27 UTC 2020


Lois,  So many nice cleanups in this patch!

http://cr.openjdk.java.net/~lfoltan/bug_jdk8230199.0/webrev/src/hotspot/cpu/sparc/sharedRuntime_sparc.cpp.frames.html

1910 ss.skip_array_prefix(1); // skip one '['
1911 if (ss.is_primitive())
1912 in_elem_bt[i] = ss.type();
1913 // else what is in_elem_bt[i]?

This code is for the not-well kept secret JavaCritical, where the native 
function can only take an array of primitives (checked elsewhere that I 
can't find right now).  So I believe this code can just assert 
ss.is_primitive() for all these cases.  Only code on solaris sparc uses 
this feature, so you can run tests on sparc to verify.

http://cr.openjdk.java.net/~lfoltan/bug_jdk8230199.0/webrev/src/hotspot/share/classfile/systemDictionary.cpp.frames.html

2207 if (!Signature::is_array(class_name)) {


Did you try adding SignatureStream before this, to avoid the Symbol 
refcounting madness?

http://cr.openjdk.java.net/~lfoltan/bug_jdk8230199.0/webrev/src/hotspot/share/jvmci/jvmciCompilerToVM.hpp.frames.html

hpp files shouldn't include an inline.hpp file.  Can this include 
oop.hpp instead?

http://cr.openjdk.java.net/~lfoltan/bug_jdk8230199.0/webrev/src/hotspot/share/oops/symbol.cpp.frames.html

Should SignatureStream arguments be const references?  Or should they be 
written to?

This looks really good!
Coleen


On 1/24/20 4:41 PM, Lois Foltan wrote:
> Please review the following enhancement to consolidate signature 
> parsing code in Hotspot sources.  This change removes duplicate blocks 
> of code that parse field or method signatures, provides a new 
> Signature class to support basic signature queries on Symbol operands 
> and enhances the SignatureStream class to parse field signatures in 
> addition to methods.
>
> open webrev 
> at:http://cr.openjdk.java.net/~lfoltan/bug_jdk8230199.0/webrev/ 
> <http://cr.openjdk.java.net/~lfoltan/bug_jdk8230199.0/webrev/>
> bug link: https://bugs.openjdk.java.net/browse/JDK-8230199
> contributed-by: Lois Foltan, John Rose
>
> Testing: hs-tier1-8, jdk-tier1-2
>
> Thanks,
> Lois



More information about the hotspot-runtime-dev mailing list