Re: RFR: JDK-8218177: Bump jib format_version to support new devkit archive layout

2019-01-31 Thread Christian Tornqvist
Looks good.

Thanks,
Christian

> On Jan 31, 2019, at 4:23 PM, Erik Joelsson  wrote:
> 
> With the new devkits I deployed recently (Oracle internal), the file layout 
> changed as a top level directory inside the tar archive was introduced. This 
> works in most cases, but there are certain situations where jib does not 
> handle this well. This causes AOT testing on Linux to stop working with the 
> run-test-prebuilt make target.
> 
> This issue is being fixed in Jib, but to activate the fix a bump in the 
> format_version is required. This patch does that, as well as adjusting to 
> conform to the new behavior.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8218177
> 
> Webrev: http://cr.openjdk.java.net/~erikj/8218177/webrev.01/
> 
> (note that the Jib fix is not yet live in production)
> 
> /Erik
> 



Re: (urgent) RFR: JDK-8218084: Revert JDK-8218057

2019-01-30 Thread Christian Tornqvist
Looks good, thanks for dealing with this so quickly

Thanks,
Christian

> On Jan 30, 2019, at 12:47 PM, Erik Joelsson  wrote:
> 
> The addition of new build profiles in jib-profiles.js caused our internal 
> build and test system to break down. While we fix the resilience of that 
> system, we need to back this change out. This patch is the result of:
> 
> $ hg backout 8830bb9587c2
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8218084
> 
> Webrev: http://cr.openjdk.java.net/~erikj/8218084/webrev.01/
> 
> /Erik
> 



Re: RFR: JDK-8216021: RunTest.gmk might set concurrency level to 1 on Windows

2019-01-03 Thread Christian Tornqvist
Looks good, thanks for fixing this!

Thanks,
Christian

> On Jan 3, 2019, at 2:30 AM, Erik Joelsson  wrote:
> 
> When running tests using "make run-test-prebuilt" on Windows, the calculation 
> of available memory may fail. This only happens on certain systems, and is 
> likely related to which version of Cygwin that is being used. The consequence 
> is that concurrency gets set to 1. The cause of the problem is mishandling of 
> Windows \r line endings, which Cygwin tools are known to handle differently 
> in different versions.
> 
> To fix the problem here, I've added explicit filtering of \r before trying to 
> interpret the memory size as a number.
> 
> I checked the corresponding construct in configure, and it seems to work on 
> the same machine where RunTestsPrebuilt.gmk failed, so I left it alone.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8216021
> 
> Webrev: http://cr.openjdk.java.net/~erikj/8216021/webrev.01/
> 
> /Erik
> 



Re: RFR: JDK-8215991: Stop hiding exception from ArtifactResolver failures in tests

2019-01-02 Thread Christian Tornqvist
Looks good.

Thanks,
Christian

> On Jan 2, 2019, at 5:46 AM, Erik Joelsson  wrote:
> 
> In the test lib, we have an ArtifactResolver which is used by tests that need 
> external resources. Internally at Oracle we hook Jib into this to provide 
> such dependencies. For other users, it's possible to specify the path to such 
> a dependency using a system property.
> 
> If something goes wrong while resolving the dependencies, the 
> ArtifactResolver currently swallows the exception and falls back on the 
> default property based resolver, making it hard to figure out the problem.
> 
> If the user provided the environment for calling Jib, I don't think we should 
> try to revert to the default resolver on exception, but instead the exception 
> should be propagated to the test.
> 
> Webrev: http://cr.openjdk.java.net/~erikj/8215991/webrev.01/
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8215991
> 
> /Erik
> 



Re: RFR: JDK-8211727: Adjust default concurrency settings for running tests on Sparc

2018-11-13 Thread Christian Tornqvist
Hi Erik,

Looks good, thanks for fixing this.

Thanks,
Christian

> On Nov 13, 2018, at 1:34 PM, Erik Joelsson  wrote:
> 
> This patch changes the formula for default test concurrency in RunTest.gmk. 
> The current formula is:
> 
> min(cpus/2, 12)
> 
> This seems to work well enough on the x64 machines we currently run our tests 
> on, but less so for Sparc. I have now run rather extensive testing in our lab 
> and have come up with a new formula that provides much better test 
> reliability while preserving as much test throughput as possible. The new 
> formula is cpus/4 for sparcs with up to 16 cpus and cpus/5 for larger 
> machines. For non Sparc it's still cpus/2 and I've removed the cap for all.
> 
> In addition to this, since Sparc generally have lower per thread performance, 
> at least when running JDK tests, I have bumped the default timeout factor 
> from 4 to 8 for Sparc.
> 
> With these defaults, we were able to remove a lot of special cases for Sparc 
> in other parts of our configurations and I was able to get clean runs of all 
> the lower tiers of testing, on each of our machine classes in the lab.
> 
> In addition to this, the test 
> compiler/jsr292/ContinuousCallSiteTargetChange.java, which had its timeout 
> increased in JDK-8212028, no longer needs an increased timeout with the new 
> defaults.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8211727
> 
> Webrev: http://cr.openjdk.java.net/~erikj/8211727/webrev.01/
> 
> /Erik
> 



Re: RFR: JDK-8213005: Missing symbols in hs_err files on Windows after JDK-8212028

2018-10-25 Thread Christian Tornqvist
Hi Erik,

This looks good, thanks for fixing this!

Thanks,
Christian

> On Oct 25, 2018, at 4:27 PM, Erik Joelsson  wrote:
> 
> When running tests through the new RunTest makefile framework in Mach5, we 
> are now missing symbol information in hs_err files on Windows. This is caused 
> by the new run-test-prebuilt target not properly setting up _NT_SYMBOL_PATH 
> which in turn is caused by the variable CYGPATH not being set.
> 
> This patch sets CYGPATH in RunTestPrebuiltSpec.gmk. I also modified some 
> cosmetics in the _NT_SYMBOLS_PATH setup to make it easier to understand, at 
> least for me.
> 
> Verfied by running "run-test-prebuilt" with a fastdebug build and setting 
> JTREG=JAVA_OPTIONS=-XX:ErrorHandlerTest=14. With the fix, the generated 
> hs_err*.log contains symbols in the stacktrace.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8213005
> 
> Webrev: 
> http://cr.openjdk.java.net/~erikj/8213005/webrev.01/make/RunTests.gmk.sdiff.html
> 
> /Erik
> 



RFR: 8212008: Use of TREAT_EXIT_CODE_1_AS_0 hide problems with jtreg Java

2018-10-10 Thread Christian Tornqvist
Hi,

When running JDK and Hotspot jtreg tests through make, TREAT_EXIT_CODE_1_AS_0 
is set to true. Any issue with the Java installation used to run jtreg won’t be 
reflected in the exit code. This has been seen a bunch of times in our internal 
CI system where we thought we ran tests but instead no tests were run at all 
because the jtreg Java installation had been deleted/corrupted. 

Webrev: http://cr.openjdk.java.net/~ctornqvi/webrev/8212008/webrev.00/ 

Bug: https://bugs.openjdk.java.net/browse/JDK-8212008 


Thanks,
Christian

Re: RFR(S/M) [trivial] 8206429 : [REDO] 8202561 clean up TEST.groups

2018-07-05 Thread Christian Tornqvist
I agree with Igor, this wouldn’t have been found by “normal” pre-integration 
testing. Change looks good, sorry for the extra work you had to do.

Thanks,
Christian

> On Jul 5, 2018, at 5:33 PM, Igor Ignatyev  wrote:
> 
> well, I did test it last time, I just didn't expect that some parts of infra 
> do submission a bit weirdly. now (at least as far as I can tell) I tested 
> submission in the exact same way.
> 
> -- Igor
> 
>> On Jul 5, 2018, at 2:18 PM, Vladimir Kozlov  
>> wrote:
>> 
>> I am fine with re-do if you were able re-test it to verify that it actually 
>> works. Not like last time.
>> 
>> Thanks,
>> Vladimir
>> 
>>> On 7/5/18 1:02 PM, Igor Ignatyev wrote:
>>> http://cr.openjdk.java.net/~iignatyev//8206429/webrev.00/index.html
 243 lines changed: 0 ins; 124 del; 119 mod;
