[13] RFR(M): 8163511: Allocation of compile task fails with assert: "Leaking compilation tasks?"

[Tobias Hartmann]

Thanks again, Vladimir.

Best regards,
Tobias

On 06.03.19 19:27, Vladimir Kozlov wrote:
> Yes, this looks good to me too.
> 
> Thanks,
> Vladimir
> 
> On 3/6/19 3:14 AM, Erik Österlund wrote:
>> Hi Tobias,
>>
>> On 2019-03-06 11:50, Tobias Hartmann wrote:
>>> Hi Erik,
>>>
>>> On 05.03.19 09:09, Erik Osterlund wrote:
>>>> Liveness of objects only change in safepoints. So if you are in the vm thread state, and ask if
>>>> a holder is alive, its result is stable.
>>>>
>>>> With concurrent class unloading, marking terminates in a safepoint. At that snapshot in time, it
>>>> is determined what is alive (strongly and through finalizers) and dead (although it might be
>>>> lazily evaluated during concurrent execution). When that safepoint is released, that view will
>>>> be consistent.
>>>>
>>>> As there are seemingly no safepoint checks in this code, all liveness information will be
>>>> consistent and none of the imagined races exist with concurrent class unloading. It behaves as
>>>> if it was all done in a safepoint, even though it was not.
>>>>
>>>> The key is that the klass_holder liveness (as well as the jweaks you use) is determined using a
>>>> phantom load via the access API. That phantom load gives consistent results in the vm thread
>>>> state as-if reference processing was done in a safepoint. The phantom loads can lazily compute
>>>> what the value would have been, had reference processing been done in a safepoint, as it has
>>>> stable liveness information from that safepoint. Then concurrent reference processing races for
>>>> producing the same values.
>>>>
>>>> Basically there is nothing to worry about here as far as I can see. Looks good.
>>> Thanks a lot for the thorough explanation and the review.
>>>
>>>> Although freeing the jweaks in one place instead of two would make the code easier to follow. Up
>>>> to you though and I don’t need another webrev for that.
>>> I could only factor the freeing into a separate method and call it from CompileTask::free() and
>>> CompileTask::select_for_compilation(). But I don't think that would make the code easier to read.
>>
>> Ah, okay.
>>
>>> Relying on the assumption that either both _method_holder and _hot_method_holder are weak references
>>> or both are strong, I've slightly refactored the code in CompileTask::free():
>>>
>>> http://cr.openjdk.java.net/~thartmann/8163511/webrev.02
>>
>> The new webrev looks good to me.
>>
>> Thanks,
>> /Erik
>>
>>>
>>> Best regards,
>>> Tobias
>>