RFR 8166848: Performance bug: SystemDictionary - optimization

[Ioi Lam]

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
>>>
>>
>