Re: Building jdk9 on Windows x64 and Visual Studio 2015 Communityedition

[Magnus Ihse Bursie]

On 2015-12-18 05:11, David Holmes wrote:

On 18/12/2015 3:28 AM, Kumar Srinivasan wrote:

I am good with this change!.

I would like the rest of the component teams to weigh in on the others.


This needs to be sent to all the affected component teams, or even 
better split into three pieces: build, hotspot and JDK libs, and then 
sent.


I sent the RFR (in another thread) to awt-dev 
, hotspot-dev , 
core-libs-dev . I thought that was 
enough. Do you suggest any more mailing lists?


/Magnus



Hotspot changes looked okay to me - though the need for casts in 
various places was unpleasant - we may want neater ways to deal with 
that.



Thanks,
David


Thanks
Kumar

On 12/16/2015 12:18 PM, Magnus Ihse Bursie wrote:

On 2015-12-16 16:33, Kumar Srinivasan wrote:

Hello,

http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01/jdk/src/jdk.pack200/share/native/common-unpack/utils.h.udiff.html 




You are undefining Windows math.h OVERFLOW, what is it defined
as ? With you redefining this, will it cause problems for users of
this API, likely to affect JNI apps.

Probably need to redefine the pack200 OVERFLOW constant to
something else, probably PACK200_OVERFLOW to prevent namespace
collisions.


I agree, this is a better solution. I've updated the webrev with this
solution (although I used the name PSIZE_OVERFLOW to align with
PSIZE_MAX).

http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.02 




/Magnus




Kumar

On 12/16/2015 5:35 AM, Magnus Ihse Bursie wrote:

On 2015-11-05 18:03, Timo Kinnunen wrote:
Hi,
  I have signed the OCA and emailed a scan according the
instructions. I separated the freetype changes into a separate
batch as suggested. I have shared the patch files on OneDrive, they
are my Public folder. Here’s the link to the folder:
https://onedrive.live.com/redir?resid=a243a3e0b2aaacfa%21107
  The OneDrive folder should contain these 4 files:
  freetype_JDK9.patch
vs2015_JDK9.patch
vs2015_JDK9_hotspot.patch
vs2015_JDK9_jdk.patch
  The first two target the root repository, the other two are for
hotspot and jdk repositories, respectively.
  I rebased the patches on JDK9 tip today. I ran “make
images” and fixed a couple of new errors that have appeared since
the previous version. A couple of the changes I had made were also
not needed any more.
  Please have a look!

Hi Timo,

I'm sorry for the long delay.

I have created JDK-8145548 for the freetype fix. I'm sponsoring this
fix. I'll send out a proper review on cr.openjdk.java.net shortly.

The vs2015 changes are more complicated since they touch multiple
places in the code. Also, your patch had started bitrotting slightly
during my long response time. I'm trying to fix it up, and will post
a review when I have sorted everything out. I will verify that the
change does not break any of our supported platforms, but I'd like
you to verify that the patch still works on VS2015. When I publish
the webrev, there will be a download link to the patch file.

Also, this patch touches both hotspot code and multiple jdk modules,
so it will need to be reviewed by other groups as well, besides the
build team.

/Magnus


Sent from Mail for Windows 10

From: Magnus Ihse Bursie
Sent: Friday, October 23, 2015 17:13
To: timo.kinnu...@gmail.com;build-dev
Subject: Re: Building jdk9 on Windows x64 and Visual Studio 2015
Communityedition
On 2015-09-25 17:55, timo.kinnu...@gmail.com wrote:

Hi,

I’ve been going over the Windows build of the whole JDK for a
while with VS 2015 and now I have patches that allow the build to
complete.

I’ve made changes in the root repository as well as in hotspot and
jdk repositories. The changes fall broadly in three categories:
enabling the v140 toolchain and improving freetype compilation,
adding casts to where pointers are truncated and miscellaneous
small-scale code changes.

The patch to the root repository streamlines handling of freetype
by implementing a default source directory at $HOME/freetype under
Cygwin. It is checked during configure and used for compiling if
“--with-freetype-src” is not specified. A help message giving the
unpacking command with the correct directory is also included.
This patch is about 90 lines without counting
generated-configure.sh changes.

The patches to jdk and hotspot contain native code changes only
and no changes to make-files. These are about 580 and 290 lines,
respectively. All patches are generated with “hg diff -g”.

Would you be willing to incorporate these? How should I proceed
with this?

  Hi Timo,
  First of all, I apologize that you have not recieved any response
for
far too long. :-(
  Thank you for your interest in helping to improve OpenJDK!
  In general, a patch to allow compilation on VS 2015
Community edition
sounds like a good edition to OpenJDK. I am willing to sponsor this
patch and help you work with getting it accepted.
  My first question to you is: have you signed the OCA? Also, unless
you've done so already,

Re: RFR: JDK-8145549 Add support for Visual Studio 2015 Community edition

[Magnus Ihse Bursie]

On 2015-12-16 14:50, Magnus Ihse Bursie wrote:

There is an interest from the community to build OpenJDK using Visual Studio 
2015 Community edition.

This patch is provided by Timo Kinnunen . I am 
sponsoring this patch.

The changes to the source code files are mostly casts to uintptr_t, but there 
are some other changes as well.

I'm not quite sure who's the owner of all these files. If I'm missing some 
group, please help me and forward the mail to them.

Bug: https://bugs.openjdk.java.net/browse/JDK-8145549
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01

/Magnus


Kumar had a reply to this which unfortunately ended up on a different 
thread on which the component teams were not cc'ed. Here are the 
relevant parts:



On 12/16/2015 12:18 PM, Magnus Ihse Bursie wrote:
On 2015-12-16 16:33, Kumar Srinivasan wrote:

Hello,

http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01/jdk/src/jdk.pack200/share/native/common-unpack/utils.h.udiff.html 



You are undefining Windows math.h OVERFLOW, what is it defined
as ? With you redefining this, will it cause problems for users of
this API, likely to affect JNI apps.

Probably need to redefine the pack200 OVERFLOW constant to
something else, probably PACK200_OVERFLOW to prevent namespace
collisions.


I agree, this is a better solution. I've updated the webrev with this 
solution (although I used the name PSIZE_OVERFLOW to align with 
PSIZE_MAX).


http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.02 



/Magnus


/Magnus

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Magnus Ihse Bursie]

