RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v5]

[Kelvin Nilsen]

On Mon, 5 Jun 2023 12:46:58 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove three asserts making comparisons between atomic volatile variables
>>   
>>   Though changes to the volatile variables are individually protected by
>>   Atomic load and store operations, these asserts were not assuring
>>   atomic access to multiple volatile variables, each of which could be
>>   modified independently of the others.  The asserts were therefore not
>>   trustworthy, as has been confirmed by more extensive testing.
>
> src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp line 660:
> 
>> 658: void ShenandoahGeneration::increase_used(size_t bytes) {
>> 659:   Atomic::add(&_used, bytes);
>> 660: }
> 
> Note that C++ subexpression evaluation order is undefined.  Here is an example of what can go wrong with the removed assertion:
> 
> ThisThread:  fetches _affiliated_region_count * begins to multiply with region_size
> OtherThread: increases _used and increases _affiliated_region_count appropriately (for a large allocation)
> ThisThread: fetches _used and observes assert violation

We have seen this race manifest in actual testing, which is what has motivated us to remove the assertions.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14185#discussion_r1218045035