RFR: 8274191: Improve g1 evacuation failure injector performance

[Kim Barrett]

On Thu, 23 Sep 2021 12:13:18 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

> Hi all,
> 
>   can I have reviews for this change that improves evacuation failure injector performance to be usable for pinned region performance work?
> 
> The suspected reason I think why performance is so bad (image in the CR), is that the unsynchronized reads and writes to `G1YoungGCEvacFailureInjector::_evacuation_failure_object_count` cause massive performance issue if done millions of times.
> 
> This change makes that counter thread-local, fixing that issue completely.
> 
> I initially had a prototype using a `THREAD_LOCAL`, but when testing I had the feeling that this is/was slightly slower and I thought that adding a variable to every thread where most do not use them seems to be a waste.
> 
> Testing: local testing, GHA
> 
> Thanks,
>   Thomas

Good find.  Looks good.  Just a couple nits.

src/hotspot/share/gc/g1/g1ParScanThreadState.cpp line 90:

> 88: #ifndef PRODUCT
> 89:     _evac_failure_inject_counter(0),
> 90: #endif

This might be less cluttering as `NOT_PRODUCT(_evac_failure_inject_counter(0) COMMA)`.  Your call...

src/hotspot/share/gc/g1/g1YoungGCEvacFailureInjector.inline.hpp line 40:

> 38:   }
> 39:   // Injecting evacuation failures is in effect for current GC
> 40:   // Access to _evacuation_failure_alot_count is not atomic;

Comment about non-atomicity seems like it should not be needed anymore.

-------------

Marked as reviewed by kbarrett (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5650