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

[Erik Osterlund]

Hi Dean,

Thanks for the review!

/Erik

> On 30 Nov 2018, at 21:21, dean.long at oracle.com wrote:
> 
>> On 11/30/18 10:47 AM, Erik Osterlund wrote:
>> Hi Dean,
>> 
>>>> On 30 Nov 2018, at 19:41, dean.long at oracle.com wrote:
>>>> 
>>>> On 11/30/18 3:19 AM, 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.
>>>> 
>>> Is it correct to say that the loop was there before (in new_ic_stub), but now you've moved it into the callers?
>> Yes, that is exactly right.
> 
> Sounds good!
> 
> dl
> 
>> 
>> /Erik
>> 
>>> dl
>>> 
>>>> 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
>