[9] RFR(S): 8064892: Non-methods code cache overflow is not handled correctly

[Tobias Hartmann]

Hi Vladimir,

On 18.08.2016 16:46, Vladimir Kozlov wrote:
> On 8/18/16 12:29 AM, Tobias Hartmann wrote:
>> Hi Vladimir,
>>
>> thanks for the review!
>>
>> On 17.08.2016 21:42, Vladimir Kozlov wrote:
>>> The print code (include guarding check) should be under #ifdef ASSERT since the flag is debug flag.
>>
>> Why is that necessary? Develop flags are replaced by constants in product and such checks will be constant folded:
>> globals.hpp:583: // develop flags are settable / visible only during development and are constant in the PRODUCT version
>>
>> We have the same (unguarded) check below in codeCache.cpp:495 and in general we don't guard other develop flags with #ifdef ASSERT (for example, TraceICs).
> 
> My mistake (I was confused by udiffs). I thought "(type != code_blob_type && ..." check guards only assert code.
> Yes, you don't need #ifdef here.

Right, the udiff really looks confusing here.

>>> Otherwise looks good. So JFR will get this new message instead. Right?
>>
>> By "new message", do you mean the code cache full message now reporting the 'orig_code_blob_type'? If so, yes, the originally requested code blob type is passed to the JFR event via CompileBroker::handle_full_code_cache() -> CodeCache::report_codemem_full() -> EventCodeCacheFull::set_codeBlobType(). But as before, a JFR event is triggered only if the entire code cache is full. We don't trigger a "Code cache full" event if only a single segment/heap is full and we are able to store the code in another heap. For this case, I added the PrintCodeCacheExtension output.
> 
> Reading through bug report and it was requested originally always fire an event even if there is space in other segments.
> But you don't do that (case 2.) with reasonable arguments and I agree. But you need agreement from Dmitrij who filed the bug.

Okay, sure. Dmitrij, are you okay with the changes?

Thanks,
Tobias

> 
> Thanks,
> Vladimir
> 
>>
>> Thanks,
>> Tobias
>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 8/17/16 4:04 AM, Tobias Hartmann wrote:
>>>> Hi,
>>>>
>>>> please review the following patch:
>>>> https://bugs.openjdk.java.net/browse/JDK-8064892
>>>> http://cr.openjdk.java.net/~thartmann/8064892/webrev.00/
>>>>
>>>> There are two problems with the current handling of code cache exhaustion:
>>>> 1) If the non-nmethod code heap is full, we always try to use the non-profiled or the profiled code heap as a fallback. If those heaps are full as well, we issue a warning stating that the last tried code heap is full. Like this, we never print a warning for the non-nmethod code heap.
>>>> 2) If a code heap is full but we still have enough space in another code heap, no warning or JFR event is triggered.
>>>>
>>>> I fixed 1) by keeping track of the originally requested code blob type and use it for printing the code cache full message if allocation in other heaps failed. Regarding 2), I don't think a code cache full message or JFR event should be triggered because the code cache is not full, we just failed to allocate in the preferred segment. In general, this is expected because the amount of code generated at runtime is not static but segment sizes are. The user should not worry about such fallbacks and therefore we should only print a warning if the entire code cache is full. However, to keep track of this, I added debug output that can be enabled via -XX:+PrintCodeCacheExtension and prints a message if allocation in the preferred segment failed. For example, "Extension of CodeHeap 'non-nmethods' failed. Trying to allocate in CodeHeap 'non-profiled nmethods'".
>>>>
>>>> Tested with JPRT and RBT (running).
>>>>
>>>> Thanks,
>>>> Tobias
>>>>