RFR 8048268: G1 Code Root Migration performs poorly

[Mikael Gerdin]

Jon,

On 2014-08-30 01:12, Jon Masamitsu wrote:
> Mikael,
>
> When an nmethod is made not_entrant,
> the  G1CodeRootSet::remove() is called
> and if the entries in the table go to 0, the
> table is deleted (right?).   And then a new
> table is reallocated if needed again (right?)
> If yes and yes, why not just  keep the table?

Yes and yes.
I was just trying to keep memory usage down, I guess.
Also, since there is no current policy for going back to a smaller table 
if the number of entries goes down this is a way to shrink the size if 
the amount of code roots pointing into a region changes without the 
region being evacuated.
I'm not sure if it's worth it to keep the code for deallocating the 
table if the size goes to 0, though.

/Mikael

>
> Jon
>
>
> On 8/26/2014 8:42 AM, Mikael Gerdin wrote:
>> Hi,
>>
>> In order to combat the spikes in code root migration times I suggest
>> that we
>> reimplement the code cache remembered set using hash tables instead of
>> the
>> current chunked array variant.
>>
>> While we're at it I suggest that we get rid of the entire migration
>> phase and
>> update the code cache remembered set during the parallel RSet scanning
>> phase.
>> The contains()-check when adding during RSet scanning is designed to
>> be lock-
>> free in order to reduce contention on the HRRS locks.
>> This led me to remove some contains-checks in asserts since they were
>> done
>> during a phase where operations which could not guarantee lock-free
>> reads to
>> succeed were performed.
>>
>> Testing: Kitchensink 14hrs, JPRT, Aurora perf testing of several industry
>> benchmarks and CRM Fuse (where it actually makes a difference since we
>> had
>> 300ms spikes in code root migration times).
>>
>> The table sizes in G1CodeRootSet are completely unscientific but seem
>> to work
>> good enough for now. An even larger table size could possibly be
>> considered
>> for pathological cases where we get thousands of nmethods (as can
>> occur in CRM
>> Fuse) but currently the two sizes seem good enough.
>>
>> This change depends on "JDK-8056084: Refactor Hashtable to allow
>> implementations without rehashing support" since the remembered sets are
>> allocated and deallocated I needed to allow for deallocation of
>> instances of
>> HashtableEntry and deallocation of freelist contents.
>>
>> Webrev: http://cr.openjdk.java.net/~mgerdin/8048268/nm-hashtable/webrev/
>> Buglink: https://bugs.openjdk.java.net/browse/JDK-8048268
>>
>> a note about g1RemSetSummary.cpp
>> the code failed to update _max_code_root_mem_sz, so the code to list
>> the most
>> expensive code root remset was broken.
>>
>> /Mikael
>