RFR 8169389 : Use a bitmap to control StackTraceElement::toString format and save footprint

Brent Christian brent.christian at oracle.com
Tue Dec 13 20:53:01 UTC 2016


Thank you, Mandy and Daniel.

As a point of interest, automated testing uncovered a couple things I 
had to change before pushing:

http://hg.openjdk.java.net/jdk9/dev/hotspot/rev/accf1676e416

* Updated the CHECK_OFFSET line, so fastdebug builds

http://hg.openjdk.java.net/jdk9/dev/jdk/rev/ddc8f2ae290b

* In the test case, only check for a classloader named 'app' if we are 
using the system classloader.  This is usually the case, but not when 
running regtests using the 'make' target (URLClassLoader).

Thanks,
-Brent
On 12/10/2016 06:51 AM, Daniel Fuchs wrote:
> Hi Brent,
>
> This looks really good now!
>
> best regards,
>
> -- daniel
>
> On 10/12/16 01:16, Brent Christian wrote:
>> On 12/07/2016 04:05 PM, Mandy Chung wrote:
>>>
>>> I suggest to add two utility methods rather than the has method:
>>>    boolean dropClassLoaderName()
>>>    boolean dropModuleVersion()
>>
>> Done.
>>
>>>   430    if (m != null && m.isNamed() &&
>>>   431          (isHashedInJavaBase(m) ||
>>> !m.getDescriptor().version().isPresent())) {
>>>   432        bits |= JDK_NON_UPGRADEABLE_MODULE;
>>>   433    }
>>>
>>> I think this should simply be:
>>>     if (isHashedInJavaBase(m)) {..}
>>>
>>
>> Done.
>>
>>> Can you retain the javadoc of toLoaderModuleClassName, revised if
>>> appropriate, in the computeFormat method?
>>
>> Updated.
>>
>>> line 322-325: what about:
>>>
>>> The toString method may return two different values on two
>>> StackTraceElement instances that are equal for example when
>>> one created via the constructor and one obtained from Throwable
>>> or StackFrame where an implementation may choose to omit some
>>> element in the returned string.
>>
>> That sounds good.
>>
>>> Is @apiNote in equals necessary?  Maybe the one added in toString is
>>> adequate?
>>
>> I'm fine without it - removed.
>>
>>
>> I also fixed test code for a case which only works with the images build.
>>
>> Updated webrev:
>> http://cr.openjdk.java.net/~bchristi/8169389/webrev.04/
>>
>> -Brent
>



More information about the core-libs-dev mailing list