Review request: 8015422: Large performance hit when the StringTable is walked twice in Parallel Scavenge
stefan.karlsson at oracle.com
Mon May 27 18:43:54 UTC 2013
This patch changes some Runtime code, so I've CC:ed hotspot-runtime-dev.
On 2013-05-27 16:25, Per Lidén wrote:
> On 2013-05-27 13:50, Stefan Karlsson wrote:
>> 8015422: Large performance hit when the StringTable is walked twice in
>> Parallel Scavenge
>> Summary: Combine the calls to StringTable::unlink and
>> StringTable::oops_do in Parallel Scavenge.
>> The patch is built on top of this clean-up patch:
>> The fix has been verified to give ~10% lower young GC times on the CRM
>> Sales Opty, the benchmark where this issue was found.
> Nice! Looks good,
> just a minor naming convention question. Would it make sense to call
> the new unlink() function oops_do() instead? I think of oops_do() as
> "adjust or remove references"
I prefer to think of oops_do() as functions that should be agnostic to
the OopClosure passed in. The closure could mark, adjust, verify, print,
> and unlink() is to me more "nothing needs to be adjusted, just remove
> any dead references".
What about StringTable::clean(BoolObjectClosure* is_alive, OopClosure* f) ?
That mimics the parameter naming of:
void JNIHandleBlock::weak_oops_do(BoolObjectClosure* is_alive,
void JvmtiTagMap::do_weak_oops(BoolObjectClosure* is_alive, OopClosure* f)
Both of these functions applies 'f' if the is_alive answers true for the
given oop and "cleans" the entry/oop if the answer is false, just like
> Or maybe unlink/oops_do has a different meaning to other people?
> A more general comment, or more like a suggestion for a future
> improvement. My guess is that the overwhelming majority of the
> interned String objects are old, meaning that most of the table
> traversal we do every young GC is a waste of time. If the table can
> provide something like young_oops_do(), which only traverses the young
> references, we can probably cut another ~10% of the young GC times in
> the FA CRM case.
This patch is a quick fix for some low-hanging fruits.
thanks for the review,
> /Per (not a reviewer)
More information about the hotspot-gc-dev