On 2015-12-17 22:24, David Holmes wrote:

On 17/12/2015 6:18 PM, Magnus Ihse Bursie wrote:

On 2015-12-17 09:08, David Holmes wrote:

On 17/12/2015 5:54 PM, Erik Joelsson wrote:



On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:

One more thing, where should this fix be pushed? Do you need it
urgently
in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it
affects nightly testing and JPRT. If Alejandro agrees I suggest
pushing to jdk9/hs and it can then be pulled down to the team repos.


Will do.

With regard to the fix ... previously DEBUG_BINARIES was never set in
spec.gmk and so was never externally introduced into the hotspot 
build
this way. So why not simply change the name of the variable so 
that it

has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at
this point. This patch aims to restore the "external" and "zipped"
settings to exactly what they were before the patch, as was promised.


I had not inferred from any of this that what was being done via
NativeCompilation.gmk was in any way a problem. I would have expected
any problems there to be readily seen before this was reviewed and
pushed. So I'm a bit confused about this.


The changes in NativeCompilation.gmk looked perfectly fine. They turned
out to trigger a bug (or, at the very least, very unsuspected behavior)
which was deeply hidden in the hotspot linux makefiles, where setting
DEBUG_BINARIES did indeed enable the -g flag, but also, as a side
effect, turned fastdebug builds into slowdebug builds. This was not
something that we could possibly forsee.


That was my understanding too - that this was simply a side-effect on 
hotspot, hence my suggestion to rename the variable. But Erik then 
said this was also not working correctly for the JDK side - which 
means the problem is far worse.


JDK-8036003 introduced the the use of DEBUG_BINARIES in jdk code as 
well. So it "affects" jdk libraries as well, but not with the kind of 
performance regression. However, it did enable debug information were we 
didn't have it before in the Oracle builds. While I believe this is a 
good thing, it should be added under more controlled forms. Therefore we 
want to revert the effect of DEBUG_BINARIES for the kinds of build we do 
at Oracle (external/zipped), keep the effect added in JDK-8036003 for 
the new kind of builds the community requested (internal), and finally 
we want to enable debug symbols for all our jdk libraries as a separate 
change, JDK-8145596.


The only thing left to do is for the community to provide a fix to the 
problem that the newly added functionality of debug-symbols=internal 
means that hotspot in effect turns into a slowdebug build. Since the 
community regularly builds by setting DEBUG_BINARIES=true on the command 
line, it's not really a regression for them, but it's still a bug since 
they get unnecessarily slow builds. Either they need to provide a patch, 
or wait for the new hotspot build where this won't be a problem.


/Magnus


It was not "far worse", but



David
-




I'm tempted to say rollback the whole thing rather than hack at it.


I don't think there is any compelling reason to rollback the whole
change. The basic idea is sound. However, we do need to work on how to
handle the debug symbols "under the hood". This deficit has been known
for a long time to me and Erik but we have not spent any time on it.
This change brought the problems up to daylight, and I think that's a
good thing.

I've started working on JDK-8145596, which will provide a proper
solution to how we handle debug symbols. It is based on JDK-8036003.

/Magnus




And apologies as I'm just about to go offline for a few hours at least.

David
-




We will followup with a more complete fix.

/Erik

Thanks,
David


/Erik

On 2015-12-16 22:34, Erik Joelsson wrote:

Hello,

Please review this quick fix for the build issue introduced in
Hotspot
by JDK-8036003. The short story is that if you set
DEBUG_BINARIES=true
when building Hotspot fastdebug, you essentially get a slowdebug
build. For an explanation of why, see comment in bug. This
behavior is
of course also a bug, but not something I will address in this 
quick

fix.

What happened in JDK-8036003 was that a new configure API for
controlling debug symbols was introduced. The two main settings of
this new parameter, --with-native-debug-symbols, that we use
internally at Oracle are "external" and "zipped". It was 
important to

us that the behavior of these did not change with JDK-8036003, but
exactly that did happen, because both of these settings now cause
DEBUG_BINARIES=true to be set. This variable has never been set by
configure before and because of the above weird behavior in the
Hotspot makefiles, we are having problems.

My proposed quick fix is to not set DEBUG_BINARIES=true for
"external"
or "zipped". It can remain true for "internal" since Oracle never
bui

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Magnus Ihse Bursie]

On 2015-12-17 22:32, David Holmes wrote:

On 17/12/2015 7:24 PM, Erik Joelsson wrote:

On 2015-12-17 09:08, David Holmes wrote:

On 17/12/2015 5:54 PM, Erik Joelsson wrote:



On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:

