Request Review: JDK-6479237 (cl) Add support for classloader names

[Mandy Chung]

> On Nov 2, 2016, at 6:10 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
> 
> 
> 
> On 11/2/16 8:53 PM, Mandy Chung wrote:
>>> On Nov 2, 2016, at 5:48 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>>> 
>>> Yes, the names of the library methods don't match the JVM names anymore.  I think changing the names are unnecessary and makes it not obvious the mapping.
>> Is there such convention?  I wasn’t aware of that.  In addition, it would be impractical to synchronize the native methods to match JVM function names.
> 
> Yes, there's a convention of having very similar names.  It's not 100% but having completely different names here is really not helpful for maintaining this code.   This is from looking at the names in src/java.base/share/native/libjava.  Some of the names have additional namespace-like strings like "Class" but basically they're close to the same.  I think it's a good convention to have.  In this case, I think the names toStackTraceElement and getStackTraceElements are pretty well descriptive and don't see the reason to change them.
> 


This change was motivated due to two different places filling StackTraceElement (Throwable and StackFrameInfo).  So I added the factory methods to allocate and fill in StackTraceElement object(s) in StackTraceElement and removed the package-private no-arg constructor added by JDK-8150778 [1]. Any change to create such factory methods will be in one single place.

I like toStackTraceElement while getStackTraceElements does not quite represent what it does.  “fillStackTraceFromThrowable” is a more appropriate name.  

The original method Throwable::getStackTraceElements fills in the StackTraceElement array with this Throwable.  I think that method can be renamed to “fillStackTraceFromThrowable” which is clearer.

I suggest to rename the JVM function:
   JVM_GetStackTraceElements => JVM_FillStackTraceElementsFromThrowable
   JVM_ToStackTraceElement => JVM_FillStackTraceElement

Are you okay with this?

> I'll read the thread.  We've had a runtime offsite all week and haven't been able to read this thread, and thought you'd checked in what I'd reviewed before.
> 

There is no change in the hotspot part.

Mandy
[1] https://bugs.openjdk.java.net/browse/JDK-8150778