RFR 8166848: Performance bug: SystemDictionary - optimization

[serguei.spitsyn at oracle.com]

+1

Thanks,
Serguei


On 5/17/17 16:53, Ioi Lam wrote:
> Hi Coleen,
>
> This looks much cleaner. Thanks for doing it.
>
> - Ioi
>
> On 5/17/17 2:02 PM, coleen.phillimore at oracle.com wrote:
>>
>> Ioi, Thanks for pushing me to remove this duplication.  How's this?   
>> I reran with runThese and -XX:+VerifyBeforeGC, and built product.
>>
>> http://cr.openjdk.java.net/~coleenp/8166848.03/webrev
>>
>> thanks,
>> Coleen
>>
>> On 5/17/17 12:43 PM, Ioi Lam wrote:
>>> I know many people are not fans of macros or templates, but the 
>>> hashtable iteration code is repeated in many places.
>>>
>>> For example, the verification code is the same across these two tables:
>>>
>>>  352 void PackageEntryTable::verify() {
>>>  353   int element_count = 0;
>>>  354   int max_bucket_count = 0;
>>>  355   for (int index = 0; index < table_size(); index++) {
>>>  356     int bucket_count = 0;
>>>  357     for (PackageEntry* probe = bucket(index);
>>>  358                               probe != NULL;
>>>  359                               probe = probe->next()) {
>>>  360       probe->verify();
>>>  361       element_count++;
>>>  362       bucket_count++;
>>>  363     }
>>>  364     max_bucket_count = MAX2(max_bucket_count, bucket_count);
>>>  365   }
>>>  366   guarantee(number_of_entries() == element_count,
>>>  367             "Verify of Package Entry Table failed");
>>>  368   DEBUG_ONLY(verify_lookup_length(max_bucket_count, "Package 
>>> Entry Table"));
>>>  369 }
>>>
>>>  509 void ModuleEntryTable::verify() {
>>>  510   int element_count = 0;
>>>  511   int max_bucket_count = 0;
>>>  512   for (int i = 0; i < table_size(); i++) {
>>>  513     int bucket_count = 0;
>>>  514     for (ModuleEntry* probe = bucket(i);
>>>  515                               probe != NULL;
>>>  516                               probe = probe->next()) {
>>>  517       probe->verify();
>>>  518       element_count++;
>>>  519       bucket_count++;
>>>  520     }
>>>  521     max_bucket_count = MAX2(max_bucket_count, bucket_count);
>>>  522   }
>>>  523   guarantee(number_of_entries() == element_count,
>>>  524             "Verify of Module Entry Table failed");
>>>  525   DEBUG_ONLY(verify_lookup_length(max_bucket_count, "Module 
>>> Entry Table"));
>>>  526 }
>>>
>>>
>>> and void Dictionary::verify() only has a bit more code inside the 
>>> inner loop. How about using a template here. The following is 
>>> probably not the best way to do it (but it compiles :-)
>>>
>>> #include <stdio.h>
>>>
>>> class Hashtable {
>>> public:
>>>   template <class Table, class Entry> void do_verify() {
>>>     for (int i=0; i<10; i++) { // pseudo code for the table iteration
>>>       void *probe = NULL;      // pseudo code for getting an entry
>>>       Table::verify_entry((Entry*)probe);
>>>     }
>>>   }
>>> };
>>>
>>> class MyHashentry {/*...*/};
>>>
>>> class MyHashtable : public Hashtable {
>>> public:
>>>   void verify() {
>>>     do_verify<MyHashtable, MyHashentry>();
>>>   }
>>>   static void verify_entry(MyHashentry *probe) {
>>>     // ...
>>>   }
>>> };
>>>
>>>
>>> Thanks
>>> - Ioi
>>>
>>>
>>> On 5/15/17 5:30 AM, coleen.phillimore at oracle.com wrote:
>>>>
>>>>> Summary: Check instead that a bucket isn't 10x the average
>>>>>
>>>>> See bug and linked bugs for more details.
>>>>>
>>>>> Tested with RBT nightly tests (tier2-5).
>>>>>
>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8166848.01/webrev
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8166848
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>
>>>
>>
>