>>> Hi all,
>>> could you please review this redo of 8202561? 8202561 was backed out b/c 
>>> our CI infra didn't support multiple test group files. now such 
>>> configurations are supported, so 8202561 can be re-integrated.
>>> the patch is the exact 8202561 changeset and it has applied clearly.
>>> webrev: http://cr.openjdk.java.net/~iignatyev//8206429/webrev.00/index.html
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8206429
>>> testing: successfully submitted adhoc jobs which use a "quick" test group 
>>> in the same way CI system does
>>> Thanks,
>>> -- Igor
> 



Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations

2018-04-12 Thread Christian Tornqvist


> On Apr 12, 2018, at 11:54 21AM, Iris Clark  wrote:
> 
> Hi.
> 
> I believe that the internal page Christian references is for the test system.
> 
> If you want to know whether the push arrived in the repository, you could 
> subscribe to jdk-submit-chan...@openjdk.java.net.  The archive of recent push 
> notifications is public:
> 
>
> http://mail.openjdk.java.net/pipermail/jdk-submit-changes/2018-April/thread.html
> 
> I wonder if the test system could be enhanced to send a brief notification 
> when a job is queued?

I’ve opened an enhancement for adding a notification once the job has been 
handed off to our build and test farm.

Thanks,
Christian
> 
> Thanks,
> Iris
> 
> -Original Message-
> From: Christian Tornqvist 
> Sent: Thursday, April 12, 2018 6:12 AM
> To: Baesken, Matthias 
> Cc: core-libs-...@openjdk.java.net; Alexey Ivanov ; 
> Doerr, Martin ; build-dev 
> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in function 
> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some 
> places in function declarations/implementations
> 
> 
> 
>> On Apr 12, 2018, at 9:07 48AM, Baesken, Matthias  
>> wrote:
>> 
>>> Your submit job ran without failures, we were doing maintenance on 
>>> the jdk- submit repo yesterday and had turned off notifications. 
>>> Sorry for the inconvenience.
>> 
>> Hi Christian , Thanks for  the information about the submit job success.
>> 
>> Is there a way  to check (e.g. webpage)  that a submit job has "arrived"  
>> and is queued for   build/test  ?
> 
> Unfortunately that webpage is only available internally at this point, we 
> could look into sending an email notification that the job has been started 
> if that would help?
> 
> Thanks,
> Christian
> 
>> Would have been helpful  in this situation .
>> 
>> Best regards, Matthias
>> 
>> 
>>> -Original Message-
>>> From: Christian Tornqvist [mailto:christian.tornqv...@oracle.com]
>>> Sent: Donnerstag, 12. April 2018 14:58
>>> To: Baesken, Matthias 
>>> Cc: Alexey Ivanov ; Magnus Ihse Bursie 
>>> ; build-dev >> d...@openjdk.java.net>; Doerr, Martin 
>>> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in 
>>> function declarations/implementations - was : RE: missing JNIEXPORT / 
>>> JNICALL at some places in function declarations/implementations
>>> 
>>> Hi Matthias,
>>> 
>>> 
>>>> On Apr 12, 2018, at 3:49 35AM, Baesken, Matthias
>>>  wrote:
>>>> 
>>>> Hi,  could  someone please  sponsor  the change  now ?
>>>> 
>>>> And  could someone please check  what happened  to the submit-repo ?
>>>> Yesterday I pushed to  the submit repo  to   check my  change  ,  but  no
>>> response   so far .
>>>> Maybe  the submit repo  is not working currently  ,  not sure  about it .
>>> 
>>> Your submit job ran without failures, we were doing maintenance on 
>>> the jdk- submit repo yesterday and had turned off notifications. 
>>> Sorry for the inconvenience.
>>> 
>>> Thanks,
>>> Christian
>>>> 
>>>> 
>>>> Best regards , Matthias
>>>> 
>>>> 
>>>> 
>>>> 
>>>>> -Original Message-
>>>>> From: Baesken, Matthias
>>>>> Sent: Mittwoch, 11. April 2018 11:20
>>>>> To: 'Alexey Ivanov' ; Magnus Ihse Bursie 
>>>>> 
>>>>> Cc: build-dev ; Doerr, Martin 
>>>>> 
>>>>> Subject: RE: 8201226 missing JNIEXPORT / JNICALL at some places in
>>> function
>>>>> declarations/implementations - was : RE: missing JNIEXPORT / 
>>>>> JNICALL at some places in function declarations/implementations
>>>>> 
>>>>>> 
>>>>>> Was main() exported via map files?
>>>>>> 
>>>>> 
>>>>> Seems main was exported , I can find it in jdk10  in  e.g.  :
>>>>> 
>>>>> make/mapfiles/launchers/mapfile-sparcv9
>>>>> make/mapfiles/launchers/mapfile-x86_64
>>>>> 
>>>>> 
>>>>> Best regards, Matthias
>>>>> 
>>>>> 
>>>>>> -Original Message-
>>>>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com]
>>>>>> Sent: Mittwoch, 11. April 2018 11:11
>>>>>> To: Baesken, Matth

Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations

2018-04-12 Thread Christian Tornqvist


> On Apr 12, 2018, at 9:07 48AM, Baesken, Matthias  
> wrote:
> 
>> Your submit job ran without failures, we were doing maintenance on the jdk-
>> submit repo yesterday and had turned off notifications. Sorry for the
>> inconvenience.
> 
> Hi Christian , Thanks for  the information about the submit job success.
> 
> Is there a way  to check (e.g. webpage)  that a submit job has "arrived"  and 
> is queued for   build/test  ?

Unfortunately that webpage is only available internally at this point, we could 
look into sending an email notification that the job has been started if that 
would help?

Thanks,
Christian

> Would have been helpful  in this situation .
> 
> Best regards, Matthias
> 
> 
>> -Original Message-
>> From: Christian Tornqvist [mailto:christian.tornqv...@oracle.com]
>> Sent: Donnerstag, 12. April 2018 14:58
>> To: Baesken, Matthias 
>> Cc: Alexey Ivanov ; Magnus Ihse Bursie
>> ; build-dev > d...@openjdk.java.net>; Doerr, Martin 
>> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in function
>> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at
>> some places in function declarations/implementations
>> 
>> Hi Matthias,
>> 
>> 
>>> On Apr 12, 2018, at 3:49 35AM, Baesken, Matthias
>>  wrote:
>>> 
>>> Hi,  could  someone please  sponsor  the change  now ?
>>> 
>>> And  could someone please check  what happened  to the submit-repo ?
>>> Yesterday I pushed to  the submit repo  to   check my  change  ,  but  no
>> response   so far .
>>> Maybe  the submit repo  is not working currently  ,  not sure  about it .
>> 
>> Your submit job ran without failures, we were doing maintenance on the jdk-
>> submit repo yesterday and had turned off notifications. Sorry for the
>> inconvenience.
>> 
>> Thanks,
>> Christian
>>> 
>>> 
>>> Best regards , Matthias
>>> 
>>> 
>>> 
>>> 
>>>> -Original Message-
>>>> From: Baesken, Matthias
>>>> Sent: Mittwoch, 11. April 2018 11:20
>>>> To: 'Alexey Ivanov' ; Magnus Ihse Bursie
>>>> 
>>>> Cc: build-dev ; Doerr, Martin
>>>> 
>>>> Subject: RE: 8201226 missing JNIEXPORT / JNICALL at some places in
>> function
>>>> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at
>>>> some places in function declarations/implementations
>>>> 
>>>>> 
>>>>> Was main() exported via map files?
>>>>> 
>>>> 
>>>> Seems main was exported , I can find it in jdk10  in  e.g.  :
>>>> 
>>>> make/mapfiles/launchers/mapfile-sparcv9
>>>> make/mapfiles/launchers/mapfile-x86_64
>>>> 
>>>> 
>>>> Best regards, Matthias
>>>> 
>>>> 
>>>>> -Original Message-
>>>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com]
>>>>> Sent: Mittwoch, 11. April 2018 11:11
>>>>> To: Baesken, Matthias ; Magnus Ihse
>> Bursie
>>>>> 
>>>>> Cc: build-dev ; Doerr, Martin
>>>>> 
>>>>> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in
>>>> function
>>>>> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at
>>>>> some places in function declarations/implementations
>>>>> 
>>>>> 
>>>>> On 11/04/2018 08:44, Baesken, Matthias wrote:
>>>>>>> JIMAGE_FindResource doesn't have JNICALL modifier now, does it?
>>>>>> Hi  Alexey, yes that's true .
>>>>>> 
>>>>>>> Please remove JNIEXPORT from main():
>>>>>>> src/java.base/share/native/launcher/main.c
>>>>>>> src/jdk.pack/share/native/unpack200/main.cpp
>>>>>> I would  prefer to keep it for now .
>>>>>> I notice  some  comments  in our SAPJVM code base  about needing
>>>>> JNIEXPORT for  main  for Solaris  (we were running  in SAPJVM without
>>>>> mapfiles in the past already).
>>>>>> Maybe  that’s related to
>>>>>> 
>>>>>> src/java.base/unix/native/libjli/java_md_solinux.c
>>>>>> 
>>>>>> where main  is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT,
>> "main");
>>>>>> but I am not sure about this.
>>>>

Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations

