RFR: JDK-8162340: Better class stream parsing

Karen Kinnear karen.kinnear at oracle.com
Wed Jul 27 11:58:34 UTC 2016


Many thanks.

Karen
> On Jul 26, 2016, at 10:36 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> On 27/07/2016 8:18 AM, Karen Kinnear wrote:
>> I updated the web rev after all - with Coleen’s suggested changes and shortened new comment lines.
>> 
>> http://cr.openjdk.java.net/~acorn/8162340.hs.3/webrev/
>> 
>> I will push once I hear from David.
> 
> Looks good to me. Thanks for clearing up my confusion over Platform vs. System/Application loader. :)
> 
> Thanks,
> David
> 
>> thanks all,
>> Karen
>> 
>>> On Jul 26, 2016, at 1:50 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>>> 
>>> 
>>> Thanks!
>>> Coleen
>>> 
>>> On 7/26/16 1:31 PM, Karen Kinnear wrote:
>>>> Coleen,
>>>> 
>>>> Thank you for the reminder on this review -
>>>> 
>>>>> On Jul 21, 2016, at 6:45 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>>>>> 
>>>>> 
>>>>> http://cr.openjdk.java.net/~acorn/8162340.hs/webrev/src/share/vm/classfile/systemDictionary.cpp.udiff.html
>>>>> 
>>>>> +// Handles unsafe_DefineAnonymousClass and redefineclasses
>>>>> +// redefinedclasses do not add to the class hierarchy
>>>>> 
>>>>> Can you change /redefineclasses/RedefineClasses/?  We pattern match on this.
>>>> Fixed.
>>>>> Can you remove the additional blank line at @@ -1418,10 +1393,11 @@
>>>> Fixed the two additional blank lines - thank you for catching.
>>>>> http://cr.openjdk.java.net/~acorn/8162340.hs/webrev/src/share/vm/oops/instanceKlass.cpp.udiff.html
>>>>> 
>>>>> Since check_prohibited_package() is called from instanceKlass, and doesn't use any instance variables of either class, it should be a local static function defined in instanceKlass.cpp.
>>>> Done. Made it private static for now.
>>>>> http://cr.openjdk.java.net/~acorn/8162340.hs/webrev/src/share/vm/oops/symbol.hpp.udiff.html
>>>>> 
>>>>> Awesome!  I'm glad you found this comment to remove.
>>>> thanks to Frederic.
>>>>> Nobody's going to miss parsed_name.  This is a nice change!
>>>> I hoped you would like that :-)
>>>> 
>>>> thanks for the review,
>>>> Karen
>>>>> Thanks,
>>>>> Coleen
>>>>> 
>>>>> On 7/21/16 1:23 PM, Karen Kinnear wrote:
>>>>>> Please review:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8162340
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~acorn/8162340.hs/webrev/
>>>>>> http://cr.openjdk.java.net/~acorn/8162340.jdk.test/webrev
>>>>>> 
>>>>>> When moving the check from resolve_from_stream to set_package, I removed
>>>>>> the no longer need parsed_class (!) which I should have cleaned up when
>>>>>> tightening the placeholder table cleanup years ago.
>>>>>> 
>>>>>> Tests run:
>>>>>> 1. updated VMAnonymousClass test
>>>>>> 2. jcks
>>>>>> 3. rbt hs-nightly-runtime linux-x64 in progress
>>>>>> 
>>>>>> thanks,
>>>>>> Karen
>>>>>> 
>>>>>> 
>>> 
>> 



More information about the hotspot-runtime-dev mailing list