RFR(M) : 8059624 : Test task: WhiteBox API for testing segmented codecache feature

[Igor Ignatyev]

updated webrev: http://cr.openjdk.java.net/~iignatyev/8059624/webrev.01/
679 lines changed: 643 ins; 2 del; 34 mod;

Hi Vladimir/Tobias,

thanks for review. please see my answers inline.

On 11/06/2014 02:55 AM, Vladimir Kozlov wrote:
> Hi Igor,
>
> codeBlob.hpp
>
> Why you need?:
>
> +   // placement
> +   inline void* operator new(size_t s, void* p) throw() { return p; }
I need this to execute ctor in WhiteBox::allocateCodeHeap:
> 887   new (blob) BufferBlob("WB::DummyBlob", full_size);

it's impossible to use default placement new, since BufferBlob overloads 
operator new.

> compileBroker.cpp
>
> I think MonitorLockerEx locker() should be before the loop. Then you may
> not need to get lock in WB_UnlockCompilation.
I moved MonitorLockerEx before the loop, but I still need a lock in 
UnlockCompilation to call notify_all();

> whitebox.cpp
>
> allocateCodeHeap() - what if CodeCache::allocate() failed to allocate?
it will return null, see AllocationCodeHeapTest.java:
>   93         while ((addr = WHITE_BOX.allocateCodeHeap(size, type.id)) != 0) {
>   94             blobs.add(addr);
>   95         }

> In WB_GetCodeHeapEntries() checks that blobs array is not empty after
> the loop which add entries.
should I? I thought that blobs.length() will be 0 and blobs.begin() will 
equal blobs.end() in this case, won't it?

> You need to prevent sweeping when you processing codecache blobs
> (getCodeHeapEntries). Otherwise pointers from getCodeBlobs() may point
> to dead or overwritten space, consider -Xcomp -XX:+DeoptimizeALot
sweeper does its work after acquiring CodeCache_lock and there is lock 
CodeCache before 1st loop, so I introduced struct CodeBlobStub and store 
it in GrowableArray instead of CodeBlob.
>
> Thanks,
> Vladimir

On 11/06/2014 12:11 PM, Tobias Hartmann wrote:
> Hi Igor,
>
> looks good (not a reviewer). In addition to Vladimir's comments:
>
> 'WhiteBox::allocateCodeHeap' should be 'allocate_code_heap'. I think the name is
> a bit misleading since we don't allocate a CodeHeap but a CodeBlob in a
> CodeHeap. Maybe use 'code_heap_allocate' or 'allocate_code_blob'. Same for
> 'WB_AllocateCodeHeap'.
agree, changed it to AllocateCodeBlob/FreeCodeBlob
// I just tried to mimic WB_AllocateMetaspace
>
> In 'BlobType::getAvailable' you always return the NonProfiledCodeHeap if
> CodeCacheSegmentation is enabled. But if we only use the interpreter (-Xint), we
> don't have any method code heaps. Also with 'TieredStopAtLevel <=
> CompLevel_simple' we don't need the ProfiledCodeHeap (see
> 'CodeCache::heap_available'). The corresponding tests will hit the "heap is
> null" assert in 'CodeCache::allocate'.
I updated 'BlobType::getAvailable' to handle such the situations.

> On 11/5/14 8:25 AM, Igor Ignatyev wrote:
>> http://cr.openjdk.java.net/~iignatyev/8059624/webrev.00/
>> 660 lines changed: 624 ins; 2 del; 34 mod;
>>
>> Hi all,
>>
>> please review the patch which adds new WhiteBox methods needed for
>> better testing SegmentedCodeCache:
>>   - perform allocation in code cache
>>   - force code cache sweep
>>   - lock/unlock compilation
>>   - get a segment id for nmethod.
>> besides these methods, the patch also adds a method to get all entries
>> in code heap.
>>
>> changes in product code:
>>   - monitor 'Compilation_lock' was added to implement compilation
>> locking/unlocking
>>   - align_code_offset function was made a static member of CodeBlob
>>   - WhiteBox was made a friend of several classes
>>
>> testing: jprt, new added tests
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8059624
>>