2018-04-12 Thread Christian Tornqvist
Hi Matthias,


> On Apr 12, 2018, at 3:49 35AM, Baesken, Matthias  
> wrote:
> 
> Hi,  could  someone please  sponsor  the change  now ?
> 
> And  could someone please check  what happened  to the submit-repo ?
> Yesterday I pushed to  the submit repo  to   check my  change  ,  but  no  
> response   so far .
> Maybe  the submit repo  is not working currently  ,  not sure  about it .

Your submit job ran without failures, we were doing maintenance on the 
jdk-submit repo yesterday and had turned off notifications. Sorry for the 
inconvenience.

Thanks,
Christian
> 
> 
> Best regards , Matthias
> 
> 
> 
> 
>> -Original Message-
>> From: Baesken, Matthias
>> Sent: Mittwoch, 11. April 2018 11:20
>> To: 'Alexey Ivanov' ; Magnus Ihse Bursie
>> 
>> Cc: build-dev ; Doerr, Martin
>> 
>> Subject: RE: 8201226 missing JNIEXPORT / JNICALL at some places in function
>> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at
>> some places in function declarations/implementations
>> 
>>> 
>>> Was main() exported via map files?
>>> 
>> 
>> Seems main was exported , I can find it in jdk10  in  e.g.  :
>> 
>> make/mapfiles/launchers/mapfile-sparcv9
>> make/mapfiles/launchers/mapfile-x86_64
>> 
>> 
>> Best regards, Matthias
>> 
>> 
>>> -Original Message-
>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com]
>>> Sent: Mittwoch, 11. April 2018 11:11
>>> To: Baesken, Matthias ; Magnus Ihse Bursie
>>> 
>>> Cc: build-dev ; Doerr, Martin
>>> 
>>> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in
>> function
>>> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at
>>> some places in function declarations/implementations
>>> 
>>> 
>>> On 11/04/2018 08:44, Baesken, Matthias wrote:
> JIMAGE_FindResource doesn't have JNICALL modifier now, does it?
 Hi  Alexey, yes that's true .
 
> Please remove JNIEXPORT from main():
> src/java.base/share/native/launcher/main.c
> src/jdk.pack/share/native/unpack200/main.cpp
 I would  prefer to keep it for now .
 I notice  some  comments  in our SAPJVM code base  about needing
>>> JNIEXPORT for  main  for Solaris  (we were running  in SAPJVM without
>>> mapfiles in the past already).
 Maybe  that’s related to
 
 src/java.base/unix/native/libjli/java_md_solinux.c
 
 where main  is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main");
 but I am not sure about this.
 So I better keep  the JNIEXPORT  for the main functions,   could be
>>> removed in another  cleanup  if really needed.
>>> 
>>> OK. Let them stay then.
>>> Was main() exported via map files?
>>> 
>>> 
>>> The change looks good to me.
>>> 
>>> Regards,
>>> Alexey
>>> 
 
> You can reference both yourself and me as
> Contributed-by: mbaesken, aivanov
> when pushing the changeset if you don't mind.
> 
 Sure .
 
 Best regards, Matthias
 
 
> -Original Message-
> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com]
> Sent: Dienstag, 10. April 2018 21:34
> To: Baesken, Matthias ; Magnus Ihse
>>> Bursie
> 
> Cc: build-dev ; Doerr, Martin
> 
> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in
>>> function
> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL
>> at
> some places in function declarations/implementations
> 
> Hi Matthias,
> 
> On 10/04/2018 11:14, Baesken, Matthias wrote:
>> Hello,  I  had to  do another small adjustment to make jimage.hpp/cpp
>>> match. Please review :
>> 
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/
> JIMAGE_FindResource doesn't have JNICALL modifier now, does it?
> 
> I've successfully built 32 bit Windows with your patch.
> 
> 
> Please remove JNIEXPORT from main():
> src/java.base/share/native/launcher/main.c
> src/jdk.pack/share/native/unpack200/main.cpp
> 
>> With the latest webrev I could finally build jdk/jdk successfully on both
>>> win32bit and win64 bit.
>> 
>> Thanks again  to Alexey  to provide  the   incorporated patch .
> You can reference both yourself and me as
> Contributed-by: mbaesken, aivanov
> when pushing the changeset if you don't mind.
> 
> 
> Regards,
> Alexey
> 
>> 
>> Best regards, Matthias
>> 
>> 
>> 
>>> -Original Message-
>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com]
>>> Sent: Montag, 9. April 2018 17:14
>>> To: Baesken, Matthias ; Magnus Ihse
> Bursie
>>> 
>>> Cc: build-dev ; Doerr, Martin
>>> 
>>> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in
> function
>>> declarations/implementations - was : RE: missing JNIEXPORT /
>> JNICALL
>>> at
>>> some places in function declarations/implementations
>>> 
>>> Hi Matthias,
>>> 
>>> On 09/04/2018 15:38, Baesken, Matthias wrote:
 Hi  Alexey,

Re: RFR(M): 8200126: [TESTBUG] Open source VM runtime signal tests

2018-04-02 Thread Christian Tornqvist
Hi Misha,

This looks good, thanks for doing this.

Thanks,
Christia

> On Mar 29, 2018, at 10:41 04PM, Mikhailo Seledtsov 
>  wrote:
> 
> While testing I discovered build errors on Mac and Solaris. The following 
> statement " BUILD_HOTSPOT_JTREG_EXECUTABLES_LIBS_exesigtest := -ljvm" was 
> added to a Linux-only block. I have updated the make file to add this for any 
> platform w/o conditions; the " exesigtest.c" is excluded from Windows anyway 
> down below in the make file. I am not 100% sure this is the correct way to 
> modify the make file; if not please advise the correct way.
> With this fix all 4 builds pass.
> 
> Here is the updated webrev:
>http://cr.openjdk.java.net/~mseledtsov/8200126.02.open/index.html
> 
> Thank you,
> Misha
> 
> 
> On 3/29/18, 4:00 PM, mikhailo wrote:
>> I have addressed feedback from Christian, David and Magnus. Here is the 
>> updated webrev:
>> 
>> http://cr.openjdk.java.net/~mseledtsov/8200126.01.open/index.html
>> 
>> 
>> I have also confirmed that output from exesigtest.c printf() is logged into 
>> .jtr files.
>> 
>>  Grepped for "signal", I can see the output such as:
>>  TestSigxfsz.jtr:SIGXFSZ: signal handler using function 'sigset' has 
>> been set
>>  TestSigxfsz.jtr:SIGXFSZ: signal handler for signal 25 has been 
>> processed
>>  TestSigxfsz.jtr:SIGXFSZ: signal has been sent successfully
>>  TestSigxfsz.jtr:SIGXFSZ: signal has been received
>>  Also can see other output from the printf, such as all initVM logs.
>> 
>> 
>> Thank you,
>> Misha
>> 
>> On 03/29/2018 02:49 PM, mikhailo wrote:
>>> Magnus,
>>> 
>>> Thank you for advice. I have updated the makefile accordingly. Will post 
>>> updated webrev shortly.
>>> 
>>> 
>>> Misha
>>> 
>>> 
>>> On 03/28/2018 03:26 PM, Magnus Ihse Bursie wrote:
 Yes, you seem to have based this off an old version of 
 JtregNativeHotspot.gmk.
 
 If you update the file I think you see how you should do it, but I'll give 
 you some help:
 
 ifeq ($(OPENJDK_TARGET_OS), windows)
  BUILD_HOTSPOT_JTREG_EXECUTABLES_CFLAGS_exeFPRegs := -MT
  BUILD_HOTSPOT_JTREG_EXCLUDE += exesigtest.c
 endif
