RFR(M) : 8064669 : compiler/whitebox/AllocationCodeBlobTest.java crashes / asserts

[Igor Ignatyev]

Thank you, Tobias.

Igor

On 11/27/2014 02:12 PM, Tobias Hartmann wrote:
> Hi Igor,
>
> looks good (not a reviewer). I like the changes to 'sweeper.cpp'.
>
> Best,
> Tobias
>
> On 26.11.2014 18:45, Igor Ignatyev wrote:
>> Hi,
>>
>> can I get a review for this fix?
>>
>> Igor
>>
>> On 11/23/2014 11:23 AM, Igor Ignatyev wrote:
>>> http://cr.openjdk.java.net/~iignatyev/8064669/webrev.01/
>>> 346 lines changed: 271 ins; 42 del; 33 mod;
>>>
>>> On 11/21/2014 06:33 PM, Igor Ignatyev wrote:
>>>> Vladimir,
>>>>
>>>> On 11/20/2014 10:37 PM, Vladimir Kozlov wrote:
>>>>> Could you add comments to create_sweeper_thread()? It is not clear from
>>>>> first glance.
>>>> sure.
>>> - replaced instantiation of j.l.Thread by Thread::allocate_threadObj method
>>> - added comments
>>>>>
>>>>> You create a second CodeCacheSweeperThread (first is created during VM
>>>>> startup) to just run sweeper_thread_entry(). Right? How these 2 threads
>>>>> interacts (I mean do we need locks?). Or CodeCache_lock in
>>>>> sweeper_thread_entry() is enough?
>>>> it is enough for everything except statistics like _flushed_count,
>>>> _zombified_count, _total_time_sweeping, etc. I'll think how to deal w/
>>>> them.
>>> - introduced NMethodSweeper::_stat_lock -- the mutex which is used
>>> during sweeper's statistics updates
>>> - moved statistics updates from NMethodSweeper::process_nmethod
>>>>>
>>>>> How you stop InfiniteLoop daemon thread? It will run after VM exits?
>>>> not it won't, deamon threads stop on VM exit.
>>>>
>>>>>
>>>>> The rest change seems fine.
>>> during testing, I found that sweeper_thread can complete its work and be
>>> deleted, so sweeper_thread->threadObj() can be collected before
>>> JNIHandles::make_local creates a handler. to prevent such situations, I
>>> moved Thread::start after JNIHandles::make_local
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 11/14/14 7:03 AM, Igor Ignatyev wrote:
>>>>>> http://cr.openjdk.java.net/~iignatyev/8064669/webrev.00/
>>>>>> 261 lines changed: 231 ins; 22 del; 8 mod;
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Please review patch:
>>>>>>
>>>>>> Problems:
>>>>>> 0. NMethodSweeper::possibly_sweep is executed outside
>>>>>> CodeCacheSweeperThread
>>>>>> 1. DummyBlob is initialized after the lock is released, so other
>>>>>> threads
>>>>>> are able to see uninitialized blob
>>>>>> 2. NMethodSweeper::sweep_code_cache can pass null to process_nmethod,
>>>>>> this also can lead to a crash. this situation can happen if safepoint
>>>>>> happens during sweeping and after safepoint another thread iterates
>>>>>> over
>>>>>> all nmethods.
>>>>>>
>>>>>> Fix:
>>>>>> 0. WB_ForceNMethodSweep use CodeCacheSweeperThread to execute
>>>>>> possibly_sweep
>>>>>> 1. move ctor invocation to locked section
>>>>>> 2. handle safepoint after processing nmethod in
>>>>>> NMethodSweeper::sweep_code_cache
>>>>>>
>>>>>> 3. added new test to verify that forceNMethodSweep actually work
>>>>>> 4. AllocationCodeBlobTest is modified
>>>>>>    - to checks that deoptimization works fine w/ dummy blobs
>>>>>>    - to execute forceNMethodSweep more often to increase a chance to
>>>>>> catch such kind problems
>>>>>>
>>>>>> jbs:https://bugs.openjdk.java.net/browse/JDK-8064669
>>>>>> testing: jprt + compiler/whitebox tests