RFR 8190359: Reduce the number of recorded klass dependencies

[Ioi Lam]

On 1/30/18 12:30 PM, harold seigel wrote:
> 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?

Hi Harold，

Thanks for the explanation. I think removing these lines will make the 
code much easier to understand.

>>
>> 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.
>
Ideally we should have a negative test case that tries to violates 
loader constraints, but is prevented to do so even when the constraints 
aren't recorded in this case. However, I've no idea how to write that :-(

Thanks
- Ioi


> 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
>>
>