RFR: 8059625 - JEP-JDK-8043304: Test task: DTrace- tests for segmented codecache feature

[Dmitrij Pochepko]

Hi,
please review updated webrev

http://cr.openjdk.java.net/~iignatyev/dpochepk/8059625/webrev.01/

Thanks,
Dmitrij
> On 12/23/14 12:37 AM, Dmitrij Pochepko wrote:
>> Hi,
>>
>> Thank you for catching these issues.
>> I have a question regarding last comment: does it make any difference 
>> to change "reading of static member 3 times" to "copying into static 
>> member of another class and then read it 3 times"?
>
> Yes, it does (it is a minor issue though).
> It is because the class names are pretty big.
> It would simplify the code and improve readability, right?
>
> You already do it two times:
>   179             List<Executable>  tml
>   180                     = SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
>   ...
>   235             List<Executable>  mlist
>   236                     = SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
>
> My suggestion is to do it once somewhere before the line 74:
>    static final List<Executable>  MLIST = SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
> and then use the mlist in other places where it is needed:
>
>    75         String params = MLIST.stream()
>   ...
>   182                 d.put(MLIST.get(i).getName(), new MethodData(MLIST.get(i).getName(),
>   ...
>   239             for (int i = MLIST.size() - 1; i>  -1; i--) {
>   240                 sb.append(MLIST.get(i).getName()).append(delimeter);
> These lines will go away:
>   179             List<Executable>  tml
>   180                     = SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
>   ...
>   235             List<Executable>  mlist
>   236                     = SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
>
>
> Both private static classes are inner classes of the public one.
> The MLIST will be in a context for the inner classes, and so, needs no 
> prefixing.
>
>
> Thanks,
> Serguei
>
>>
>> Thanks,
>> Dmitrij
>>> Hi Dmitry,
>>>
>>>
>>> It looks good in general.
>>> Some minor comments are below.
>>>
>>> test/testlibrary/com/oracle/java/testlibrary/dtrace/DtraceRunner.java
>>>
>>>    42     public static final String PERMIT_DESCTUCTIVE_ACTIONS_DTRACE_OPTION = "w";
>>>    A typo in the constant name: DESCTUCTIVE => DESTRUCTIVE
>>>
>>>
>>> test/compiler/codecache/dtrace/SegmentedCodeCacheDtraceTest.java
>>>
>>>    84                 DtraceRunner.PERMIT_DESCTUCTIVE_ACTIONS_DTRACE_OPTION,
>>>    A typo in the constant name: DESCTUCTIVE => DESTRUCTIVE
>>>
>>>
>>>    60     private static final String WORKER_CLASS_NAME
>>>    61             = SegmentedCodeCacheDtraceTestWorker.class.getName();
>>>   ...
>>>    80         runner.runDtrace(JDKToolFinder.getTestJDKTool("java"), JAVA_OPTS,
>>>    81                 SegmentedCodeCacheDtraceTestWorker.class.getName(), params,
>>>    The WORKER_CLASS_NAME can be used at 81.
>>>
>>>    75         String params = SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST.stream()
>>>   ...
>>>   179             List<Executable>  tml
>>>   180                     = SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
>>>   ...
>>>   235             List<Executable>  mlist
>>>   236                     = SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
>>>     The TESTED_METHODS_LIST is used three times.
>>>     It can be cached at the top and re-used.
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>
>>> On 12/19/14 11:03 AM, Dmitrij Pochepko wrote:
>>>> Hi all,
>>>>
>>>> Please review changes for 
>>>> https://bugs.openjdk.java.net/browse/JDK-8059625 -  
>>>> JEP-JDK-8043304: Test task: DTrace- tests for segmented codecache 
>>>> feature
>>>>
>>>> Description: this fix introduce dtrace test, which verify that 
>>>> different combinations of available compile levels(and, in case 
>>>> compile levels allows it, different code heaps as result)  doesn't 
>>>> affect callstack shown by dtrace. There is a control class 
>>>> SegmentedCodeCacheDtraceTest.java and class for running via dtrace 
>>>> SegmentedCodeCacheDtraceTestWorker.java. A dtrace d script is also 
>>>> present (SegmentedCodeCacheDtraceTestScript.d). A control class is 
>>>> using DtraceRunner.java to run dtrace and then analyzing results 
>>>> using class SegmentedCodeCacheDtraceResultsAnalyzer with 
>>>> DtraceResultsAnalyzer interface.
>>>> There is also a small class CompilerUtils.java created for usefull 
>>>> common code.
>>>>
>>>> webrev: 
>>>> http://cr.openjdk.java.net/~iignatyev/dpochepk/8059625/webrev.00/
>>>>
>>>> Additional note: Please note that this path assumes that fix for 
>>>> JDK-8066440 - Various changes in testlibrary for JDK-8059613 is 
>>>> also applied.
>>>>
>>>> Thanks,
>>>> Dmitrij
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20141223/ae5867a1/attachment.html>