Quantcast

RFR(S): 8164944: Refactor ProcessTools to get rid of dependency on java.management

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR(S): 8164944: Refactor ProcessTools to get rid of dependency on java.management

Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
> 92 lines changed: 36 ins; 36 del; 20 mod;

Hi all,

could you please review this small fix which removes dependency on java.management module from the testlibrary?

jdk.test.lib.process.ProcessTools class has getVmInputArgs method which depend on java.management, this method is used only by j.d.t.process.ProcessTools::executeTestJvmAll and j.t.l.cli.CommandLineOptionTest class.
 - ProcessTools::executeTestJvmAll is a variation of executeTestJvm which prepends vm flags from jtreg @run.
 - CommandLineOptionTest is a library class for cli testing, it uses flags from ProcessTools::getVmInputArgs to create a new jvm with all flags
 
the fix removes ProcessTools::executeTestJvmAll method, moves getVmInputArgs to a separate class jdk.test.lib.management.InputArguments. c.c.s.scenario.Executor and j.t.l.cli.CommandLineOptionTest are updated to use getVmInputArgs from that class. c.j.compilerToVM.DebugOutputTest is refactored to pass add-exports and patch module directly. all the affected classes and obviously updated to use executeTestJvm instead of executeTestJvmAll.

after this fix many hotspot tests won't depend on java.management, hence that module might be removed from their @modules, this will be done separately by 8178416.

jbs: https://bugs.openjdk.java.net/browse/JDK-8164944
webrev: http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
testing: :hotspot_all
8178416: https://bugs.openjdk.java.net/browse/JDK-8178416

Thanks,
-- Igor
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S): 8164944: Refactor ProcessTools to get rid of dependency on java.management

Igor Ignatyev
forgot to update copyrights, http://cr.openjdk.java.net/~iignatyev//8164944/webrev.01/index.html

On Apr 12, 2017, at 12:03 PM, Igor Ignatyev <[hidden email]> wrote:

http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
92 lines changed: 36 ins; 36 del; 20 mod;

Hi all,

could you please review this small fix which removes dependency on java.management module from the testlibrary?

jdk.test.lib.process.ProcessTools class has getVmInputArgs method which depend on java.management, this method is used only by j.d.t.process.ProcessTools::executeTestJvmAll and j.t.l.cli.CommandLineOptionTest class.
- ProcessTools::executeTestJvmAll is a variation of executeTestJvm which prepends vm flags from jtreg @run.
- CommandLineOptionTest is a library class for cli testing, it uses flags from ProcessTools::getVmInputArgs to create a new jvm with all flags

the fix removes ProcessTools::executeTestJvmAll method, moves getVmInputArgs to a separate class jdk.test.lib.management.InputArguments. c.c.s.scenario.Executor and j.t.l.cli.CommandLineOptionTest are updated to use getVmInputArgs from that class. c.j.compilerToVM.DebugOutputTest is refactored to pass add-exports and patch module directly. all the affected classes and obviously updated to use executeTestJvm instead of executeTestJvmAll.

after this fix many hotspot tests won't depend on java.management, hence that module might be removed from their @modules, this will be done separately by 8178416.

jbs: https://bugs.openjdk.java.net/browse/JDK-8164944
webrev: http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
testing: :hotspot_all
8178416: https://bugs.openjdk.java.net/browse/JDK-8178416

Thanks,
-- Igor

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S): 8164944: Refactor ProcessTools to get rid of dependency on java.management

Vladimir Kozlov
In reply to this post by Igor Ignatyev
Looks fine to me.

Vladimir

On 4/12/17 12:03 PM, Igor Ignatyev wrote:

> http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
>> 92 lines changed: 36 ins; 36 del; 20 mod;
>
> Hi all,
>
> could you please review this small fix which removes dependency on java.management module from the testlibrary?
>
> jdk.test.lib.process.ProcessTools class has getVmInputArgs method which depend on java.management, this method is used only by j.d.t.process.ProcessTools::executeTestJvmAll and j.t.l.cli.CommandLineOptionTest class.
>  - ProcessTools::executeTestJvmAll is a variation of executeTestJvm which prepends vm flags from jtreg @run.
>  - CommandLineOptionTest is a library class for cli testing, it uses flags from ProcessTools::getVmInputArgs to create a new jvm with all flags
>
> the fix removes ProcessTools::executeTestJvmAll method, moves getVmInputArgs to a separate class jdk.test.lib.management.InputArguments. c.c.s.scenario.Executor and j.t.l.cli.CommandLineOptionTest are updated to use getVmInputArgs from that class. c.j.compilerToVM.DebugOutputTest is refactored to pass add-exports and patch module directly. all the affected classes and obviously updated to use executeTestJvm instead of executeTestJvmAll.
>
> after this fix many hotspot tests won't depend on java.management, hence that module might be removed from their @modules, this will be done separately by 8178416.
>
> jbs: https://bugs.openjdk.java.net/browse/JDK-8164944
> webrev: http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
> testing: :hotspot_all
> 8178416: https://bugs.openjdk.java.net/browse/JDK-8178416
>
> Thanks,
> -- Igor
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S): 8164944: Refactor ProcessTools to get rid of dependency on java.management