One more thing, where should this fix be pushed? Do you need it
urgently
in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it
affects nightly testing and JPRT. If Alejandro agrees I suggest
pushing to jdk9/hs and it can then be pulled down to the team repos.


Will do.

With regard to the fix ... previously DEBUG_BINARIES was never set in
spec.gmk and so was never externally introduced into the hotspot 
build
this way. So why not simply change the name of the variable so 
that it

has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at
this point. This patch aims to restore the "external" and "zipped"
settings to exactly what they were before the patch, as was promised.


I had not inferred from any of this that what was being done via
NativeCompilation.gmk was in any way a problem. I would have expected
any problems there to be readily seen before this was reviewed and
pushed. So I'm a bit confused about this.


I didn't follow this particular review closely as Magnus was on it and
so I had missed the NativeCompilation part. It's true that it is very
unlikely to be part of the problem described in this bug, but I still
feel that the safest action at this point is to restore the behavior of
"external" and "zipped" to what they used to be.


My concern is recognizing that these adjustments do in fact restore 
the old behaviour. From the hotspot side it seemed cleaner to avoid 
the breakage by using a different variable name.


I'm not sure about this talk about a different variable name? The 
*point* of the patch was *exactly* to set DEBUG_BINARIES=true for 
internal builds. The problem was that it also set it for 
external/zipped, which was incorrect. But neither the writer of the 
patch nor me (or any other reviewer) realized that this would have this 
"hammer" effect.






Magnus is working on a
complete fix where debug symbols will be enabled for everything in
NativeCompilation.

I'm tempted to say rollback the whole thing rather than hack at it.


Rolling back will be much more work for me than submitting this patch.
There are two changes already that depend on this change. I also don't
dislike the idea of the new configure parameter.


Rolling back the new configure parameters and then reinstating them 
again would also not win us any friends as it would be very 
disruptive. As others have noted the way we introduce and remove 
configure parameters needs to be looked at.
This makes it sound like we're sloppy with configure arguments when we 
actually try hard not to be.


Normally, we don't have any problems with introducing new configure 
arguments. Most often, when we introduce a new argument, the behaviour 
falls back to the old behaviour as default if the argument is not 
specified. From time to time, we actually need to change the behaviour 
of configure, and this does not really apply. But then again, all 
modification somehow change behaviour regardless of if any options are 
changed, an all changes in behaviour breaks someones workflow 
(compulsory xkcd reference: https://xkcd.com/1172/).


In this case, not setting the new configure option resulted in the old 
default behaviour.


As for removing options, we are more conservative. We never remove 
options, we just deprecate them. (With a few exceptions, e.g. sometimes 
we have introduced temporary options which are clearly announced as 
such.) Our plan is to remove deprecated options once some time has 
passed (e.g. next major release) but, like the Java language itself, so 
far we have not actually removed any deprecated options. :-)


When we deprecate an option, we ignore the value it sets, but prints a 
warning stating that the option is deprecated. The configure call will 
not fail, though. The warning is repeated as the very last thing so it 
should be easy for the user to spot, like this:


The following warnings were produced. Repeated here for convenience:
WARNING: Option --with-override-jaxp is deprecated and will be ignored.

In some cases we've tried to write some "glue" to interpret old and 
deprecated options in terms of new. (I only think we've done this in the 
closed source). I don't really like it. It means a lot of tests, 
handling situations like what if both old and new are set, and they 
conflict? Or if both old and new are set and they do *not* conflict? 
What if the behaviour has changed slightly, so it does not really match? 
When should we stop helping the user to convert from the old to the new?


I was mainly concerned that the out-of-the-box default behaviour was 
unchanged.


The stated goal of the patch was that out of the box behaviour should 
have been unchanged. Th

Re: Building jdk9 on Windows x64 and Visual Studio 2015 Communityedition

[David Holmes]

On 18/12/2015 10:37 PM, Magnus Ihse Bursie wrote:

On 2015-12-18 05:11, David Holmes wrote:

On 18/12/2015 3:28 AM, Kumar Srinivasan wrote:

I am good with this change!.

I would like the rest of the component teams to weigh in on the others.


This needs to be sent to all the affected component teams, or even
better split into three pieces: build, hotspot and JDK libs, and then
sent.


I sent the RFR (in another thread) to awt-dev
, hotspot-dev ,
core-libs-dev . I thought that was
enough. Do you suggest any more mailing lists?


Sorry didn't see that mail till much later - my mail is sorted by subject :)

Cheers,
David


/Magnus



Hotspot changes looked okay to me - though the need for casts in
various places was unpleasant - we may want neater ways to deal with
that.


Thanks,
David


Thanks
Kumar

On 12/16/2015 12:18 PM, Magnus Ihse Bursie wrote:

On 2015-12-16 16:33, Kumar Srinivasan wrote:

Hello,

http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01/jdk/src/jdk.pack200/share/native/common-unpack/utils.h.udiff.html



You are undefining Windows math.h OVERFLOW, what is it defined
as ? With you redefining this, will it cause problems for users of
this API, likely to affect JNI apps.

Probably need to redefine the pack200 OVERFLOW constant to
something else, probably PACK200_OVERFLOW to prevent namespace
collisions.


I agree, this is a better solution. I've updated the webrev with this
solution (although I used the name PSIZE_OVERFLOW to align with
PSIZE_MAX).

http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.02



/Magnus




Kumar

On 12/16/2015 5:35 AM, Magnus Ihse Bursie wrote:

On 2015-11-05 18:03, Timo Kinnunen wrote:
Hi,
  I have signed the OCA and emailed a scan according the