>>> 
>> 



Re: RFR: JDK-8199197: Set _NT_SYMBOL_PATH when running tests on windows

2018-03-07 Thread Christian Tornqvist
Hi Erik,

This looks good, many thanks for fixing this! 

Thanks,
Christian

> On Mar 7, 2018, at 12:06 32PM, Erik Joelsson  wrote:
> 
> When running tests on Windows in a distributed environment we currently don't 
> have access to debug symbols, which causes stack traces in hs_err files to be 
> useless. When running local tests this works because the product under test 
> was built on the same machine and the debug symbols are still available 
> there. For this to work in distributed testing, we need to both provide the 
> symbols bundle as well as setting up the _NT_SYMBOL_PATH environment variable 
> to point to the correct directories. This patch does just that for both the 
> old and new test makefiles. I have verified by running a modified jtreg test 
> in Mach5 that crashes hotspot and verified that the stack trace contains 
> symbols.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8199197
> 
> Webrev: http://cr.openjdk.java.net/~erikj/8199197/webrev.01/
> 
> /Erik
> 



Re: (11) RFR (S) JDK-8198640: VS2017 (LNK4281) Link Warning Against Missed ASLR Optimization

2018-02-23 Thread Christian Tornqvist
Forgot to say that the webrev looks good!

> On Feb 23, 2018, at 3:53 PM, Christian Tornqvist 
>  wrote:
> 
> Sounds like a good plan :)
> 
> Thanks,
> Christian
> 
>>> On Feb 23, 2018, at 3:22 PM, Lois Foltan  wrote:
>>> 
>>> On 2/23/2018 3:18 PM, Christian Tornqvist wrote:
>>> 
>>> Hi Lois,
>>> 
>>> Why do we link jvm.dll with -base?
>> Hi Christian,
>> It is not clear to me why we do, so I was going to follow up with an RFE to 
>> investigate & suggest the removal of -base if unnecessary.
>> Lois
>> 
>>> 
>>> Thanks,
>>> Christian
>>> 
>>>> On Feb 23, 2018, at 3:11 PM, Lois Foltan  wrote:
>>>> 
>>>> Please review this fix to ignore linker warning (LNK4281).  This is a new 
>>>> linker warning generated by VS2017 v15.5 to "to point out any 64-bit image 
>>>> specified to link with a lower than 4GB base address doesn't get best ASLR 
>>>> optimization". The Hotspot jvm.dll is specifically linked with 
>>>> -base:0x800.  As recommended by 
>>>> https://developercommunity.visualstudio.com/content/problem/160970/upgrading-from-154-to-155-throw-lnk4281-warning.html,
>>>>  this linker warning can be suppressed with -ignore.
>>>> 
>>>> open webrev at http://cr.openjdk.java.net/~lfoltan/bug_jdk8198640/webrev/
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8198640
>>>> 
>>>> Testing (hs-tier1-3 & jdk-tier1-3) in progress
>>>> 
>>>> Thanks,
>>>> Lois
>>>> 
>>>> 
>> 



Re: (11) RFR (S) JDK-8198640: VS2017 (LNK4281) Link Warning Against Missed ASLR Optimization

2018-02-23 Thread Christian Tornqvist
Sounds like a good plan :)

Thanks,
Christian

> On Feb 23, 2018, at 3:22 PM, Lois Foltan  wrote:
> 
>> On 2/23/2018 3:18 PM, Christian Tornqvist wrote:
>> 
>> Hi Lois,
>> 
>> Why do we link jvm.dll with -base?
> Hi Christian,
> It is not clear to me why we do, so I was going to follow up with an RFE to 
> investigate & suggest the removal of -base if unnecessary.
> Lois
> 
>> 
>> Thanks,
>> Christian
>> 
>>> On Feb 23, 2018, at 3:11 PM, Lois Foltan  wrote:
>>> 
>>> Please review this fix to ignore linker warning (LNK4281).  This is a new 
>>> linker warning generated by VS2017 v15.5 to "to point out any 64-bit image 
>>> specified to link with a lower than 4GB base address doesn't get best ASLR 
>>> optimization". The Hotspot jvm.dll is specifically linked with 
>>> -base:0x800.  As recommended by 
>>> https://developercommunity.visualstudio.com/content/problem/160970/upgrading-from-154-to-155-throw-lnk4281-warning.html,
>>>  this linker warning can be suppressed with -ignore.
>>> 
>>> open webrev at http://cr.openjdk.java.net/~lfoltan/bug_jdk8198640/webrev/
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8198640
>>> 
>>> Testing (hs-tier1-3 & jdk-tier1-3) in progress
>>> 
>>> Thanks,
>>> Lois
>>> 
>>> 
> 



Re: (11) RFR (S) JDK-8198640: VS2017 (LNK4281) Link Warning Against Missed ASLR Optimization

2018-02-23 Thread Christian Tornqvist
Hi Lois,

Why do we link jvm.dll with -base? 

Thanks,
Christian

> On Feb 23, 2018, at 3:11 PM, Lois Foltan  wrote:
> 
> Please review this fix to ignore linker warning (LNK4281).  This is a new 
> linker warning generated by VS2017 v15.5 to "to point out any 64-bit image 
> specified to link with a lower than 4GB base address doesn't get best ASLR 
> optimization". The Hotspot jvm.dll is specifically linked with 
> -base:0x800.  As recommended by 
> https://developercommunity.visualstudio.com/content/problem/160970/upgrading-from-154-to-155-throw-lnk4281-warning.html,
>  this linker warning can be suppressed with -ignore.
> 
> open webrev at http://cr.openjdk.java.net/~lfoltan/bug_jdk8198640/webrev/
> bug link https://bugs.openjdk.java.net/browse/JDK-8198640
> 
> Testing (hs-tier1-3 & jdk-tier1-3) in progress
> 
> Thanks,
> Lois
> 
> 



RFR: 8196197 - Enable the make system to calculate concurrency for JDK tests

2018-01-26 Thread Christian Tornqvist
Please review this small change that makes the JDK make test target use the 
same method for calculating jtreg concurrency as Hotspot. Verified locally and 
in Mach5.

Webrev: http://cr.openjdk.java.net/~ctornqvi/webrev/8196197/webrev.00/ 


Thanks,
Christian

RFR: 8194636 - Apply CONCURRENCY_FACTOR to max value in concurrency calculation

2018-01-09 Thread Christian Tornqvist
Please review this small change that allows the CONCURRENCY_FACTOR to also be 
applied when we hit the max value (currently 12) when calculating concurrency.

Webrev: http://cr.openjdk.java.net/~ctornqvi/webrev/8194636/webrev.00/ 


Thanks,
Christian

Re: RFR: 8191768 - Introduce a concurrency factor to be able to scale up or down jtreg concurrency when running Hotspot tests

2017-11-29 Thread Christian Tornqvist


> On Nov 29, 2017, at 2:22 AM, David Holmes  wrote:
> 
> Catching up after mini-vacation ...
> 
> On 23/11/2017 2:47 AM, Christian Tornqvist wrote:
>> Please review this small change that allows a user to scale up or down the 
>> concurrency for running Hotspot jtreg tests using CONCURRENCY_FACTOR.
>> Webrev: http://cr.openjdk.java.net/~ctornqvi/webrev/8191768/webrev.00/ 
>> <http://cr.openjdk.java.net/~ctornqvi/webrev/8191768/webrev.00/>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8191768 
>> <https://bugs.openjdk.java.net/browse/JDK-8191768>
> 
> So setting CONCURRENCY_FACTOR = 0 will ensure no concurrent tests will run?

It would, but you should probably just set CONCURRENCY=1 instead 

> 
> Are we guaranteed that the higher-level testing framework will only run one 
> jtreg test job per system at a time?
> 
> Thanks,
> David
> 
>> Thanks,
>> Christian



RFR: 8191768 - Introduce a concurrency factor to be able to scale up or down jtreg concurrency when running Hotspot tests

2017-11-22 Thread Christian Tornqvist
Please review this small change that allows a user to scale up or down the 
concurrency for running Hotspot jtreg tests using CONCURRENCY_FACTOR.

