RFR (L) 7133093: Improve system dictionary performance

[Ioi Lam]

Hi Coleen,

Maybe the bug should be renamed to "Use one Dictionary per class loader 
instance"? That way it's more obvious what it is when you look at the 
repo history.

1.

Why is assert_locked_or_safepoint(SystemDictionary_lock) necessary in 
SystemDictionary::find_class (line 1826), but not necessary 
SystemDictionary::find (line 951)? Since you removed NoSafepointVerifier 
nosafepoint in the latter, maybe this means it's safe to remove the 
assert_locked_or_safepoint in the former?

2.

  455 static ClassLoaderData* _current_loader_data = NULL;
  456 static Klass*           _current_class_entry = NULL;
  457
  458 InstanceKlass* ClassLoaderDataGraph::try_get_next_class() {

How about moving the static fields into an iterator object. That way you 
don't need to keep track of the globals

ClassLoaderDataGraphIterator {
    ClassLoaderData* _current_loader_data
    Klass*           _current_class_entry;

    InstanceKlass* try_get_next_class() { ....}
};

3. Double check locking in ClassLoaderData::dictionary() -- someone else 
should look at this :-)

4. We may need a better strategy for deciding the size of each dictionary.

  565 const int  _primelist[10] = {1, 107, 1009};
  571 Dictionary* ClassLoaderData::dictionary() {
  579     if ((dictionary = _dictionary) == NULL) {
  580       int size;
  581       if (this == the_null_class_loader_data() || 
is_system_class_loader_data()) {
  582         size = _primelist[2];
  583       } else if 
(class_loader()->is_a(SystemDictionary::reflect_DelegatingClassLoader_klass())) 
{
  584         size = _primelist[0];
  585       } else {
  586         size = _primelist[1];
  587       }
  588       dictionary = new Dictionary(this, size);

I'll do some investigation on this issue and get back to you.

The rest of the changes look good to me.

Thanks
- Ioi


On 6/23/17 4:42 PM, coleen.phillimore at oracle.com wrote:
> Summary: Implement one dictionary per ClassLoaderData for faster 
> lookup and removal during class unloading
>
> See RFE for more details.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/7133093.01/webrev
> bug link https://bugs.openjdk.java.net/browse/JDK-7133093
>
> Tested with full "nightly" run in rbt, plus locally class loading and 
> unloading tests:
>
> jtreg hotspot/test/runtime/ClassUnload
>
> jtreg hotspot/test/runtime/modules
>
> jtreg hotspot/test/gc/class_unloading
>
> make test-hotspot-closed-tonga FILTER=quick TEST_JOBS=4 
> TEST=vm.parallel_class_loading
>
> csh ~/testing/run_jck9  (vm/lang/java_lang)
>
> runThese -jck - uses class loader isolation to run each jck test and 
> unloads tests when done (at -gc:5 intervals)
>
>
> Thanks,
> Coleen
>
>