instructions. I separated the freetype changes into a separate
batch as suggested. I have shared the patch files on OneDrive, they
are my Public folder. Here’s the link to the folder:
https://onedrive.live.com/redir?resid=a243a3e0b2aaacfa%21107
  The OneDrive folder should contain these 4 files:
  freetype_JDK9.patch
vs2015_JDK9.patch
vs2015_JDK9_hotspot.patch
vs2015_JDK9_jdk.patch
  The first two target the root repository, the other two are for
hotspot and jdk repositories, respectively.
  I rebased the patches on JDK9 tip today. I ran “make
images” and fixed a couple of new errors that have appeared since
the previous version. A couple of the changes I had made were also
not needed any more.
  Please have a look!

Hi Timo,

I'm sorry for the long delay.

I have created JDK-8145548 for the freetype fix. I'm sponsoring this
fix. I'll send out a proper review on cr.openjdk.java.net shortly.

The vs2015 changes are more complicated since they touch multiple
places in the code. Also, your patch had started bitrotting slightly
during my long response time. I'm trying to fix it up, and will post
a review when I have sorted everything out. I will verify that the
change does not break any of our supported platforms, but I'd like
you to verify that the patch still works on VS2015. When I publish
the webrev, there will be a download link to the patch file.

Also, this patch touches both hotspot code and multiple jdk modules,
so it will need to be reviewed by other groups as well, besides the
build team.

/Magnus


Sent from Mail for Windows 10

From: Magnus Ihse Bursie
Sent: Friday, October 23, 2015 17:13
To: timo.kinnu...@gmail.com;build-dev
Subject: Re: Building jdk9 on Windows x64 and Visual Studio 2015
Communityedition
On 2015-09-25 17:55, timo.kinnu...@gmail.com wrote:

Hi,

I’ve been going over the Windows build of the whole JDK for a
while with VS 2015 and now I have patches that allow the build to
complete.

I’ve made changes in the root repository as well as in hotspot and
jdk repositories. The changes fall broadly in three categories:
enabling the v140 toolchain and improving freetype compilation,
adding casts to where pointers are truncated and miscellaneous
small-scale code changes.

The patch to the root repository streamlines handling of freetype
by implementing a default source directory at $HOME/freetype under
Cygwin. It is checked during configure and used for compiling if
“--with-freetype-src” is not specified. A help message giving the
unpacking command with the correct directory is also included.
This patch is about 90 lines without counting
generated-configure.sh changes.

The patches to jdk and hotspot contain native code changes only
and no changes to make-files. These are about 580 and 290 lines,
respectively. All patches are generated with “hg diff -g”.

Would you be willing to incorporate these? How should I proceed
with this?

  Hi Timo,
  First of all, I apologize that you have not recieved any response
for
far too long. :-(
  Thank you for your interest in helping to improve OpenJDK!
  In general, a patch to allow compilation on VS 2015
Community edition
sounds like a good edition to OpenJDK. I am willing to sponsor this
patch and

Build failure on windows 'freetypeScaler.c'

[Boaz Nahum]

Hi

Lately, I have this error:

f:/Dev/JDKBuild/valhalla/jdk/src/java.desktop/share/native/libfontmanager/freetypeScaler.c(106)
: error C2220: warning treated as error - no 'object' file generated
f:/Dev/JDKBuild/valhalla/jdk/src/java.desktop/share/native/libfontmanager/freetypeScaler.c(106)
: warning C4101: 'stream' : unreferenced local variable

I comment out:

static void freeNativeResources(JNIEnv *env, FTScalerInfo* scalerInfo) {
//void *stream;


And now it works.

Thx
Boaz

Re: Build failure on windows 'freetypeScaler.c'

[Magnus Ihse Bursie]

On 2015-12-18 14:16, Boaz Nahum wrote:

Hi

Lately, I have this error:

f:/Dev/JDKBuild/valhalla/jdk/src/java.desktop/share/native/libfontmanager/freetypeScaler.c(106)
: error C2220: warning treated as error - no 'object' file generated
f:/Dev/JDKBuild/valhalla/jdk/src/java.desktop/share/native/libfontmanager/freetypeScaler.c(106)
: warning C4101: 'stream' : unreferenced local variable

I comment out:

static void freeNativeResources(JNIEnv *env, FTScalerInfo* scalerInfo) {


This is a know problem which is fixed by JDK-8139803.

/Magnus

Re: RFR 4823133: RandomAccessFile.length() is not thread-safe

[Magnus Ihse Bursie]

On 2015-12-15 18:25, Martin Buchholz wrote:

_FILE_OFFSET_BITS is generally an all-or-nothing thing, because it
affects interoperability between translation units.  It would be good
to convert all of the JDK build to use -D_FILE_OFFSET_BITS=64, but
that would be a big job.  So traditionally the JDK has instead used
the functions made available via _LARGEFILE64_SOURCE.  But that is
also a JDK-wide job now, because every call to plain stat in the
source code is broken on 32-bit systems with 64-bit inodes, which are
becoming more common.

I recommend the _FILE_OFFSET_BITS=64 strategy, but it's probably a job
for build-dev, not core-libs-dev.


If I understand things correctly, then setting _FILE_OFFSET_BITS=64 
would mean two things:
1) Just pass this additional define to all C files being compiled (a 
build change)
2) Update all calls to file operating functions to make sure the proper 
64-bit types are used (changes in source code).


The change for 1) is a trivial one liner, while getting 2) correct is a 
serious undertaking in the source code. Such a change must be done by 
the component teams, not the build team. With that said, I think it is a 
reasonable change and I'll support it as much as I can. But as the 
division of labour is done here, it's not the work of the build team to 
go and make heavy modifications to the product source code.