Dmitry Fazunenko
In reply to this post by Igor Ignatyev
Hi Igor,

On 12.04.2017 22:14, Igor Ignatyev wrote:
> forgot to update copyrights, http://cr.openjdk.java.net/~iignatyev//8164944/webrev.01/index.html <http://cr.openjdk.java.net/~iignatyev//8164944/webrev.01/index.html>

It's become much better :)
Now the fix looks perfect to me. Thanks for making this change!

Thanks,
Dima

>
>> On Apr 12, 2017, at 12:03 PM, Igor Ignatyev <[hidden email]> wrote:
>>
>> http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
>>> 92 lines changed: 36 ins; 36 del; 20 mod;
>> Hi all,
>>
>> could you please review this small fix which removes dependency on java.management module from the testlibrary?
>>
>> jdk.test.lib.process.ProcessTools class has getVmInputArgs method which depend on java.management, this method is used only by j.d.t.process.ProcessTools::executeTestJvmAll and j.t.l.cli.CommandLineOptionTest class.
>> - ProcessTools::executeTestJvmAll is a variation of executeTestJvm which prepends vm flags from jtreg @run.
>> - CommandLineOptionTest is a library class for cli testing, it uses flags from ProcessTools::getVmInputArgs to create a new jvm with all flags
>>
>> the fix removes ProcessTools::executeTestJvmAll method, moves getVmInputArgs to a separate class jdk.test.lib.management.InputArguments. c.c.s.scenario.Executor and j.t.l.cli.CommandLineOptionTest are updated to use getVmInputArgs from that class. c.j.compilerToVM.DebugOutputTest is refactored to pass add-exports and patch module directly. all the affected classes and obviously updated to use executeTestJvm instead of executeTestJvmAll.
>>
>> after this fix many hotspot tests won't depend on java.management, hence that module might be removed from their @modules, this will be done separately by 8178416.
>>
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8164944
>> webrev: http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
>> testing: :hotspot_all
>> 8178416: https://bugs.openjdk.java.net/browse/JDK-8178416
>>
>> Thanks,
>> -- Igor

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S): 8164944: Refactor ProcessTools to get rid of dependency on java.management

David Holmes
In reply to this post by Igor Ignatyev
Hi Igor,

Overall this looks good.

To be clear the old executeTestJvmAllArgs() presumably provides the VM
options from @run _and_ @module - right? So if they don't exist in a
test we can simply replace the usage of executeTestJvmAllArgs with
executeTestJvm - as done in:

test/compiler/c2/cr7200264/TestDriver.java

And for other tests we either grab the VM input arguments as a separate
step, or else reconstruct them as needed (eg for the module flags in
DebugOutputTest).

Thanks,
David

On 13/04/2017 5:03 AM, Igor Ignatyev wrote:

> http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
>> 92 lines changed: 36 ins; 36 del; 20 mod;
>
> Hi all,
>
> could you please review this small fix which removes dependency on java.management module from the testlibrary?
>
> jdk.test.lib.process.ProcessTools class has getVmInputArgs method which depend on java.management, this method is used only by j.d.t.process.ProcessTools::executeTestJvmAll and j.t.l.cli.CommandLineOptionTest class.
>  - ProcessTools::executeTestJvmAll is a variation of executeTestJvm which prepends vm flags from jtreg @run.
>  - CommandLineOptionTest is a library class for cli testing, it uses flags from ProcessTools::getVmInputArgs to create a new jvm with all flags
>
> the fix removes ProcessTools::executeTestJvmAll method, moves getVmInputArgs to a separate class jdk.test.lib.management.InputArguments. c.c.s.scenario.Executor and j.t.l.cli.CommandLineOptionTest are updated to use getVmInputArgs from that class. c.j.compilerToVM.DebugOutputTest is refactored to pass add-exports and patch module directly. all the affected classes and obviously updated to use executeTestJvm instead of executeTestJvmAll.
>
> after this fix many hotspot tests won't depend on java.management, hence that module might be removed from their @modules, this will be done separately by 8178416.
>
> jbs: https://bugs.openjdk.java.net/browse/JDK-8164944
> webrev: http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
> testing: :hotspot_all
> 8178416: https://bugs.openjdk.java.net/browse/JDK-8178416
>
> Thanks,
> -- Igor
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S): 8164944: Refactor ProcessTools to get rid of dependency on java.management

Igor Ignatyev
Hi David,

