RFR 8048268: G1 Code Root Migration performs poorly

[Jon Masamitsu]

On 9/1/2014 12:47 AM, Mikael Gerdin wrote:
> 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.

OK.  That's sufficient reason.

Changes look good.  I have not looked at some of the recent renaming
but don't wait for me on that since Thomas and Bengt has looked at
that.

Reviewed.

Jon

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