/Magnus






On Tue, Dec 15, 2015 at 8:31 AM, Roger Riggs  wrote:

Hi Yvom,

Minor comments:

src/java.base/share/native/libjava/RandomAccessFile.c:
  - "length fail" might be clearer as "GetLength failed"

src/java.base/unix/native/libjava/io_util_md.c:

  - Please add a comment before the define of FILE_OFFSET_BITS to indicate
where it is used and why it is there.
  - BTW, are there any unintended side effects?
Perhaps a different issue but perhaps 64 bit offsets should be used
everywhere

src/java.base/windows/native/libjava/io_util_md.c
  - Line 592: Using INVALID_HANDLE_VALUE is better than -1 and is used
elsewhere in the file
BTW, Testing for invalid handle might be unnecessary since the call to
GetFileSizeEx will fail
if it is invalid, yielding the same result.

Roger


On 12/10/2015 5:52 AM, vyom wrote:

Hi All,

Please review my changes for below bug.

Bug: JDK-4823133 : RandomAccessFile.length() is not thread-safe

Webrev:http://cr.openjdk.java.net/~vtewari/4823133/webrev0.0/


This change ensure that  length() does not temporarily changes the file
pointer and it will make sure that there is no race
condition in case of multi thread uses.

Thanks,
Vyom

Re: RFR: JDK-8145549 Add support for Visual Studio 2015 Community edition

[Wang Weijun]

Hi Vinnie

I take a look and it includes a change for 
src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp in the 
Java_sun_security_mscapi_KeyStore_getKeyLength() function.

It seems there is no sun.security.mscapi.KeyStore#getKeyLength on the java 
side. Is the function useless now?

Thanks
Max

> On Dec 16, 2015, at 9:50 PM, Magnus Ihse Bursie 
>  wrote:
> 
> There is an interest from the community to build OpenJDK using Visual Studio 
> 2015 Community edition. 
> 
> This patch is provided by Timo Kinnunen . I am 
> sponsoring this patch.
> 
> The changes to the source code files are mostly casts to uintptr_t, but there 
> are some other changes as well.
> 
> I'm not quite sure who's the owner of all these files. If I'm missing some 
> group, please help me and forward the mail to them.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8145549
> WebRev: 
> http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01
> 
> /Magnus

Re: Build failure on windows 'freetypeScaler.c'

[Maurizio Cimadamore]

Fix pushed

Maurizio

On 18/12/15 13:16, Boaz Nahum wrote:

Hi

Lately, I have this error:

f:/Dev/JDKBuild/valhalla/jdk/src/java.desktop/share/native/libfontmanager/freetypeScaler.c(106)
: error C2220: warning treated as error - no 'object' file generated
f:/Dev/JDKBuild/valhalla/jdk/src/java.desktop/share/native/libfontmanager/freetypeScaler.c(106)
: warning C4101: 'stream' : unreferenced local variable

I comment out:

static void freeNativeResources(JNIEnv *env, FTScalerInfo* scalerInfo) {
//void *stream;


And now it works.

Thx
Boaz

Re: RFR: 8145132: Initial updates for ios x86_64 mobile/dev builds.

[Gary Adams]

On 12/18/15 07:34, Magnus Ihse Bursie wrote:

On 2015-12-17 20:40, Gary Adams wrote:

On 12/17/15 11:09, Magnus Ihse Bursie wrote:

On 2015-12-17 14:19, Gary Adams wrote:

I've revised the original webrev based on some feedback received.
   - reverted white space only changes
   - proper copyrights on the new files
   - some hotspot files contained previously removed code

  Webrev; http://cr.openjdk.java.net/~gadams/8145132/webrev.01/

Planning to push this first batch tomorrow.


Hi Gary,

There seems to be multiple merge/diff errors in this patch. I'm 
seeing several location where this patch contains changes that is 
part of other, recently pushed change sets. This makes it hard to 
fully understand what changes you are contributing in this patch, to 
speak nothing of the merge problems that are likely to arise if this 
patch were pushed as it is.
My bad. In getting the workspace to build I had diff'ed the files 
against the latest jdk9-dev sources
and pulled in changes that were ahead of the recently cloned 
mobile/dev repos.
The next merge will bring in those updates. mobile/dev is cloned from 
jdk9-b94 and latest jdk9/dev

is at jdk9-9+b96.

While reverting the code that had newer functionality than it should,
I found the issue that dragged me down the rabbit hole. The change
to use JAVA_JAVAC in the SetupCompilers logic was a thread that
unraveled into more code than I should have touched.  I'll double
check on the next resync to make sure everything is OK.




I found several other issues as well. I'm sorry I have not been able 
to review this code before. It's a quite massive patch. If you want 
to commit the patch as-is in the mobile/dev forest and then fix my 
comments later before pushing further to mobile/jdk9, it's ok for 
me. I understand that it's tricky to manage a patch of this size. 
(But I think it's better to fix issues now, if you ask me...)
I'd like to do any simple cleanups before the initial push. We know 
there will be more pushes coming, so I'm not opposed to partial solutions

being put back at this time.


I'll start with something that you and Erik already has discussed: 
how to express tests for logic that is common to ios and macosx. 
There are places in the patch right now that I'm still not happy with.


First of all, I don't think you need to be shy of testing for macosx 
or ios. It's a bit more to write, but it's very clear to the reader, 
and code is read more often than written.
I am uncomfortable with all the places platform specific changes need 
to be made.
There isn't an easy way to express platform A is like platform B with 
the following

differences.


I agree. Platforms are alike and different in very amorphous ways. 
We've tried to capture some of this using the concept of "os type" 
(that is, to group unix-like operating systems) and "os env" (to 
differentiate the unix-emulation layers on windows). It works sort-of, 
but it's not perfect. For instance, in some respects macosx is indeed 
"unix", but in many other (GUI etc), it's different. We could of 
course try to create some kind of category "unix-except-macosx" for 
that, but in the end I don't think having 
src/java.base/unix-but-not-macosx/ would fly. :-)


So this is a tricky problem, and I believe it can only be "solved" in 
terms of getting a "best effort" solution for the platforms and 
combinations we currently support. When that support matrix changes 
too much (as with mobile), we might have to revise our model. Let's 
keep an open discussion on the best way to solve this for mobile. For 
now, though, I think the best way is to try and use the current 
framework as far as possible. Then we will learn what the actual 
limitations are and where they need to be fixed.

OK




Second, I see you introduce a OPENJDK_TARGET_OS_ENV=macosx for ios. 
It's a bit strange, since it changes the meaning of the OS_ENV 
(previously it was only used to differentiate between cygwin and 
msys in Windows), but I think it could be a reasonable modification. 
This means that you can test if OPENJDK_TARGET_OS_ENV == macosx to 
cover both the case of macosx and ios. Let's just assume that you 
need to know what you're doing when looking at OS_ENV. So this could 
be an alternative to a lot of "if target == macosx or target == 
ios", if you don't want to type that everywhere. For android it 
seems less of a point of setting OS_ENV. Or do you think you could 
unify android/linux code by this?
I thought previously we were told not to use OPENJDK_TARGET_OS_ENV 
and I asked at that time for "documentation".


You probably were. :) And it is perhaps best that you don't. :-) In 
any way, I don't see any references to the OS_ENV in this code anyway, 
so given that, you should remove this.


