RFR: JDK-8162340: Better class stream parsing

Coleen Phillimore coleen.phillimore at oracle.com
Tue Jul 26 22:34:44 UTC 2016


Looks good!
Coleen

On 7/26/16 6:18 PM, 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.
>
> 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