On Fri, 27 Oct 2023 20:53:29 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:

>> The JEP covers the idea very well, so I'm only covering some implementation 
>> details here:
>> 
>> * regions get a "pin count" (reference count). As long as it is non-zero, we 
>> conservatively never reclaim that region even if there is no reference in 
>> there. JNI code might have references to it.
>> 
>> * the JNI spec only requires us to provide pinning support for typeArrays, 
>> nothing else. This implementation uses this in various ways:
>> 
>>   * when evacuating from a pinned region, we evacuate everything live but 
>> the typeArrays to get more empty regions to clean up later.
>> 
>>   * when formatting dead space within pinned regions we use filler objects. 
>> Pinned regions may be referenced by JNI code only, so we can't overwrite 
>> contents of any dead typeArray either. These dead but referenced typeArrays 
>> luckily have the same header size of our filler objects, so we can use their 
>> headers for our fillers. The problem is that previously there has been that 
>> restriction that filler objects are half a region size at most, so we can 
>> end up with the need for placing a filler object header inside a typeArray. 
>> The code could be clever and handle this situation by splitting the to be 
>> filled area so that this can't happen, but the solution taken here is 
>> allowing filler arrays to cover a whole region. They are not referenced by 
>> Java code anyway, so there is no harm in doing so (i.e. gc code never 
>> touches them anyway).
>> 
>> * G1 currently only ever actually evacuates young pinned regions. Old pinned 
>> regions of any kind are never put into the collection set and automatically 
>> skipped. However assuming that the pinning is of short length, we put them 
>> into the candidates when we can.
>> 
>>   * there is the problem that if an applications pins a region for a long 
>> time g1 will skip evacuating that region over and over. that may lead to 
>> issues with the current policy in marking regions (only exit mixed phase 
>> when there are no marking candidates) and just waste of processing time 
>> (when the candidate stays in the retained candidates)
>> 
>>     The cop-out chosen here is to "age out" the regions from the candidates 
>> and wait until the next marking happens.
>> 
>>     I.e. pinned marking candidates are immediately moved to retained 
>> candidates, and if in total the region has been pinned for 
>> `G1NumCollectionsKeepUnreclaimable` collections it is dropped from the 
>> candidates. Its current value is fairly random.
>> 
>> * G1 pauses got a new tag if there were pinned regions in the collection 
>> set. I.e. in a...
>
> src/hotspot/share/gc/g1/g1EvacFailureRegions.hpp line 36:
> 
>> 34: class HeapRegionClaimer;
>> 35: 
>> 36: // This class records for every region on the heap whether it has to be 
>> retained
> 
> I feel the term "retain" has two diff meanings in this PR:
> 
> 1. retain == pinned or evac-fail
> 2. should_retain_evac_failed_region
> 
> 1 is during scavenging while 2 is after scavenging.

Maybe rename `should_retain_evac_failed_region` to 
`should_keep_retained_region[_in_old]` or something?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16342#discussion_r1376219339

Reply via email to