Webrev: http://cr.openjdk.java.net/~ctornqvi/webrev/8191768/webrev.00/ 
 
Bug: https://bugs.openjdk.java.net/browse/JDK-8191768 
 

Thanks,
Christian

RFR: 8189115 - Pass JIB_DATA_DIR to jtreg harness

2017-10-11 Thread Christian Tornqvist
Please review this small change that allows a user to specify a location for 
the JIB cache when running jtreg tests that use the JIB Artifact resolver.

Webrev: http://cr.openjdk.java.net/~ctornqvi/webrev/8189115/webrev.00/ 
 

Thanks,
Christian

RFR: 8188038 - Add Windows-x64-open bundles to jib-profiles.js

2017-09-28 Thread Christian Tornqvist
Please review this small change that adds the bundle names for the 
Windows-x64-open and ri builds to jib-profiles. 

Webrev: http://cr.openjdk.java.net/~ctornqvi/webrev/8188038/webrev.00/ 


Thanks,
Christian



RFR(XS): 8186218 - Make JIB exclude webrev from all sub-repo levels when creating source bundles

2017-08-31 Thread Christian Tornqvist
Please review this change that updates the exclude filter used by JIB when 
creating source bundles, it’ll now ignore webrev folders and webrev.zip in all 
repository levels.

Tested locally using JIB.

Webrev: http://cr.openjdk.java.net/~ctornqvi/webrev/8186218/webrev.00/ 

Bug: https://bugs.openjdk.java.net/browse/JDK-8186218 


Thanks,
Christian



Re: RFR: JDK-8186470: JDK10 hotspot integration has broken all MacOS dummy builds

2017-08-22 Thread Christian Tornqvist
Hi Erik,

This looks good, thanks for fixing this.

Thanks,
Christian

> On Aug 22, 2017, at 7:59 AM, Erik Joelsson  wrote:
> 
> When running the build on Macosx with a very long path to the root dir, we 
> have started hitting the command line length limit while linking the gtest 
> libjvm.dylib. In our case, the length of the path is not under our control, 
> so we need to find a way to deal with this.
> 
> make/common/NativeCompilation.gmk already utilizes the @-file feature of most 
> toolchains to mitigate this. The problem here is that clang does not properly 
> work with @-files when linking.
> 
> This patch rewrites the object file list to paths relative to the output dir 
> when it seems likely to be necessary, and makes sure the link command is 
> executed in that directory. I've tried to only make this rewrite happen when 
> needed (clang, >500 objects, >10 path elements in the output dir). This means 
> that for most users, there should be no difference from today and the 
> contents of the .cmdline files will continue to be runnable from any 
> directory.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8186470
> 
> Webrev: http://cr.openjdk.java.net/~erikj/8186470/webrev.01/
> 
> /Erik
> 



RFR(XS): 8184772 - Make it possible to pass arguments only to the Java running the tests when running jtreg through make

2017-08-14 Thread Christian Tornqvist
Hi everyone,

Please review this small change that enables passing options only to the Java 
under test when running jtreg through the make files. Options passed as 
-vmoption will be passed to both to the Java compiling and running the tests, 
options passed as -javaoptions are only passed to the Java running the tests.

Change has been verified locally and in our test lab by running the Hotspot 
jtreg tests along with JDK tier1-3.

Webrev: http://cr.openjdk.java.net/~ctornqvi/webrev/8184772/webrev.00/ 

Bug (Unfortunately not publicly available): 
https://bugs.openjdk.java.net/browse/JDK-8184772 


Thanks,
Christian

Re: JDK-8180600: make run-test does not work with jib test dependencies

2017-05-18 Thread Christian Tornqvist
Hi Erik,

This looks good, thanks for fixing this.

Thanks,
Christian

> On May 18, 2017, at 8:18 AM, Erik Joelsson  wrote:
> 
> The new run-test target does not work with the new jib test dependencies 
> feature in jdk10-hs. The fix is small, just adding JIB_JAR to the command 
> line for jtreg if present, as is done in make/TestCommon.gmk.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8180600
> 
> Patch:
> 
> diff -r 91ac8096f365 make/RunTests.gmk
> --- a/make/RunTests.gmk
> +++ b/make/RunTests.gmk
> @@ -358,6 +358,10 @@
> $1_JTREG_BASIC_OPTIONS += -nativepath:$$($1_JTREG_NATIVEPATH)
>   endif
> 
> +  ifneq ($(JIB_JAR), )
> +$1_JTREG_BASIC_OPTIONS += -cpa:$(JIB_JAR)
> +  endif
> +
>   run-test-$1:
> $$(call LogWarn)
> $$(call LogWarn, Running test '$$($1_TEST)')
> 
> 
> /Erik
> 



Re: RFR: JDK-8180081: Adjust Jib and JDL configuration for 10 to support promotable builds

2017-05-10 Thread Christian Tornqvist
Hi Erik,

This looks good, thanks for fixing this!

Thanks,
Christian

> On May 10, 2017, at 2:12 PM, Erik Joelsson  wrote:
> 
> This patch contains adjustments for the jib-profiles.js configuration needed 
> for supporting the next generation Oracle internal build and test system.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8180081
> 
> Webrev: http://cr.openjdk.java.net/~erikj/8180081/webrev.01/
> 
> /Erik
> 



Re: RFR: JDK-8179078: Jib run-test-prebuilt profile missing dependency on bootjdk

2017-04-21 Thread Christian Tornqvist
Hi Erik,

This looks good, I’ve verified that the fix works. Thanks for fixing this!

Thanks,
Christian

