RFR(xs): 8134749: SoftReferences declared dead too early

[Jon Masamitsu]

On 03/10/2016 06:55 AM, Per Liden wrote:
> Hi Jon,
>
> On 2015-09-01 12:57, Per Liden wrote:
>> Hi Jon,
>>
>> On 2015-08-31 20:20, Jon Masamitsu wrote:
>>> Per,
>>>
>>> Change looks good.
>>>
>>> Could you please run some tests with your fix and
>>> -XX:+UseDynamicNumberOfGCThreads and
>>> -XX:+UseParallelGC threads?  UseDynamicNumberOfGCThreads sometimes 
>>> moves
>>> work among the queues to get them into queues that will be processed by
>>> an active
>>> GC worker.  Thanks.
>>>
>>
>> Thanks for looking at the patch. I'll do some test runs with those 
>> options.
>
> Apologies for the delay, I thought I had replied, but is seems this 
> bug completely fell of my radar. Anyway, I just re-ran all tonga tests 
> related to reference processing, with the following flag combinations:
>
> 1) -XX:+UseG1GC -XX:ParallelGCThreads=8 -XX:+UseDynamicNumberOfGCThreads
> 2) -XX:+UseG1GC -XX:ParallelGCThreads=8 -XX:-UseDynamicNumberOfGCThreads
> 3) -XX:+UseParallelGC -XX:ParallelGCThreads=8 
> -XX:+UseDynamicNumberOfGCThreads
> 4) -XX:+UseParallelGC -XX:ParallelGCThreads=8 
> -XX:-UseDynamicNumberOfGCThreads
>
> They all passed.
>
> I also revisited the code and I can't see anything problematic 
> interactions with UseDynamicNumberOfGCThreads.
>

Ok.  Thanks for the check.

Jon

> cheers,
> Per
>
>>
>> cheers,
>> /Per
>>
>>
>>> Jon
>>>
>>>
>>> On 08/31/2015 07:15 AM, Per Liden wrote:
>>>>
>>>> Hi,
>>>>
>>>> While preparing a different patch I looked through the
>>>> ReferenceProcessor and noticed what looks like a silent mistreatment
>>>> of SoftReferences.
>>>>
>>>> Summary: RefProcPhase1Task uses WorkerThread::id() to select the list
>>>> to process. After the recent changes to how WorkGang works[1], this no
>>>> longer guarantees that all reference lists are processed. This in turn
>>>> means we might kill SoftReferences even when the policy says they
>>>> should be kept alive.
>>>>
>>>> The fix here would be to use the worker_id (named "i" in this code)
>>>> passed into the Task's work() function instead of using
>>>> WorkerThread::id().
>>>>
>>>>
>>>> [1] There's no longer any guarantee that all GangWorker threads will
>>>> execute the Task's work() function, since a quick-to-wakeup worker
>>>> thread might steal the work intended for a slow-to-wakeup worker 
>>>> thread.
>>>>
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~pliden/8134749/webrev.0/
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8134749
>>>>
>>>> Testing: jprt
>>>>
>>>>
>>>> Btw, I'll to send out a follow-up patch (JDK-8134744) which makes sure
>>>> WorkerThread::id() and the worker_id always match to avoid this type
>>>> of bugs in the future.
>>>>
>>>> /Per
>>>