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

[Tobias Hartmann]

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