Re: RFR(S): 8197906: Enable CDS mode execution of jtreg tests via make
Hi Igor, Thank you very much for catching this, and for the explanation. I have updated the patch and running tests now. I assume the recommended update is small/trivial enough that it does not need another webrev; let me know otherwise. Thank you, Misha On 2/14/18, 9:00 AM, Igor Ignatyev wrote: [1] and [2] are equivalent for cygwin, but it doesn't mean they are equivalent JVM. I recall there were some problems when cygwin paths passed to JVM. so to be on the safe side, I'd use mixed or windows path to the archive in JVM flags. Thanks, -- Igor On Feb 14, 2018, at 7:26 AM, Mikhailo Seledtsov wrote: Hi Igor, Thank you for review. This is what I see from the test execution logs on Windows: // At archive creation time: -XX:SharedArchiveFile=T:/testoutput/jtreg/cds_archive.jsa -Xshare:dump // At archive use time: -vmoption:-XX:+UnlockDiagnosticVMOptions -vmoption:-XX:SharedArchiveFile=/cygdrive/t/testoutput/jtreg/cds_archive.jsa -javaoptions:-Xshare:auto -javaoptions:-Xlog:class+path=trace -javaoptions:-showversion I am not a Windows expert, and especially not a wiz in Cygwin. Do you know if these 2 paths are equivalent? [1] T:/testoutput/jtreg/cds_archive.jsa [2] /cygdrive/t/testoutput/jtreg/cds_archive.jsa If these are equivalent, then the code is OK as is. If not, I will change it and rerun the tests. Please let me know. Thank you, Misha On 2/13/18, 8:41 PM, Igor Ignatyev wrote: Hi Misha, have you check that it works on windows w/ GENERATE_CDS_ARCHIVE=true? it seems that you should called GETMIXEDPATH in CDS_VM_ARGS: CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=$(CDS_ARCHIVE_FILE) vs CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=$(shell $(GETMIXEDPATH) "$(CDS_ARCHIVE_FILE)") otherwise, tests won't be able to find cds archive during test execution. -- Igor On Feb 13, 2018, at 8:17 PM, Mikhailo Seledtsov wrote: Please review this small change that enables execution of any jtreg test(s) in CDS mode via make. Please see bug description for details. Thanks to Erik and Igor for their help with this change. JBS: https://bugs.openjdk.java.net/browse/JDK-8197906 WebRev: http://cr.openjdk.java.net/~mseledtsov/8197906.01/ Testing: - ran hotspot_runtime in CDS mode with this change - Linux-x64 - no new failures - running several test sets in CDS mode via distributed test system - pass - running pre-integration sanity testing (HS tier1, tier2) - in progress Thank you, Misha
Re: RFR(S): 8197906: Enable CDS mode execution of jtreg tests via make
Hi Magnus, Thank you for review and feedback. I will update my change based on your recommendation, the retest. If tests pass, I am planning to push. Thank you, Misha On 2/14/18, 12:04 PM, Magnus Ihse Bursie wrote: On 2018-02-14 16:26, Mikhailo Seledtsov wrote: Hi Igor, Thank you for review. This is what I see from the test execution logs on Windows: // At archive creation time: > -XX:SharedArchiveFile=T:/testoutput/jtreg/cds_archive.jsa -Xshare:dump // At archive use time: >-vmoption:-XX:+UnlockDiagnosticVMOptions -vmoption:-XX:SharedArchiveFile=/cygdrive/t/testoutput/jtreg/cds_archive.jsa -javaoptions:-Xshare:auto -javaoptions:-Xlog:class+path=trace -javaoptions:-showversion I am not a Windows expert, and especially not a wiz in Cygwin. Do you know if these 2 paths are equivalent? That's likely incorrect. Java will not be able to correctly interpred /cygdrive/t. I believe the fix should be to do: CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=$(shell $(GETMIXEDPATH) "$(CDS_ARCHIVE_FILE)") instead. Otherwise the patch looks OK to me. /Magnus [1] T:/testoutput/jtreg/cds_archive.jsa [2] /cygdrive/t/testoutput/jtreg/cds_archive.jsa If these are equivalent, then the code is OK as is. If not, I will change it and rerun the tests. Please let me know. Thank you, Misha On 2/13/18, 8:41 PM, Igor Ignatyev wrote: Hi Misha, have you check that it works on windows w/ GENERATE_CDS_ARCHIVE=true? it seems that you should called GETMIXEDPATH in CDS_VM_ARGS: CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=$(CDS_ARCHIVE_FILE) vs CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=$(shell $(GETMIXEDPATH) "$(CDS_ARCHIVE_FILE)") otherwise, tests won't be able to find cds archive during test execution. -- Igor On Feb 13, 2018, at 8:17 PM, Mikhailo Seledtsov wrote: Please review this small change that enables execution of any jtreg test(s) in CDS mode via make. Please see bug description for details. Thanks to Erik and Igor for their help with this change. JBS: https://bugs.openjdk.java.net/browse/JDK-8197906 WebRev: http://cr.openjdk.java.net/~mseledtsov/8197906.01/ Testing: - ran hotspot_runtime in CDS mode with this change - Linux-x64 - no new failures - running several test sets in CDS mode via distributed test system - pass - running pre-integration sanity testing (HS tier1, tier2) - in progress Thank you, Misha
Re: RFR(S): 8197906: Enable CDS mode execution of jtreg tests via make
On 2018-02-14 16:26, Mikhailo Seledtsov wrote: Hi Igor, Thank you for review. This is what I see from the test execution logs on Windows: // At archive creation time: > -XX:SharedArchiveFile=T:/testoutput/jtreg/cds_archive.jsa -Xshare:dump // At archive use time: > -vmoption:-XX:+UnlockDiagnosticVMOptions -vmoption:-XX:SharedArchiveFile=/cygdrive/t/testoutput/jtreg/cds_archive.jsa -javaoptions:-Xshare:auto -javaoptions:-Xlog:class+path=trace -javaoptions:-showversion I am not a Windows expert, and especially not a wiz in Cygwin. Do you know if these 2 paths are equivalent? That's likely incorrect. Java will not be able to correctly interpred /cygdrive/t. I believe the fix should be to do: CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=$(shell $(GETMIXEDPATH) "$(CDS_ARCHIVE_FILE)") instead. Otherwise the patch looks OK to me. /Magnus [1] T:/testoutput/jtreg/cds_archive.jsa [2] /cygdrive/t/testoutput/jtreg/cds_archive.jsa If these are equivalent, then the code is OK as is. If not, I will change it and rerun the tests. Please let me know. Thank you, Misha On 2/13/18, 8:41 PM, Igor Ignatyev wrote: Hi Misha, have you check that it works on windows w/ GENERATE_CDS_ARCHIVE=true? it seems that you should called GETMIXEDPATH in CDS_VM_ARGS: CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=$(CDS_ARCHIVE_FILE) vs CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=$(shell $(GETMIXEDPATH) "$(CDS_ARCHIVE_FILE)") otherwise, tests won't be able to find cds archive during test execution. -- Igor On Feb 13, 2018, at 8:17 PM, Mikhailo Seledtsov wrote: Please review this small change that enables execution of any jtreg test(s) in CDS mode via make. Please see bug description for details. Thanks to Erik and Igor for their help with this change. JBS: https://bugs.openjdk.java.net/browse/JDK-8197906 WebRev: http://cr.openjdk.java.net/~mseledtsov/8197906.01/ Testing: - ran hotspot_runtime in CDS mode with this change - Linux-x64 - no new failures - running several test sets in CDS mode via distributed test system - pass - running pre-integration sanity testing (HS tier1, tier2) - in progress Thank you, Misha
Re: RFR(S): 8197906: Enable CDS mode execution of jtreg tests via make
[1] and [2] are equivalent for cygwin, but it doesn't mean they are equivalent JVM. I recall there were some problems when cygwin paths passed to JVM. so to be on the safe side, I'd use mixed or windows path to the archive in JVM flags. Thanks, -- Igor > On Feb 14, 2018, at 7:26 AM, Mikhailo Seledtsov > wrote: > > Hi Igor, > > Thank you for review. This is what I see from the test execution logs on > Windows: > > // At archive creation time: > > -XX:SharedArchiveFile=T:/testoutput/jtreg/cds_archive.jsa -Xshare:dump > > > // At archive use time: > > >-vmoption:-XX:+UnlockDiagnosticVMOptions > > -vmoption:-XX:SharedArchiveFile=/cygdrive/t/testoutput/jtreg/cds_archive.jsa > > -javaoptions:-Xshare:auto -javaoptions:-Xlog:class+path=trace > > -javaoptions:-showversion > > > I am not a Windows expert, and especially not a wiz in Cygwin. Do you know if > these 2 paths are equivalent? > [1] T:/testoutput/jtreg/cds_archive.jsa > [2] /cygdrive/t/testoutput/jtreg/cds_archive.jsa > > If these are equivalent, then the code is OK as is. If not, I will change it > and rerun the tests. Please let me know. > > > Thank you, > Misha > > > On 2/13/18, 8:41 PM, Igor Ignatyev wrote: >> Hi Misha, >> >> have you check that it works on windows w/ GENERATE_CDS_ARCHIVE=true? it >> seems that you should called GETMIXEDPATH in CDS_VM_ARGS: >>> CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions >>> -XX:SharedArchiveFile=$(CDS_ARCHIVE_FILE) >> vs >>> CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=$(shell >>> $(GETMIXEDPATH) "$(CDS_ARCHIVE_FILE)") >> otherwise, tests won't be able to find cds archive during test execution. >> >> -- Igor >> >>> On Feb 13, 2018, at 8:17 PM, Mikhailo >>> Seledtsov wrote: >>> >>> Please review this small change that enables execution of any jtreg test(s) >>> in CDS mode >>> via make. Please see bug description for details. >>> >>> Thanks to Erik and Igor for their help with this change. >>> >>>JBS: https://bugs.openjdk.java.net/browse/JDK-8197906 >>>WebRev: http://cr.openjdk.java.net/~mseledtsov/8197906.01/ >>> >>>Testing: >>> - ran hotspot_runtime in CDS mode with this change - Linux-x64 - no >>> new failures >>> - running several test sets in CDS mode via distributed test system - >>> pass >>> - running pre-integration sanity testing (HS tier1, tier2) - in >>> progress >>> >>> Thank you, >>> Misha >>>
Re: RFR(S): 8197906: Enable CDS mode execution of jtreg tests via make
Hi Igor, Thank you for review. This is what I see from the test execution logs on Windows: // At archive creation time: > -XX:SharedArchiveFile=T:/testoutput/jtreg/cds_archive.jsa -Xshare:dump // At archive use time: >-vmoption:-XX:+UnlockDiagnosticVMOptions -vmoption:-XX:SharedArchiveFile=/cygdrive/t/testoutput/jtreg/cds_archive.jsa -javaoptions:-Xshare:auto -javaoptions:-Xlog:class+path=trace -javaoptions:-showversion I am not a Windows expert, and especially not a wiz in Cygwin. Do you know if these 2 paths are equivalent? [1] T:/testoutput/jtreg/cds_archive.jsa [2] /cygdrive/t/testoutput/jtreg/cds_archive.jsa If these are equivalent, then the code is OK as is. If not, I will change it and rerun the tests. Please let me know. Thank you, Misha On 2/13/18, 8:41 PM, Igor Ignatyev wrote: Hi Misha, have you check that it works on windows w/ GENERATE_CDS_ARCHIVE=true? it seems that you should called GETMIXEDPATH in CDS_VM_ARGS: CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=$(CDS_ARCHIVE_FILE) vs CDS_VM_ARGS := -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=$(shell $(GETMIXEDPATH) "$(CDS_ARCHIVE_FILE)") otherwise, tests won't be able to find cds archive during test execution. -- Igor On Feb 13, 2018, at 8:17 PM, Mikhailo Seledtsov wrote: Please review this small change that enables execution of any jtreg test(s) in CDS mode via make. Please see bug description for details. Thanks to Erik and Igor for their help with this change. JBS: https://bugs.openjdk.java.net/browse/JDK-8197906 WebRev: http://cr.openjdk.java.net/~mseledtsov/8197906.01/ Testing: - ran hotspot_runtime in CDS mode with this change - Linux-x64 - no new failures - running several test sets in CDS mode via distributed test system - pass - running pre-integration sanity testing (HS tier1, tier2) - in progress Thank you, Misha
Re: RFR(S): 8197906: Enable CDS mode execution of jtreg tests via make
cc'ing build-dev as this is a shared makefile change Thanks, David On 14/02/2018 2:17 PM, Mikhailo Seledtsov wrote: Please review this small change that enables execution of any jtreg test(s) in CDS mode via make. Please see bug description for details. Thanks to Erik and Igor for their help with this change. JBS: https://bugs.openjdk.java.net/browse/JDK-8197906 WebRev: http://cr.openjdk.java.net/~mseledtsov/8197906.01/ Testing: - ran hotspot_runtime in CDS mode with this change - Linux-x64 - no new failures - running several test sets in CDS mode via distributed test system - pass - running pre-integration sanity testing (HS tier1, tier2) - in progress Thank you, Misha
