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

[Filipp Zhinkin]

I've asked because the webrev was uploaded to 8059613/**,
so please make sure that it will be pushed along with 8066440.

Thanks,
Filipp.

On 12/23/2014 03:35 PM, Dmitrij Pochepko wrote:
> Hi,
> yes, correct.
>
> Thanks,
> Dmtirij
>> 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
>>>
>>
>