Thanks, Vladimir.
Best regards,
Tobias
On 17.03.2016 16:04, Vladimir Kozlov wrote:
> Looks good.
>> Thanks,
> Vladimri
>> On 3/17/16 1:29 AM, Tobias Hartmann wrote:
>> Hi Vladimir,
>>>> thanks for the review!
>>>> On 16.03.2016 19:38, Vladimir Kozlov wrote:
>>> Nice graphs!
>>>>>> nmethod.hpp - we try to avoid calling SNRH() in such case but use fatal() with printing value (_state in this case) to get more info in error output.
>>>> Okay, I changed it to _fatal.
>>>>> In invalidate_osr_method() add {} for != NULL check.
>>>> Done.
>>>>> I am a little worry about next change:
>>>>>> ! if (is_osr_method() && is_in_use()) {
>>> invalidate_osr_method();
>>>>>> May be we should move is_in_use() check inside invalidate_osr_method() and in false case in debug VM do verification that osr nmethod is not on the list.
>>>> Right, I changed remove_osr_nmethod() to return true if the osr nmethod was found/removed and moved the is_in_use() check into invalidate_osr_method(). Like his, we verify that the nmethod was invalidated if it is !is_in_use().
>>>> New webrev:
>>http://cr.openjdk.java.net/~thartmann/8023191/webrev.01/>>>> Thanks,
>> Tobias
>>>>> Otherwise looks good.
>>>>>> Thanks,
>>> Vladimir
>>>>>> On 3/16/16 10:10 AM, Tobias Hartmann wrote:
>>>> Hi,
>>>>>>>> please review the following patch.
>>>>>>>>https://bugs.openjdk.java.net/browse/JDK-8023191>>>>http://cr.openjdk.java.net/~thartmann/8023191/webrev.00/>>>>>>>> Currently, we only remove unloaded/not-entrant OSR nmethods from the code cache but never flush alive and unused ones. This is a problem if many OSR compilations are triggered but the methods are only used for a short time. If the corresponding classes are not unloaded, these nmethods will never be flushed and occupy space in the code cache. This can lead to a drop in performance from which we may never recover (see discussions [1], [2]).
>>>>>>>> My fix enables flushing of OSR nmethods, treating them the same way as we treat "normal" compilations. I implemented a fast path, that allows the sweeper to flush zombie OSR nmethods directly because they are never referenced by an inline cache.
>>>>>>>> I refactored the debug printing in NMethodSweeper::process_nmethod() to get consistent output if OSR nmethods are flushed. During testing, I noticed that we need to clean the CodeCacheSweeperThread::_scanned_nmethod reference to the flushed nmethod in NMethodSweeper::release_nmethod() because otherwise the GC may visit the zombie nmethod at a safepoint that may occur during sweeping.
>>>>>>>> I also fixed the make_unloaded()/make_zombie() code to only invoke nmethod::invalidate_osr_method() once and did some small typo/comment cleanups in related code.
>>>>>>>> I've run jittest with the latest JDK 9 build and -XX:ReservedCodeCacheSize=20m -XX:-ClassUnloading. Graph [3] shows the results: the code cache fills up quickly and performance degrades significantly. We don't recover because OSR nmethods are not flushed and we therefore don't have enough space in the code cache to compile the methods of newly loaded classes. Graph [4] shows that the problem is solved by my fix. The code cache does not completely fill up and performance remains stable at a high level. Also the number of compilations is higher with the fix (86546 vs. 132495).
>>>>>>>> Testing:
>>>> - JPRT
>>>> - RBT with hotspot_all and -Xcomp/-Xmixed
>>>> - Nashorn + Octane with -XX:StartAggressiveSweepingAt=100/50 -XX:NmethodSweepActivity=500/100 and 100 runs each
>>>>>>>> Thanks,
>>>> Tobias
>>>>>>>> [1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-February/021732.html>>>> [2] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-March/021750.html>>>> [3] https://bugs.openjdk.java.net/secure/attachment/57899/base.png>>>> [4] https://bugs.openjdk.java.net/secure/attachment/57898/fix.png>>>>