RFR: JDK-8162340: Better class stream parsing

Karen Kinnear karen.kinnear at oracle.com
Tue Jul 26 17:31:11 UTC 2016


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