RFR: 8059613 -JEP-JDK-8043304: Test task: JMX- tests and JDK-8066440 - Various changes in testlibrary for JDK-8059613

[Filipp Zhinkin]

Hi Dmitrij,

following change in BlobType:

http://cr.openjdk.java.net/~iignatyev/dpochepk/8059613/top/webrev.00

should be pushed as part of the fix for 8066440, right?

Thanks,
Filipp.

On 12/15/2014 04:53 PM, Dmitrij Pochepko wrote:
> Hi,
> see comments below
>> Hi Dmitrij,
>>
>> For PeakUsageTest I'm still worried that the asserts may fail if class loading
>> happens in between and triggers compilation of adapters.
> non-nmethods heap is excluded from 
> testing(CodeCacheUtils.isCodeHeapPredictable is used to filter out heaps 
> because of this exact reason).
>>
>> Why did you surround two of the asserts with a try? Like this the assert is
>> useless because we ignore failures.
> there is no catch block there - it's just finally block to be sure we've 
> cleaned up memory allocated before,
> since assertion can get us out without deallocating memory otherwise.
>
> Thanks,
> Dmitrij
>>
>> Otherwise it looks good.
>>
>> Best,
>> Tobias
>>
>> On 12.12.2014 23:39, Dmitrij Pochepko wrote:
>>> Hi all,
>>>
>>> i've corrected diff according to comments.
>>>
>>> please see corrected version(also modified according to latest commits)
>>>
>>> http://cr.openjdk.java.net/~iignatyev/dpochepk/8059613/top/webrev.00
>>> http://cr.openjdk.java.net/~iignatyev/dpochepk/8059613/webrev.03
>>> http://cr.openjdk.java.net/~iignatyev/dpochepk/8066440/webrev.00
>>>
>>> a jprt testing job has been created:
>>> http://scaaa637.us.oracle.com//archive/2014/12/2014-12-12-215231.mephudson.hs-comp 
>>>
>>>
>>> Thanks,
>>> Dmitrij
>>>> Hi Dmitrij,
>>>>
>>>> here are some comments about the tests (not a reviewer).
>>>>
>>>> UsageThresholdNotExceededTest:
>>>> - You can use CodeCacheUtils.MIN_ALLOCATION instead of
>>>> CodeCacheUtils.SEGMENT_SIZE * CodeCacheUtils.MIN_BLOCK_LENGTH.
>>>> - Please add a comment to 'CodeCacheUtils.WB.fullGC()'
>>>>
>>>> UsageThresholdIncreasedTest:
>>>> - Typo "hasn't been hit after allocationg smaller" -> "allocating"
>>>> - Maybe you should disable code cache sweeping here as well (as you did for 
>>>> the
>>>> UsageThresholdNotExceededTest)
>>>>
>>>> UsageThresholdExceededTest:
>>>> - Calling 'CodeCacheUtils.hitUsageThreshold' may _not_ hit the usage threshold
>>>> if the code cache sweeper is enabled and removes nmethods in between. I think
>>>> you should disable code cache sweeping.
>>>> - Please print information about the counters if the assert is hit in line 66.
>>>>
>>>> ThresholdNotificationsTest:
>>>> - Typo "testing of getUsageThreashold()" -> "getUsageThreshold()"
>>>> - If not all notifications are received/handled, the test will timeout at line
>>>> 87, right? Maybe you can add a timeout value to 'Utils.waitForCondition' and
>>>> print a useful error message if the timeout is reached.
>>>>
>>>> PoolsIndependenceTest:
>>>> - Typo "testing of getUsageThreashold()" -> "getUsageThreshold()"
>>>> - Typos "event to be recieved ..." ->   "received" (3x)
>>>>
>>>> PeakUsageTest:
>>>> - Compilation is not disabled. The asserts may fail because a method is 
>>>> compiled
>>>> in between. Even with compilation disabled, adapters and other non-nmethod 
>>>> code
>>>> may be generated and stored in the non-nmethod code heap.
>>>>
>>>> ManagerNamesTest:
>>>> - Typo: "calls in case on segmented code cache" ->   "of segmented"
>>>>
>>>> GetUsageTest:
>>>> - Why do you need 'CodeCacheUtils.WB.deoptimizeAll()'?
>>>>
>>>> General:
>>>> - Is is necessary to have CodeCacheUtils.allocateDefaultSize? In my opinion it
>>>> would be easier to understand if you invoke WB.allocateCodeBlob(...) directly.
>>>>
>>>> Please run the tests on JPRT!
>>>>
>>>> Thanks,
>>>> Tobias
>>>>
>>>> On 04.12.2014 20:13, Dmitrij Pochepko wrote:
>>>>> Hi,
>>>>>
>>>>> Please review changes for
>>>>> https://bugs.openjdk.java.net/browse/JDK-8059613 - JEP-JDK-8043304: Test 
>>>>> task:
>>>>> JMX- tests
>>>>> and
>>>>> https://bugs.openjdk.java.net/browse/JDK-8066440 - Various changes in
>>>>> testlibrary for JDK-8059613
>>>>>
>>>>> It introduce a number of tests for segmented code cache, mostly testing
>>>>> thresholds, usage,
>>>>> memory notifications using respective MemoryPoolMXBean(s).
>>>>> There is also a small modification for testlibrary(8066440) used in tests.
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~iignatyev/dpochepk/8059613/webrev.01/
>>>>>
>>>>> Testing: manually, using fastdebug nightly build from 2014-12-02
>>>>>
>>>>> Additional note: this patch assumes
>>>>> "https://bugs.openjdk.java.net/browse/JDK-8065134 -
>>>>> Need WhiteBox::allocateCodeBlob(long, int) method to be implemented" is 
>>>>> fixed.
>>>>>
>>>>> Thanks,
>>>>> Dmitrij
>