RFR (M) 8213346 Re-implement shared dictionary

[Jiangli Zhou]

Hi Ioi,

I came across the SystemDictionary::clear_invoke_method_table() call in 
MetaspaceShared::preload_and_dump() while looking into dynamic dumping. 
With your shared dictionary change, I think clear_invoke_method_table() 
is no longer needed anymore. Could you please check that?

Thanks,

Jiangli


On 11/9/18 12:22 PM, Jiangli Zhou wrote:
> Hi Ioi,
>
> This looks clean overall! I only have the following comments for 
> systemDictionaryShared.cpp.
>
> - share/classfile/systemDictionaryShared.cpp
>
> _num_constraints * 2
>
> Please use a macro or a static function for the various 
> '_num_constraints * 2', which is less error-prone and easier to maintain.
>
>  287 inline bool my_record_equals_compact_hashtable_entry
>
> How about renaming the above to klass_equals?
>
>  878 ResourceHashtable<
>  879   Symbol*, bool,
>  880   primitive_hash<Symbol*>,
>  881   primitive_equals<Symbol*>,
>  882   6661,                             // prime number
>  883   ResourceObj::C_HEAP> _unregister_class_exists;
>
> Maybe rename _unregister_class_exists to _existing_unregister_classes 
> or _loaded_unregister_class?
>
>  942 void 
> SystemDictionaryShared::set_shared_class_misc_info(InstanceKlass* k, 
> ClassFileStream* cfs) {
>
> Please add an assert to make sure it's only called for classes loaded 
> by the non-builtin loaders.
>
>  978     tty->print_cr("Preload Warning: Skipping %s from unsupported 
> location",
>  979                   k->name()->as_C_string());
>  987     tty->print_cr("Preload Warning: Skipping %s from signed JAR",
>  988                   k->name()->as_C_string());
>  997     tty->print_cr("Skipping JFR event class %s", 
> k->name()->as_C_string());
>
> We should consider using log_warning for above tty->print.
>
> Thanks,
>
> Jiangli
>
>
> On 11/8/18 10:04 PM, Ioi Lam wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8213346
>> http://cr.openjdk.java.net/~iklam/jdk12/8213346-shared-dict-using-compact-hashtable.v01/ 
>>
>>
>> Please review a long overdue cleanup of how the classes are indexed 
>> in the CDS
>> archive.
>>
>> [1] Use CompactHashtable which is more space efficient than Dictionary.
>>
>> [2] Space saving:
>>
>>     + For each shared class, we used to store a fixed-size record in
>>       SharedDictionaryEntry. That information is now stored in a
>>       variable-length structure (see RunTimeSharedClassInfo).
>>
>>     + Verifier constraints are now stored using u4 instead of Symbol*.
>>
>> Total space saving is about 45KB for the default CDS archive.
>>
>> [4] We had scattered code that checked whether a class should be 
>> archived
>>     or not. E.g., checking for code signers, JFR, etc. Now this is 
>> checked inside
>>     a consolidated function SystemDictionaryShared::should_be_excluded()
>>
>> [5] Now we have 2 separate hashtable to look up shared classes: one 
>> for the
>>     built-in classes and the other for unregistered classes. This 
>> simplifies the
>>     lookup logic and is also faster.
>>
>> [6] A lot of old code was deleted. There's a net change of about -250 
>> lines of C
>>     code.
>>
>> TESTING:
>>
>> I am running hs-tiers 1/2/3/4
>>
>> Thanks
>> - Ioi
>