Removed OS_ENV settings for both andrioid and ios from platform.m4.


What I think you need is, as I understand it, rather to express some 
"os kind", that is, saying that android is "a kind of" linux and ios 
is "a kind of" macosx, so that 

Follow up bugs from the initial review comments

[Gary Adams]

I deferred a number of changes in order to get the first set of sources 
out to the mobile/dev repos.
Just so we don't lose sight of those changes, I've filed a few bugs that 
can be addressed independently

going forward.

 JDK-8145802: CPPFLAGS sysroot support
 JDK-8145804: ARFLAGS versus AR_OUT_OPTION conflicts in static builds
 JDK-8145806: OPENJDK_TARGET_CPU_LEGACY_LIB build variable will be retired
 JDK-8145807: JavaLauncher should use standard naming and build constructs
 JDK-8145809: Demo projects for HelloWorld for ios and android
 JDK-8145811: Fix compiler warnings

Re: RFR 4823133: RandomAccessFile.length() is not thread-safe

[Martin Buchholz]

On Fri, Dec 18, 2015 at 5:35 AM, Magnus Ihse Bursie
 wrote:
> On 2015-12-15 18:25, Martin Buchholz wrote:
>>
>> _FILE_OFFSET_BITS is generally an all-or-nothing thing, because it
>> affects interoperability between translation units.  It would be good
>> to convert all of the JDK build to use -D_FILE_OFFSET_BITS=64, but
>> that would be a big job.  So traditionally the JDK has instead used
>> the functions made available via _LARGEFILE64_SOURCE.  But that is
>> also a JDK-wide job now, because every call to plain stat in the
>> source code is broken on 32-bit systems with 64-bit inodes, which are
>> becoming more common.
>>
>> I recommend the _FILE_OFFSET_BITS=64 strategy, but it's probably a job
>> for build-dev, not core-libs-dev.
>
>
> If I understand things correctly, then setting _FILE_OFFSET_BITS=64 would
> mean two things:
> 1) Just pass this additional define to all C files being compiled (a build
> change)
> 2) Update all calls to file operating functions to make sure the proper
> 64-bit types are used (changes in source code).
>
> The change for 1) is a trivial one liner, while getting 2) correct is a
> serious undertaking in the source code.

If everyone is properly pairing up their function calls with proper
types (i.e. using off_t consistently), then everything should Just
Work.

Such a change must be done by the
> component teams, not the build team.

I think this is an openjdk cultural problem - changes like this can
and should be done by one person, with help from component teams where
necessary.  It's safer to add _FILE_OFFSET_BITS=64 everywhere at once
(although hotspot and libraries can probably be cleanly separated) to
avoid type size disagreement between translation units.

For bonus points, we can later change all of those *64 functions back
to their standard equivalents.

With that said, I think it is a
> reasonable change and I'll support it as much as I can. But as the division
> of labour is done here, it's not the work of the build team to go and make
> heavy modifications to the product source code.

Re: RFR: 8145132: Initial updates for ios x86_64 mobile/dev builds.

[Thomas Stüfe]

Hi Gary,

In the Linux os layer - especially in os_linux.cpp - I see you have
reintroduced old coding we just painstakingly removed because it was not
needed for modern Linuxes anymore. For instance, coding dealing with old
Linuxthreads (vs Nptl, which is ubiquitous nowadays) or workarounds for old
glibc bugs. Is this really needed for Android? Or is this just a merge
error?