right. executeTestJvmAllArgs prepends the VM options which the current JVM was started with, which is all options from jtreg harness: @run, @module, and the options specified in the command line by -javaoptions and -vmoptions to the passed arguments. executeTestJvm prepends only the options specified in the command line. so if there is no options from @run or @module, there is no difference b/w executeTestJvmAllArgs and executeTestJvm. in other cases, we need to do some extra steps.

-- Igor
 

> On Apr 12, 2017, at 6:47 PM, David Holmes <[hidden email]> wrote:
>
> Hi Igor,
>
> Overall this looks good.
>
> To be clear the old executeTestJvmAllArgs() presumably provides the VM options from @run _and_ @module - right? So if they don't exist in a test we can simply replace the usage of executeTestJvmAllArgs with executeTestJvm - as done in:
>
> test/compiler/c2/cr7200264/TestDriver.java
>
> And for other tests we either grab the VM input arguments as a separate step, or else reconstruct them as needed (eg for the module flags in DebugOutputTest).
>
> Thanks,
> David
>
> On 13/04/2017 5:03 AM, Igor Ignatyev wrote:
>> http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
>>> 92 lines changed: 36 ins; 36 del; 20 mod;
>>
>> Hi all,
>>
>> could you please review this small fix which removes dependency on java.management module from the testlibrary?
>>
>> jdk.test.lib.process.ProcessTools class has getVmInputArgs method which depend on java.management, this method is used only by j.d.t.process.ProcessTools::executeTestJvmAll and j.t.l.cli.CommandLineOptionTest class.
>> - ProcessTools::executeTestJvmAll is a variation of executeTestJvm which prepends vm flags from jtreg @run.
>> - CommandLineOptionTest is a library class for cli testing, it uses flags from ProcessTools::getVmInputArgs to create a new jvm with all flags
>>
>> the fix removes ProcessTools::executeTestJvmAll method, moves getVmInputArgs to a separate class jdk.test.lib.management.InputArguments. c.c.s.scenario.Executor and j.t.l.cli.CommandLineOptionTest are updated to use getVmInputArgs from that class. c.j.compilerToVM.DebugOutputTest is refactored to pass add-exports and patch module directly. all the affected classes and obviously updated to use executeTestJvm instead of executeTestJvmAll.
>>
>> after this fix many hotspot tests won't depend on java.management, hence that module might be removed from their @modules, this will be done separately by 8178416.
>>
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8164944
>> webrev: http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
>> testing: :hotspot_all
>> 8178416: https://bugs.openjdk.java.net/browse/JDK-8178416
>>
>> Thanks,
>> -- Igor
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S): 8164944: Refactor ProcessTools to get rid of dependency on java.management

serguei.spitsyn@oracle.com
In reply to this post by Igor Ignatyev
Igor,

Looks good.

Thanks,
Serguei


On 4/12/17 12:14, Igor Ignatyev wrote:

> forgot to update copyrights, http://cr.openjdk.java.net/~iignatyev//8164944/webrev.01/index.html <http://cr.openjdk.java.net/~iignatyev//8164944/webrev.01/index.html>
>
>> On Apr 12, 2017, at 12:03 PM, Igor Ignatyev <[hidden email]> wrote:
>>
>> http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
>>> 92 lines changed: 36 ins; 36 del; 20 mod;
>> Hi all,
>>
>> could you please review this small fix which removes dependency on java.management module from the testlibrary?
>>
>> jdk.test.lib.process.ProcessTools class has getVmInputArgs method which depend on java.management, this method is used only by j.d.t.process.ProcessTools::executeTestJvmAll and j.t.l.cli.CommandLineOptionTest class.
>> - ProcessTools::executeTestJvmAll is a variation of executeTestJvm which prepends vm flags from jtreg @run.
>> - CommandLineOptionTest is a library class for cli testing, it uses flags from ProcessTools::getVmInputArgs to create a new jvm with all flags
>>
>> the fix removes ProcessTools::executeTestJvmAll method, moves getVmInputArgs to a separate class jdk.test.lib.management.InputArguments. c.c.s.scenario.Executor and j.t.l.cli.CommandLineOptionTest are updated to use getVmInputArgs from that class. c.j.compilerToVM.DebugOutputTest is refactored to pass add-exports and patch module directly. all the affected classes and obviously updated to use executeTestJvm instead of executeTestJvmAll.
>>
>> after this fix many hotspot tests won't depend on java.management, hence that module might be removed from their @modules, this will be done separately by 8178416.
>>
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8164944
>> webrev: http://cr.openjdk.java.net/~iignatyev//8164944/webrev.00/index.html
>> testing: :hotspot_all
>> 8178416: https://bugs.openjdk.java.net/browse/JDK-8178416
>>
>> Thanks,
>> -- Igor

Loading...