RFR: 8214338: Move IC stub refilling out of IC cache transitions

[Erik Österlund]

Any more takers? I need one more review. I have passed hs-tier1-6 twice 
with this now.

/Erik

On 2018-11-30 12:19, Erik Österlund wrote:
> Hi Dean,
> 
> Thanks for reviewing this!
> 
> On 2018-11-27 23:55, dean.long at oracle.com wrote:
>> There's a typo in the new guarantee message: "tranisition".
> 
> Fixed in-place.
> 
>> Regarding the infinite loops, can we detect if progress isn't being 
>> made and assert/panic, rather than hanging?
> 
> I'm afraid not. It's a similar analogy to the problem of progress 
> guarantees of a lock-free algorithm. They have a global progress 
> guarantee for the system, but no local guarantee of progress for each 
> operation, that can in theory be starved indefinitely due to other 
> operations winning every race. The situation is similar here, at least 
> in theory, that the thread refilling the IC stubs didn't get a single 
> stub back, because another thread started depleted the IC stubs again. 
> Naturally, if this was ever to happen, we would get time outs in our 
> tests though, to indicate that there is a problem.
> 
> Thanks,
> /Erik
> 
>> dl
>>
>> On 11/27/18 1:29 PM, Erik Österlund wrote:
>>> Hi Dean,
>>>
>>> Thank you for reviewing this.
>>>
>>> Full webrev:
>>> http://cr.openjdk.java.net/~eosterlund/8214338/webrev.01/
>>>
>>> Incremental webrev:
>>> http://cr.openjdk.java.net/~eosterlund/8214338/webrev.00_01/
>>>
>>> On 2018-11-27 19:07, dean.long at oracle.com wrote:
>>>> Hi Erik.  Can you explain why you removed the pre-allocated "next 
>>>> stub"?  I guess it was no longer necessary?  If so, then you should 
>>>> update the "always has sentinel" comment in is_empty.
>>>
>>> Updated. Indeed, the sentinel no longer serves a purpose so I removed 
>>> it. Found some more traces of the sentinel that I removed in the last 
>>> webrev.
>>>
>>>> In your guarantee messages, some use "should not fail" and some use 
>>>> "should never fail".  It's not a big deal but maybe they should be 
>>>> the same.
>>>
>>> Sure. Updated.
>>>
>>>> You introduced a couple of infinite loops.  Are we guaranteed to 
>>>> exit these loops eventually?  Is there an upper bound on how many 
>>>> iterations are needed?
>>>
>>> In my latest webrev I removed some unnecessary set_to_clean() where 
>>> IC caches are already is_clean(). With that in place, there should be 
>>> global progress guarantees and a single IC stub in the buffer should 
>>> be all you really "need". Although, you might want more. In fact, I'm 
>>> questioning if the 10K size buffer is big enough today, but I'll 
>>> leave that for another day.
>>>
>>> Thanks,
>>> /Erik
>>>
>>>> The rest looks good.
>>>>
>>>> dl
>>>>
>>>> On 11/27/18 5:00 AM, Erik Österlund wrote:
>>>>> Hi,
>>>>>
>>>>> IC stub refilling currently requires a safepoint operation. When 
>>>>> done right at the point where an CompiledIC is about to get patched 
>>>>> to a transitional state using an IC stub, locks may already be 
>>>>> held, causing a bunch of locking issues - especially for concurrent 
>>>>> class unloading. Therefore, the IC stub refilling ought to be moved 
>>>>> out so that IC cache cleaning and transitioning may be done without 
>>>>> any safepoints, and the locks in the path ought to not perform 
>>>>> safepoint checking.
>>>>>
>>>>> This is implemented by allowing IC transitions to fail when they 
>>>>> require IC stubs, and we run out of them. This propages back to a 
>>>>> higher level where IC stubs are refilled after having released the 
>>>>> IC locker.
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~eosterlund/8214338/webrev.00/
>>>>>
>>>>> Bug:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8214338
>>>>>
>>>>> Thanks,
>>>>> /Erik
>>>>
>>