RFR(S): 8166931: Do not include classes which are unusable during run time in the classlist file

[Calvin Cheung]

Hi Lois,

Thanks for your review.

On 10/11/16, 4:38 AM, Lois Foltan wrote:
>
> On 10/10/2016 11:59 PM, Calvin Cheung wrote:
>>
>> Please review this small fix for not including classes in the 
>> classlist file which are unusable during run time.
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8166931
>>
>> webrev: http://cr.openjdk.java.net/~ccheung/8166931/webrev.00/
>
> Hi Calvin,
>
> src/share/vm/classfile/classFileParser.cpp
> - line #5781, I find the if statement logic to be somewhat confusing.  
> This check seems to be only for classes defined to the boot and 
> platform class loader.  I am assuming it does not apply to the 
> application class loader because there is no way to differentiate a 
> class defined to the application class loader from being on the 
> --patch-module list and the -classpath?  Is that why the if statement 
> logic does not include the application class loader?
Yes. We do want to include the classes defined to the app class loader.
>   Maybe it is enough to improve the comment to something like:
>
>   // For the boot and platform class loaders, check if the class is 
> not found in the java runtime image
>   // or the boot loader's appended entries.  This indicates that the 
> class must be located on the --patch-module list and
>   // is not useable during run time, so should be skipped.
I've modified it a little. How about the following?
// For the boot and platform class loaders, check if the class is not 
found in the java runtime image.
// Additional check for the boot class loader is if the class is not 
found in the boot loader’s appended
// entries. This indicates that the class is not useable during run 
time, such as the ones found in the
// —patch-module entries, so it should not be included in the classlist 
file.

>
> Then please indent the start of line #5782 by one space to show that 
> the check for the platform class loader is part of that first || 
> expression.
I'll fix it.
>
> test/runtime/modules/PatchModule/PatchModuleClassList.java
> - good test!
Let me know if you want to see another webrev.

thanks,
Calvin
>
> Thanks,
> Lois
>
>>
>> Testing:
>>     JPRT with -testset hotspot
>>     jtreg tests under hotspot/runtime on all supported platforms (in 
>> progress)
>>
>> thanks,
>> Calvin
>