RFR (M) 8213346 Re-implement shared dictionary

[Jiangli Zhou]

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