RFR : 8012447 : Java CTW implementation

[Igor Ignatyev]

Thank you again, Vladimir.

Could I have a second review for this?

Best regards,
Igor Ignatyev

On 08/05/2013 09:16 PM, Vladimir Kozlov wrote:
> This looks good.
>
> thanks,
> Vladimir
>
> On 8/3/13 3:57 PM, Igor Ignatyev wrote:
>> Vladimir,
>> Thank you for review.
>> Answers inline.
>>
>> On 07/25/2013 09:43 PM, Vladimir Kozlov wrote:
>>> Hi, Igor,
>>>
>>> Add copyright header to README file.
>> added
>>> Typo in CtwTest.java:
>>>
>>> 35  * @summary testing of CompileTheWorl
>> fixed
>>> Capitalize foo, bar class names since it is java classes.
>> done
>>> Why test/testlibrary/whitebox/Makefile removes ctw.jar when it does not
>>> build it?
>> forgot to edit after copy&paste, fixed
>>
>>> CtwTest.java:
>>>
>>> - Each CTW veriation should be tested separately - separate '@run
>>> main/othervm' lines. Yes, check() should be modified for that (or hacve
>>> several version of check()).
>> I've split test into separate files.
>>
>>> - How createJarProcessBuilder() know where foo.class and bar.class are
>>> located? Usually jtreg creates them in not run directory. You copied
>>> them into classes subdir but you did not use those copies in jar
>>> creation. At least I don't see how.
>> They're copied to the run directory by 'ClassFileInstaller', see
>> jtreg-script in 'JarsTest'/'JarDirTest'
>>
>>> classes.lst
>>>     1 java.lang.String
>>>     2 java.lang.Object
>>>
>>> What if I put foo, bar into classes.lst, where it will find these
>>> classes? The example in README does not show that I need to add foo.jar
>>> or foo.class
>> Classes from 'classes.lst' will be loaded by default classloader. I have
>> added a description of that behavior into 'README' (lines 56-57) and
>> Foo, Bar into 'classes.lst'.
>>
>>> It is not intuitive why compileWholeClass() is in PathHandler.java. It
>>> should be in separate class.
>> All related to compilation logics were moved to 's.h.t.ctw.Compiler'
>>
>>> These tests may have problem on embedded systems where java runs from
>>> JRE installation which does not have jar, javac and jtreg sets 2
>>> variables COMPILEJAVA and TESTJAVA which could be different (based on
>>> jtreg's flags specified by user). Please, verify such setting when you
>>> test.
>> I modified 'JDKToolFinder.getJDKTool()' to use 'compile.jdk' property
>> (which is used on embedded systems as a path to full jdk) and added
>> 'JDKToolFinder.getCurrentJDKTool()' to retain behavior of
>> 'gc/TestVerifyDuringStartup' test.
>>
>> Also I added try-catch blocks to support profiles which doesn't contain
>> 'j.l.management' package.
>>
>> Additionally tested: runtime/NMT, runtime/RedefineObject,
>> gc/TestVerifyDuringStartup, gc/TestG1ZeroPGCTJcmdThreadPrint
>>
>> Updated webrev: http://cr.openjdk.java.net/~iignatyev/8012447/webrev.01/
>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 7/24/13 10:25 PM, Igor Ignatyev wrote:
>>>> Hi all,
>>>>
>>>> Please review patch.
>>>>
>>>> DESCRIPTION
>>>> This is replacement for CompileTheWorld (CTW) written on java. Its
>>>> purpose is to make possible the use of CTW in product builds.
>>>>
>>>> More detailed description can be found in
>>>> 'test/testlibrary/ctw/README'.
>>>>
>>>> I also added Makefile for WhiteBox to simplify its using outside the
>>>> jtreg-testbase.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8012447/webrev.00/
>>>> jbs: https://jbs.oracle.com/bugs/browse/JDK-8012447
>>>> testing: jprt testlibrary/ctw