Kind regards, Thomas
On Dec 11, 2015 16:15, "Gary Adams"  wrote:

> Here's the initial upload of changes that provides support for the ios and
> android ports
> for the mobile/dev repos. While there have been some preliminary reviews
> of the code,
> there is still more work required before we will look for more thorough
> reviews
> and an integration to mobile/jdk9 repos.
>
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8145132
>   Webrev: http://cr.openjdk.java.net/~gadams/8145132/webrev.00/
>
>
> Here's a simple configure script to generate a ios-x86_64 build for use
> with the iphone simulator. (uses homebrew 64 bit freetype from pkgconfig)
>
> export JAVA_HOME=`/usr/libexec/java_home -v 1.8`
> export PATH=$JAVA_HOME/bin:~/homebrew/bin:$PATH
>
> bash ../../configure \
> --openjdk-target=x86_64-macos-ios \
> --with-boot-jdk=$JAVA_HOME \
> --disable-warnings-as-errors \
> --disable-headful \
> --enable-static-build=yes \
> --with-jvm-variants=minimal1
>
>
> Also, tested with i586-macos-ios target for 32 bit
> with a locally built --with-freetype 2.6.2.
>

Re: RFR: 8145132: Initial updates for ios x86_64 mobile/dev builds.

[Bob Vandette]

I caught that and gave Gary an updated set of os_linux* files that hopefully 
corrected this.

Here’s the updated changes that Gary just pushed today.

http://hg.openjdk.java.net/mobile/dev/hotspot/rev/f1acc8fa34b8


Bob.

> On Dec 18, 2015, at 4:11 PM, Thomas Stüfe  wrote:
> 
> Hi Gary,
> 
> In the Linux os layer - especially in os_linux.cpp - I see you have
> reintroduced old coding we just painstakingly removed because it was not
> needed for modern Linuxes anymore. For instance, coding dealing with old
> Linuxthreads (vs Nptl, which is ubiquitous nowadays) or workarounds for old
> glibc bugs. Is this really needed for Android? Or is this just a merge
> error?
> 
> Kind regards, Thomas
> On Dec 11, 2015 16:15, "Gary Adams"  wrote:
> 
>> Here's the initial upload of changes that provides support for the ios and
>> android ports
>> for the mobile/dev repos. While there have been some preliminary reviews
>> of the code,
>> there is still more work required before we will look for more thorough
>> reviews
>> and an integration to mobile/jdk9 repos.
>> 
>>  Issue: https://bugs.openjdk.java.net/browse/JDK-8145132
>>  Webrev: http://cr.openjdk.java.net/~gadams/8145132/webrev.00/
>> 
>> 
>> Here's a simple configure script to generate a ios-x86_64 build for use
>> with the iphone simulator. (uses homebrew 64 bit freetype from pkgconfig)
>> 
>> export JAVA_HOME=`/usr/libexec/java_home -v 1.8`
>> export PATH=$JAVA_HOME/bin:~/homebrew/bin:$PATH
>> 
>> bash ../../configure \
>>--openjdk-target=x86_64-macos-ios \
>>--with-boot-jdk=$JAVA_HOME \
>>--disable-warnings-as-errors \
>>--disable-headful \
>>--enable-static-build=yes \
>>--with-jvm-variants=minimal1
>> 
>> 
>> Also, tested with i586-macos-ios target for 32 bit
>> with a locally built --with-freetype 2.6.2.
>> 

Re: RFR: JDK-8145549 Add support for Visual Studio 2015 Community edition

[Kim Barrett]

On Dec 16, 2015, at 8:50 AM, Magnus Ihse Bursie  
wrote:
> 
> There is an interest from the community to build OpenJDK using Visual Studio 
> 2015 Community edition. 
> 
> This patch is provided by Timo Kinnunen . I am 
> sponsoring this patch.
> 
> The changes to the source code files are mostly casts to uintptr_t, but there 
> are some other changes as well.
> 
> I'm not quite sure who's the owner of all these files. If I'm missing some 
> group, please help me and forward the mail to them.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8145549
> WebRev: 
> http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01
> 
> /Magnus

I only looked at hotspot files.

Breaking this up into smaller and more focused chunks would be nice.
Large changes that do 17 different things are hard to deal with.

I think the various casts to uintptr_t (commented with "What???"
below) should be done differently.  They are addressing problems of
differences between pointer size and various integer sizes, and
conversions between them.  I think in some cases there is a poorly
chosen surrounding type involved.  For the 0xdeadbeef cases, there
probably ought to be a centralized helper for that sort of thing,
since doing it properly on across different platforms might be tricky,
and that trickiness ought to be in one place.

There are a couple of "TBD"s below that I need to spend more time on.

--
Sprinkling lots of files with the following seems like a bad idea.

#if _MSC_VER >= 1900
#pragma warning( disable : 4091 ) // typedef ignored on left when no variable 
is declared
#endif

Especially since these warnings may indicate actual bugs.
https://msdn.microsoft.com/en-us/library/eehkcz60.aspx

-- 
hotspot/src/share/vm/utilities/globalDefinitions.hpp
1062 #define   badAddress((address)(intptr_t)::badAddressVal)

Change the type of badAddressVal to intptr_t instead.

-- 
hotspot/src/share/vm/runtime/os.cpp
 127 #elif defined(_WIN32) && _MSC_VER > 1800
 128   const time_t zone = _timezone;

Why does (recent) Windows need special handling here?