> On Apr 21, 2017, at 10:43 AM, Erik Joelsson  wrote:
> 
> (resending to include Christian)
> 
> 
> On 2017-04-21 16:37, Erik Joelsson wrote:
>> Hello,
>> 
>> When using the run-test-prebuilt Jib profile to run tests, we need to set 
>> JT_JAVA to point to the boot jdk for jtreg. The configuration to set JT_JAVA 
>> is there, but I forgot to also add boot_jdk as a dependency on that profile.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8179078
>> 
>> Patch:
>> 
>> diff -r 111e2e7d00f4 common/conf/jib-profiles.js
>> --- a/common/conf/jib-profiles.js
>> +++ b/common/conf/jib-profiles.js
>> @@ -584,7 +584,7 @@
>> var testOnlyProfilesPrebuilt = {
>> "run-test-prebuilt": {
>> src: "src.conf",
>> -dependencies: [ "jtreg", "gnumake", testedProfile + ".jdk",
>> +dependencies: [ "jtreg", "gnumake", "boot_jdk", testedProfile + 
>> ".jdk",
>> testedProfile + ".test", "src.full"
>> ],
>> work_dir: input.get("src.full", "install_path") + "/test",
>> 
>> 
>> /Erik
>> 
> 



RE: RFR(M): 8175300 - Enable artifact resolution for jtreg tests

2017-03-09 Thread Christian Tornqvist
Updated webrev based on feedback from Magnus Ihse Bursie:
http://cr.openjdk.java.net/~ctornqvi/webrev/8175300/webrev.01/

-Original Message-
From: hotspot-dev [mailto:hotspot-dev-boun...@openjdk.java.net] On Behalf Of
Christian Tornqvist
Sent: Tuesday, March 7, 2017 3:03 PM
To: hotspot-...@openjdk.java.net; 'build-dev' 
Subject: RFR(M): 8175300 - Enable artifact resolution for jtreg tests

Hi everyone,

 

Please review this change that adds the ability to have external
dependencies in jtreg tests. As a proof of concept, I've added Scimark to
the Hotspot repository. The concept is this, annotate the test with one or
more @Artifact annotation describing the external dependencies, these can
then either be resolved manually (using -Djdk.test.lib.artifacts.) or
automatically using a custom ArtifactManager.

 

The make changes were contributed by Erik Joelsson.

These changes have been verified by running the Scimark test locally (using
make) and in JPRT.

 

Bugs:

https://bugs.openjdk.java.net/browse/JDK-8175300

Webrev:

http://cr.openjdk.java.net/~ctornqvi/webrev/8175300/webrev.00/

 

Thanks,

Christian

 

 

 




RFR(M): 8175300 - Enable artifact resolution for jtreg tests

2017-03-07 Thread Christian Tornqvist
Hi everyone,

 

Please review this change that adds the ability to have external
dependencies in jtreg tests. As a proof of concept, I've added Scimark to
the Hotspot repository. The concept is this, annotate the test with one or
more @Artifact annotation describing the external dependencies, these can
then either be resolved manually (using -Djdk.test.lib.artifacts.) or
automatically using a custom ArtifactManager.

 

The make changes were contributed by Erik Joelsson.

These changes have been verified by running the Scimark test locally (using
make) and in JPRT.

 

Bugs:

https://bugs.openjdk.java.net/browse/JDK-8175300

Webrev:

http://cr.openjdk.java.net/~ctornqvi/webrev/8175300/webrev.00/

 

Thanks,

Christian

 

 

 



RFR(XS): 8168137 - import-hotspot build target not removed from hotspot-ide-project

2016-12-19 Thread Christian Tornqvist
Hi,

 

Please review this small change to not use the import-hotspot make target,
also updated where to find jvm.dll once the build has completed.

Tested using make hotspot-ide-project and then using VS to
build/rebuild/debug using the generated project files.

 

Webrev:

http://cr.openjdk.java.net/~ctornqvi/webrev/8168137/webrev.00/ 

Bug:

https://bugs.openjdk.java.net/browse/JDK-8168137

 

Thanks,

Christian

 



RE: JDK-8171163 Merge 9de6a70d5d81 broke test/Makefile

2016-12-13 Thread Christian Tornqvist
Hi Magnus, 

This looks good, thanks for fixing this.

Thanks,
Christian

-Original Message-
From: hotspot-dev [mailto:hotspot-dev-boun...@openjdk.java.net] On Behalf Of 
Magnus Ihse Bursie
Sent: Tuesday, December 13, 2016 7:49 AM
To: build-dev ; hotspot-dev 

Subject: RFR: JDK-8171163 Merge 9de6a70d5d81 broke test/Makefile

The merge 9de6a70d5d81 (in jdk9/hs) reintroduced code that was deleted in 
JDK-8170629.

The result of this is that the target jtreg_tests is declared two times, which 
causes make to protest.

Note that the broken merge is in jdk9/hs, so this is where the fix needs to go.

Bug: https://bugs.openjdk.java.net/browse/JDK-8171163
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8171163-borked-merge-on-test-Makefile/webrev.01

/Magnus



Re: RFR: JDK-8156018: Hotspot visual studio project generation broken

2016-05-09 Thread Christian Tornqvist
Hi Erik,

Looks good, thanks for fixing this!

Thanks,
Christian

> On May 9, 2016, at 6:09 AM, Erik Joelsson  wrote:
> 
> Friendly reminder. If at least someone who cares about VS projects could
> try it out and stamp this, that should be enough I think?
> 
> /Erik
> 
>> On 2016-05-04 11:24, Erik Joelsson wrote:
>> With the new hotspot build, there was supposed to be support for
>> creating a visual studio project, as was possible in the old build
>> system. When creating the final changeset I missed including some of
>> the files needed for that. Here are the changes as they were supposed
>> to be, including removing the old version of the files since the old
>> build has already been removed.
>> 
>> These changes were made by Magnus Ihse Bursie. Christian Törnqvist has
>> verified that the generated project works.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8156018
>> Webrev: http://cr.openjdk.java.net/~erikj/8156018/webrev.hotspot.01/
>> 
>> /Erik
> 



RE: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

2014-08-20 Thread Christian Tornqvist
I talked to Staffan Friberg (perf tech lead) about enabling this in a
release builds and this is his opinion: 

"For /homeparams I think enable it now in fastdebug, let's use it for a
while and understand home much of a benefit it is for debugging. If it is a
huge help let's consider it for release, but that will require a lot of
performance testing before commiting"

I agree with Staffan and think we should only enable it in fastdebug builds
for now.

Thanks,
Christian
 
-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com] 
Sent: Wednesday, August 20, 2014 12:02 PM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

On 8/20/14 9:48 AM, Christian Tornqvist wrote:
>> indicate that we shouldn't need this change in our debug/fastdebug
builds.
> However, you looked at the generated prologue code before and after 
> turning on this option right?
>
> Our fastdebug builds are compiled with /O2 which doesn't spill the 
> parameters onto the stack, our debug builds (with /Od) will do this so 
> passing /homeparams there is a no-op.
>
> Here is a look at the prologue code for
> ClassFileParser::parse_constant_pool_entries() before /homeparams:
>
> mov dword ptr [rsp+10h],edx
> pushrbp
> pushrsi
>
> and here is with /homeparams:
>
> mov qword ptr [rsp+18h],r8
> mov dword ptr [rsp+10h],edx
> mov qword ptr [rsp+8],rcx
> pushrbp
> pushrsi

Cool. When we did the Full Debug Symbols (FDS) project, one of the things we
did was try to use the same optimization and debug options with
RELEASE/product builds as with fastdebug builds. Since we're generating
debug info for all builds configs, we thought this was a really good design
goal unless we ran into a performance issue.

During the FDS project, we saw no performance change when we switched the
optimization options and included the various generate-debug-info options.

A long way of saying:

I think you should enable /homeparams for RELEASE/product builds.

:-)

Dan


>
> Thanks,
> Christian
>
> -Original Message-----
> From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
> Sent: Wednesday, August 20, 2014 11:17 AM
> To: Christian Tornqvist
> Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
> Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds 
> using /homeparams
>
> On 8/20/14 8:49 AM, Christian Tornqvist wrote:
>>> Do we have any idea how much of a change? Do we care? I'm presuming 
>>> the
>> increased debuggability is worth it.
>>
>> It adds 0-4 additional stack movs in function prologue code which 
>> should have a very low impact, it's only on debug/fastdebug builds.
> I missed that this was a non-RELEASE build change in my original review.
>
> These two blurbs from the MSDN note:
>
>   > However, by default in a release build, the register parameters  > 
> will not be written to the stack, into the space that is already  > 
> provided for the parameters. This makes it difficult to debug an  > 
> optimized (release) build of your program.
>
> and
>
>   > In a debug build, the stack is always populated with parameters  > 
> passed in registers.
>
> indicate that we shouldn't need this change in our debug/fastdebug builds.
> However, you looked at the generated prologue code before and after 
> turning on this option right?
>
> Does this mean that the MSDN note is not correct for the way that we 
> do debug and fastdebug builds? We might have some option enabled that 
> prevents the default /homeparams behavior from working...
>
> Dan
>
>
>>> The above block will also apply to an "ia64" build. We don't support 
>>> that
>> anymore, but I don't know if any licensees support it.
>>
>> I've changed the checks in vm.make and WinGammaPlatformVC10.java
>>
>>> Do we need to do anything about the new, but unused  platform to 
>>> make
>> lint-like tools not squawk?
>>
>> I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.
>>
>> New webrev:
>> http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/
>>
>> Thanks,
>> Christian
>>
>> -Original Message-
>> From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
>> Sent: Wednesday, August 20, 2014 9:48 AM
>> To: Christian Tornqvist
>> Cc: hotspot-...@openjdk.java.net
>> Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds 
>> using /homeparams
>>
>>> http://

RE: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

2014-08-20 Thread Christian Tornqvist
> indicate that we shouldn't need this change in our debug/fastdebug builds.
However, you looked at the generated prologue code before and after turning
on this option right?

Our fastdebug builds are compiled with /O2 which doesn't spill the
parameters onto the stack, our debug builds (with /Od) will do this so
passing /homeparams there is a no-op. 

Here is a look at the prologue code for
ClassFileParser::parse_constant_pool_entries() before /homeparams:

mov dword ptr [rsp+10h],edx
pushrbp
pushrsi

and here is with /homeparams:

mov qword ptr [rsp+18h],r8
mov dword ptr [rsp+10h],edx
mov qword ptr [rsp+8],rcx
pushrbp
pushrsi

Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com] 
Sent: Wednesday, August 20, 2014 11:17 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

On 8/20/14 8:49 AM, Christian Tornqvist wrote:
>> Do we have any idea how much of a change? Do we care? I'm presuming 
>> the
> increased debuggability is worth it.
>
> It adds 0-4 additional stack movs in function prologue code which 
> should have a very low impact, it's only on debug/fastdebug builds.

