[16] RFR(S) 8246546 Simplify SystemDictionary::is_shared_class_visible

[Calvin Cheung]

Hi Ioi,

In the new implementation of 
SystemDictionary::is_shared_class_visible_impl, it seems to be missing 
the following check which was in the old implementation:

1309         if (mod_entry != NULL && mod_entry->is_patched()) {
1310           return false;
1311         }

Maybe the check could be included in line 1377; check if the shared path 
indexes are the same and !mod_entry->is_patched?

Couple of minor nits:

filemap.cpp

Blank line #380 was accidentally deleted?

systemDictionary.hpp

632   static bool is_shared_class_visible_impl(Symbol* class_name, 
InstanceKlass* ik,

If you keep the second parameter in a separate line as before, the above 
change is unnecessary.

thanks,

Calvin


On 6/16/20 3:53 PM, Ioi Lam wrote:
> https://bugs.openjdk.java.net/browse/JDK-8246546
> http://cr.openjdk.java.net/~iklam/jdk16/8246546_simplify_is_shared_class_visible.v01/ 
>
>
> The current implementation of 
> SystemDictionary::is_shared_class_visible has grown more and more 
> complex over time. Now it has checks for many special cases and the 
> code is hard to understand.
>
> I have simplified the logic to:
>
> Between dump time and run time, if a class:
>
>   - has the same class loader
>   - belongs to the same module
>   - is loaded from the same location
>
> ... then this class is visible at run time.
>
> The new check should be more robust and faster.
>
> For validation, I kept the old implementation and assert that the new 
> code produces the exact same result. I will remove 
> SystemDictionary::is_shared_class_visible_impl_old() and 
> SystemDictionaryShared::is_shared_class_visible_for_classloader() when 
> I do the actual push.
>
> Testing -- all CDS tests passed locally. Running mach5 tiers 1-4 now.
>
> Thanks
> - Ioi