-- 
hotspot/src/share/vm/utilities/globalDefinitions_visCPP.hpp
 198 #if _MSC_VER <= 1800

Shouldn't that be "<" the version that introduced the missing
function?

-- 
hotspot/src/share/vm/prims/whitebox.cpp
1286   return (jlong)(uintptr_t)ikh->constants();

What???

--
hotspot/src/share/vm/opto/type.cpp
  53   { Bad, T_ILLEGAL,"bad",   false, 
(int)Node::NotAMachineReg, relocInfo::none  },  // Bad
  61   { Bad, T_ILLEGAL,"tuple:",false, 
(int)Node::NotAMachineReg, relocInfo::none  },  // Tuple
  62   { Bad, T_ARRAY,  "array:",false, 
(int)Node::NotAMachineReg, relocInfo::none  },  // Array

These are narrowing conversions in brace initializers, which are
forbidden by C++11.  The type of the member is int, while the
initialization expressions are non-literal uint.

In my opinion, it would be better to figure out how to fix the type
mismatch than to sprinkle with casts.

A way to look for these that doesn't require VS2015 would be to use
g++ -Wnarrowing.

-- 
hotspot/src/share/vm/opto/split_if.cpp
 258   Node *prior_n = (Node*)(uintptr_t)0xdeadbeef;
 305   prior_n = (Node*)(uintptr_t)0xdeadbeef;  // Reset IDOM walk

What???

-- 
hotspot/src/share/vm/opto/gcm.cpp
1448   _node_latency = (GrowableArray *)(uintptr_t)0xdeadbeef;

What???

-- 
hotspot/src/share/vm/opto/compile.cpp
2398   _cfg = (PhaseCFG*)(uintptr_t)0xdeadbeef;
2399   _regalloc = (PhaseChaitin*)(uintptr_t)0xdeadbeef;

What???

--
hotspot/src/share/vm/opto/buildOopMap.cpp
 618 Block *pred = (Block*)(uintptr_t)0xdeadbeef;

What???

-- 
hotspot/src/share/vm/oops/typeArrayOop.hpp
 132 *long_at_addr(which) = (jlong)contents;

Yes!  Fortunately, it looks like metadata_at_put is never called.

-- 
hotspot/src/share/vm/memory/allocation.hpp
  38 #if defined _WINDOWS && _MSC_VER >= 1900
  39  // 'noexcept' used with no exception handling mode specified; termination 
on exception is not guaranteed. Specify 

Re: RFR: 8145132: Initial updates for ios x86_64 mobile/dev builds.

[Thomas Stüfe]

Hi Bob and Gary,

Thank you for correcting this! Respect also on this large porting effort,
this looks like a lot of work and I am curious how this will turn out.

I took a short look at your changes - especially the android-related
changes  - and have a number of further questions and suggestions . I hope
that this is okay and the door is not already closed.

I'm away from any computer however (ironically typing this from an android
device, so anything weird is due to auto correct :), so before Monday I
will not be able to do anything.

I also think that this may be interesting to hotspot-runtime, because the
changes to the OS layer are somewhat extensive, so I'll take the freedom
and put them on the recipient list.

Kind regards, Thomas
On Dec 18, 2015 22:16, "Bob Vandette"  wrote:

> I caught that and gave Gary an updated set of os_linux* files that
> hopefully corrected this.
>
> Here’s the updated changes that Gary just pushed today.
>
> http://hg.openjdk.java.net/mobile/dev/hotspot/rev/f1acc8fa34b8
>
>
> Bob.
>
> > On Dec 18, 2015, at 4:11 PM, Thomas Stüfe 
> wrote:
> >
> > Hi Gary,
> >
> > In the Linux os layer - especially in os_linux.cpp - I see you have
> > reintroduced old coding we just painstakingly removed because it was not
> > needed for modern Linuxes anymore. For instance, coding dealing with old
> > Linuxthreads (vs Nptl, which is ubiquitous nowadays) or workarounds for
> old
> > glibc bugs. Is this really needed for Android? Or is this just a merge
> > error?
> >
> > Kind regards, Thomas
> > On Dec 11, 2015 16:15, "Gary Adams"  wrote:
> >
> >> Here's the initial upload of changes that provides support for the ios
> and
> >> android ports
> >> for the mobile/dev repos. While there have been some preliminary reviews
> >> of the code,
> >> there is still more work required before we will look for more thorough
> >> reviews
> >> and an integration to mobile/jdk9 repos.
> >>
> >>  Issue: https://bugs.openjdk.java.net/browse/JDK-8145132
> >>  Webrev: http://cr.openjdk.java.net/~gadams/8145132/webrev.00/
> >>
> >>
> >> Here's a simple configure script to generate a ios-x86_64 build for use
> >> with the iphone simulator. (uses homebrew 64 bit freetype from
> pkgconfig)
> >>
> >> export JAVA_HOME=`/usr/libexec/java_home -v 1.8`
> >> export PATH=$JAVA_HOME/bin:~/homebrew/bin:$PATH
> >>
> >> bash ../../configure \
> >>--openjdk-target=x86_64-macos-ios \
> >>--with-boot-jdk=$JAVA_HOME \
> >>--disable-warnings-as-errors \
> >>--disable-headful \
> >>--enable-static-build=yes \
> >>--with-jvm-variants=minimal1
> >>
> >>
> >> Also, tested with i586-macos-ios target for 32 bit
> >> with a locally built --with-freetype 2.6.2.
> >>
>
>