I missed that this was a non-RELEASE build change in my original review.

These two blurbs from the MSDN note:

 > However, by default in a release build, the register parameters  > will
not be written to the stack, into the space that is already  > provided for
the parameters. This makes it difficult to debug an  > optimized (release)
build of your program.

and

 > In a debug build, the stack is always populated with parameters  > passed
in registers.

indicate that we shouldn't need this change in our debug/fastdebug builds.
However, you looked at the generated prologue code before and after turning
on this option right?

Does this mean that the MSDN note is not correct for the way that we do
debug and fastdebug builds? We might have some option enabled that prevents
the default /homeparams behavior from working...

Dan


>
>> The above block will also apply to an "ia64" build. We don't support 
>> that
> anymore, but I don't know if any licensees support it.
>
> I've changed the checks in vm.make and WinGammaPlatformVC10.java
>
>> Do we need to do anything about the new, but unused  platform to make
> lint-like tools not squawk?
>
> I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.
>
> New webrev:
> http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/
>
> Thanks,
> Christian
>
> -Original Message-
> From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
> Sent: Wednesday, August 20, 2014 9:48 AM
> To: Christian Tornqvist
> Cc: hotspot-...@openjdk.java.net
> Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds 
> using /homeparams
>
>   > http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/
>
> General Comment
>
> The MSDN note says:
>
>   > /homeparams does imply a performance disadvantage, because it  > 
> does require a cycle to load the register parameters on to the stack.
>
> Do we have any idea how much of a change? Do we care? I'm presuming 
> the increased debuggability is worth it.
>
> make/windows/makefiles/vm.make
>   line 38: !else
>   line 39: CXX_FLAGS=$(CXX_FLAGS) /D "ASSERT" /homeparams
>   line 40: !endif
>   The above block will also apply to an "ia64" build.
>   We don't support that anymore, but I don't know if
>   any licensees support it.
>
> src/share/tools/ProjectCreator/BuildConfig.java
>   No comments.
>
> src/share/tools/ProjectCreator/WinGammaPlatformVC10.java
>   line 373: if(!platformName.equals("Win32")) {
>   line 374: addAttr(rv, "AdditionalOptions", "/homeparams");
>   The above block will also apply to an "ia64" build.
>
> src/share/tools/ProjectCreator/WinGammaPlatformVC7.java
> src/share/tools/ProjectCreator/WinGammaPlatformVC8.java
>   Do we need to do anything about the new, but unused
>   platform to make lint-like tools not squawk?
>
> Thumbs up.
>
> Dan
>
>
> On 8/19/14 5:39 PM, Christian Tornqvist wrote:
>> Hi everyone,
>>
>>
>>
>> This change adds /homeparams
>> (http://msdn.microsoft.com/en-us/library/6exwh0y6.aspx) to compiler 
>> flags when building fastdebug on Windows x64. This causes the 
>> compiler to generate code to spill the first 4 arguments to the stack 
>> (they're normally only passed in regist

RE: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

2014-08-20 Thread Christian Tornqvist
> Do we have any idea how much of a change? Do we care? I'm presuming the
increased debuggability is worth it.

It adds 0-4 additional stack movs in function prologue code which should
have a very low impact, it's only on debug/fastdebug builds.

> The above block will also apply to an "ia64" build. We don't support that
anymore, but I don't know if any licensees support it.

I've changed the checks in vm.make and WinGammaPlatformVC10.java

> Do we need to do anything about the new, but unused  platform to make
lint-like tools not squawk?

I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.

New webrev:
http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/

Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com] 
Sent: Wednesday, August 20, 2014 9:48 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

 > http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

General Comment

The MSDN note says:

 > /homeparams does imply a performance disadvantage, because it  > does
require a cycle to load the register parameters on to the stack.

Do we have any idea how much of a change? Do we care? I'm presuming the
increased debuggability is worth it.

make/windows/makefiles/vm.make
 line 38: !else
 line 39: CXX_FLAGS=$(CXX_FLAGS) /D "ASSERT" /homeparams
 line 40: !endif
 The above block will also apply to an "ia64" build.
 We don't support that anymore, but I don't know if
 any licensees support it.

src/share/tools/ProjectCreator/BuildConfig.java
 No comments.

src/share/tools/ProjectCreator/WinGammaPlatformVC10.java
 line 373: if(!platformName.equals("Win32")) {
 line 374: addAttr(rv, "AdditionalOptions", "/homeparams");
 The above block will also apply to an "ia64" build.

src/share/tools/ProjectCreator/WinGammaPlatformVC7.java
src/share/tools/ProjectCreator/WinGammaPlatformVC8.java
 Do we need to do anything about the new, but unused
 platform to make lint-like tools not squawk?

Thumbs up.

Dan


On 8/19/14 5:39 PM, Christian Tornqvist wrote:
> Hi everyone,
>
>   
>
> This change adds /homeparams
> (http://msdn.microsoft.com/en-us/library/6exwh0y6.aspx) to compiler 
> flags when building fastdebug on Windows x64. This causes the compiler 
> to generate code to spill the first 4 arguments to the stack (they're 
> normally only passed in registers), which should make it easier to debug.
>
>   
>
> I also changed the ProjectCreator to enable building with this using 
> Visual Studio. The size of jvm.dll increases with about 3% (about 504k
increase).
>
>   
>
> Verified that it builds correctly using Visual Studio and JPRT, the 
> generation of the spill code has been verified by comparing prologue 
> code for several functions in the JVM with/without using /homeparams.
>
>   
>
> Webrev:
>
> http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/
>
>   
>
> Bug:
>
> https://bugs.openjdk.java.net/browse/JDK-8027480
>
>   
>
> Thanks,
>
> Christian
>
>   
>




RE: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

2014-08-20 Thread Christian Tornqvist
Adding build-dev as well

-Original Message-
From: hotspot-dev [mailto:hotspot-dev-boun...@openjdk.java.net] On Behalf Of
Christian Tornqvist
Sent: Tuesday, August 19, 2014 7:39 PM
To: hotspot-...@openjdk.java.net
Subject: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

Hi everyone,

 

This change adds /homeparams
(http://msdn.microsoft.com/en-us/library/6exwh0y6.aspx) to compiler flags
when building fastdebug on Windows x64. This causes the compiler to generate
code to spill the first 4 arguments to the stack (they're normally only
passed in registers), which should make it easier to debug. 

 

I also changed the ProjectCreator to enable building with this using Visual
Studio. The size of jvm.dll increases with about 3% (about 504k increase).

 

Verified that it builds correctly using Visual Studio and JPRT, the
generation of the spill code has been verified by comparing prologue code
for several functions in the JVM with/without using /homeparams. 

 

Webrev:

http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

 

Bug:

https://bugs.openjdk.java.net/browse/JDK-8027480

 

Thanks,

Christian

 




RE: Proposal: jtreg tests with native components

2014-04-28 Thread Christian Tornqvist
Hi Staffan,

This sounds like a great proposal that would solve many of our issues with
tests requiring native code. Would it be possible for the make system to
pick up a custom makefile from a test folder? The use cases I see are: 

1. Launcher type tests (we have a few of these), these needs to be compiled
into an executable 
2. Test libraries that might depend on being compiled with certain
compiler/linker flags (don't think we have tests like this today though).

Thanks for working on this :)

Thanks,
Christian

-Original Message-
From: jtreg-dev [mailto:jtreg-dev-boun...@openjdk.java.net] On Behalf Of
Staffan Larsen
Sent: Friday, April 25, 2014 8:03 AM
To: build-dev@openjdk.java.net build-dev; jtreg-...@openjdk.java.net
Subject: Proposal: jtreg tests with native components

There are a couple of jtreg tests today that depend on native components
(either JNI libraries or executables). These are handled in one of two ways:

1) The binaries are pre-compiled and checked into the repository (often
inside jar files).
2) The test will try to invoke a compiler (gcc, cl, .) when the test is
being run.

Neither of these are very good solutions. #1 makes it hard to run the setup
the test for all platforms and requires binaries in the source control
system. #2 is hit-and-miss: the correct compiler may or may not be installed
on the test machine, and the approach requires platform specific logic to be
maintained.

I would like to propose that these native components are instead compiled
when the product is built by the same makefile logic as the product. At
product build time we know we have access to the (correct) compilers and we
have excellent support in the makefiles for building on all platforms.

If we build the native test components together with the product, we also
have to take care of distributing the result together with the product when
we do testing across a larger number of machines. We will also need a way to
tell the jtreg tests where these pre-built binaries are located.

I suggest that at the end of a distributed build run, the pre-built test
binaries are packaged in a zip or tar file (just like the product bits) and
stored next to the product bundles. When we run distributed tests, we need
to pick up the product bundle and the test bundle before the testing is
started.

To tell the tests where the native code is, I would like to add a flag to
jtreg to point out the path to the binaries. This should cause jtreg to set
java.library.path before invoking a test and also set a test.* property
which can be used by test to find it's native components.

This kind of setup would make it easier to add and maintain tests that have
a native component. I think this will be especially important as more tests
are written using jtreg in the hotspot repository.

Thoughts on this? Is the general approach ok? There are lots of details to
be figured out, but at this stage I would like to hear feedback on the idea
as such.

Thanks,
/Staffan




Re: RFR: 8007446: Add /MP to cl.exe speeds up windows builds of OpenJDK.

2013-10-03 Thread Christian Tornqvist
Looks good, many thanks for fixing this!

Thanks,
Christian

On Oct 3, 2013, at 5:01, Erik Joelsson  wrote:

> Please review this simple patch adding /MP to the windows cl.exe command 
> lines when supported. In my tests, the speed improvement is around 40% on the 
> hotspot part of the build on the Stockholm jprt queue and my local build. 
> This is really low hanging fruit for build speed improvements.
> 
> http://cr.openjdk.java.net/~erikj/8007446/webrev.hotspot.01/
> 
> I will also need a sponsor to push this for me if review is passed.
> 
> /Erik


RE: RFR (XS) JDK-8025569: -XX:+CheckUnhandledOops crashes on Windows

2013-10-02 Thread Christian Tornqvist
Hi Lois,

 

Looks good, might be worth clarifying that this disables CheckUnhandledOops for 
both 32 and 64-bit Windows.

 

Thanks,

Christian 

 

From: hotspot-runtime-dev-boun...@openjdk.java.net 
[mailto:hotspot-runtime-dev-boun...@openjdk.java.net] On Behalf Of Lois Foltan
Sent: Tuesday, October 1, 2013 4:51 PM
To: hotspot-runtime-...@openjdk.java.net; build-dev@openjdk.java.net
Subject: RFR (XS) JDK-8025569: -XX:+CheckUnhandledOops crashes on Windows

 

 

Please review the following fix: 

Webrev: 
http://cr.openjdk.java.net/~coleenp/bug_jdk8025569/

Bug: -XX:+CheckUnhandledOops crashes on Windows
https://bugs.openjdk.java.net/browse/JDK-8025569

Summary of fix: 
Until JDK-8024364,   
(Win/x64: os::current_frame() and os::fetch_frame_from_context() returns wrong 
frame pointer),
is fixed, -XX:+CheckUnhandledOops can not be supported for Windows 
fastdebug builds due to the need to have 
a legitimate frame returned from os::current_frame() early on in 
universe_post_init() to register & deregister oops.

Tests: 
JTREG on Windows (in progress)

Thank you, Lois 

 



RE: RFR (M): 8008772: remove gamma launcher

2013-05-01 Thread Christian Tornqvist
Hi Vladimir,

Nils attached the patch to the email, but I've put the patch at
http://cr.openjdk.java.net/~ctornqvi/webrev/vs.patch also

Thanks,
Christian

-Original Message-
From: hotspot-dev-boun...@openjdk.java.net
[mailto:hotspot-dev-boun...@openjdk.java.net] On Behalf Of Vladimir Kozlov
Sent: den 1 maj 2013 17:22
To: Nils Eliasson
Cc: build-dev@openjdk.java.net; hotspot-dev developers; Christian Thalinger
Subject: Re: RFR (M): 8008772: remove gamma launcher

Nils,

I don't see attached patch or webrev link.

thanks,
Vladimir

On 5/1/13 1:41 PM, Nils Eliasson wrote:
> Hi,
>
> Here is a patch that fixes so that it still works to create vcprojs, 
> and also sets defaults for the debugger commands.
>
> I have verified with creating vcprojs and debugging hotspot in VS 2010.
> Works great except for an annoying pop-up that warns that the launcher 
> doesn't have debug symbols (if the target jdk doesn't).
>
> It will still be some extra work for those using the commandlines 
> plugin. All saved command lines need the be prepended with
> "-XXaltjvm=$(TargetDir) -Dsun.java.launcher=gamma" and the target JDK 
> set as executable to work. We should update the plugin to help with that.
>
> A big thank you to Christian Törnqvist for the proposal!
>
> //Nils
>
>
> On 2013-04-23 20:33, Mikael Gerdin wrote:
>>
>> On 2013-04-23 20:28, Christian Thalinger wrote:
>>>
>>> On Apr 23, 2013, at 11:06 AM, Nils Eliasson 
>>>  wrote:
>>>
 As long as we fix it first and remove gamma after - I would love to 
 have some redundant code removed. I would fix it myself, I just 
 don't think I will have the time before I go on parental leave.
>>>
>>> First, I'm not removing it tomorrow.  I expected a long discussion 
>>> :-)
>>>
>>> What exactly is the problem with Visual Studio?  Why can't you just 
>>> run the java launcher instead?
>>>
>>
>> I don't know if there actually is a problem, but I don't think 
>> anyone's actually tried to tell it to use the java launcher.
>> The VS project is automatically setup to launch "hotspot.exe" from 
>> the IDE. "hotspot.exe" is equivalent to the old option LINK_INTO=AOUT
>> (IIRC) which involves linking all the VM object files into the launcher.
>>
>> Nils, perhaps you can at least try this before you leave?
>>
>> /Mikael
>>
>>> -- Chris
>>>

 //Nils

 On 2013-04-23 12:59, Mikael Gerdin wrote:
>
>
> On 2013-04-23 11:09, Nils Eliasson wrote:
>> The gamma launcher is used to run and debug hotspot from Visual 
>> Studio.
>> So removing gamma effectively kills the working environment for a 
>> number of people that use it daily.  So I am strongly against 
>> removing it.
>
> Maybe the visual studio project generator could be updated to 
> create a a "launch configuration" for launching java.exe from a 
> JDK and use the XXaltJVM flag on to select the correct jvm.dll?
>
> I agree that we shouldn't break the visual studio project but 
> currently there's nothing indicating that we can't fix it.
>
>
> /Mikael
>
>>
>> Most people working on Windows use Cygwin as the last resort 
>> since it makes a lot of thing excruciatingly slow.
>>
>> //Nils
>>
>> On 2013-04-22 22:55, Christian Thalinger wrote:
>>> On Apr 22, 2013, at 1:36 PM, Daniel D. Daugherty 
>>>  wrote:
>>>
 Chris,

 Just an observation and not a review.

 Looks like you're removing launcher support on Windows, but it 
 looks like the new hotspot.script doesn't support Windows...
 Am I missing something?
>>> Almost certainly true.  Since I'm not a Windows user (and nobody 
>>> near me is one) I have no idea how people are using the gamma 
>>> launcher on Windows (or the hotspot script for that matter).
>>>
>>> I presume most people doing debugging on the command line are 
>>> already in cygwin?  But I might be wrong.
>>>
>>> -- Chris
>>>
 Dan


 On 4/22/13 1:47 PM, Christian Thalinger wrote:
> http://cr.openjdk.java.net/~twisti/8008772/
>
> 8008772: remove gamma launcher
> Reviewed-by:
>
> Remove linking the gamma launcher and it's associated source 
> files.
>
> make/Makefile
> make/bsd/makefiles/launcher.make make/bsd/makefiles/vm.make 
> make/hotspot.script make/linux/makefiles/launcher.make
> make/linux/makefiles/vm.make
> make/solaris/makefiles/launcher.make
> make/solaris/makefiles/vm.make 
> make/windows/makefiles/debug.make 
> make/windows/makefiles/fastdebug.make
> make/windows/makefiles/launcher.make
> make/windows/makefiles/product.make
> make/windows/makefiles/projectcreator.make
> make/windows/projectfiles/common/Makefile
> src/os/posix/launcher/java_md.c 
> src/os/posix/launcher/java_md