RFR 8190359: Reduce the number of recorded klass dependencies

[harold seigel]

Hi Ioi,

Thanks for looking at this!  Please see comments inline.

On 1/30/2018 2:50 PM, Ioi Lam wrote:
> Hi Harold,
>
> Why is this needed
>
>  336   if (from_cld == to_cld ...
It's not needed. I added it as a quick way to exclude many potential 
dependencies.  I can remove it.
>
> It seems like it's already checked by here
>
>  350     if (from == to || java_lang_ClassLoader::isAncestor(from, to)) {
You are right. It is.  The first 'if' clause also handles the case where 
'from_cld' and 'to_cld' have the same class loader, but differ because 
'from_cld' is anonymous.
>
> It's probably explained by this
>
>  348     assert(from != to || from_cld->is_anonymous(), "sanity check");
If line 336 is removed then I'll remove this assert because it's no 
longer valid.
>
> ... but I think this is a rather tricky area so either (a) more 
> comments might be needed in the source code, or (b) simplify the code 
> so it requires less explanation.
Does removing lines 336 and 348 simplify the code?
>
> Also, I wonder if it's possible to add a new test case for this.
Any suggestions on how to do this?  If logging was added then it would 
be easier to write tests, but I don't think logging is needed here 
because recording dependencies is not something users care about.  
Recording dependencies is an implementation detail.

Thanks, Harold
>
> Thanks
>
> - Ioi
>
>
>
> On 1/30/18 10:38 AM, harold seigel wrote:
>> Hi,
>>
>> Please review this RFR for JDK-11 to reduce the number of class 
>> dependencies recorded by the VM.  The change primarily does this by 
>> not recording dependencies to classes that are loaded by a builtin 
>> class loader and are not anonymous.  These classes never get 
>> unloaded, so no recorded dependency is needed.
>>
>> Additionally, the change simplifies the code that deals with when the 
>> classes have the same class loader and when the dependency is to a 
>> class loaded by a parent loader.
>>
>> Open Webrev: 
>> http://cr.openjdk.java.net/~hseigel/bug_8190359/webrev/index.html
>>
>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8190359
>>
>> The change was tested with JPRT, Mach 5 tier1 - tier5 tests, and 
>> non-colocated tonga tests.  Additionally, print statements were 
>> temporarily added to the code and the output analyzed to check that 
>> dependencies were being correctly recorded or not recorded.
>>
>> Thanks, Harold
>