RFR: JDK-8162340: Better class stream parsing
Karen Kinnear
karen.kinnear at oracle.com
Tue Jul 26 22:18:09 UTC 2016
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