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

[Dmitrij Pochepko]

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
>>
>