Re: gcc, arm, and thumbs mode

[Thomas Stüfe]

Hi Anton,

thanks for looking into this.

For my specific problem, I can and probably should use .function. But my
question was more general, should we leave the decision whether to use
thumb or arm up to the toolchain. For two reasons, one is executable size -
I assume using thumb has certain advantages, executables are smaller and
you can fit more into the instruction cache, and second, because errors
like mine are probably not that uncommon and it makes me nervous that we
only caught it because someone happened to build on his Raspi.

Cheers, Thomas

On Fri, May 27, 2022 at 3:28 PM Anton Kozlov  wrote:

> Hi Thomas,
>
> On 5/27/22 16:12, Thomas Stüfe wrote:
> > P.S. I found one possible solution for my particular problem was to add
> > `.type function` to the static assembler routine. That caused gcc to use
> > the correct jump instruction (blx instead of bl). But I am not sure this
> is
> > the best solution, maybe best would be to just use the same mode for all
> > hotspot compilation units.
>
> AFAIR, that .type %function directive is a correct way to write asm
> code. At least this is what gcc generates for the C code [1]. I'm not
> sure how the annotation in the assembly code affects the caller code,
> may be link time optimization? But if adding the directive resolves the
> issue, I vote for it.
>
> (I expect arm-none crosscompiler to produce similar results compared to
> arm-linux target)
>
> Thanks,
> Anton
>
> $ echo "int main() { return 0; }" | arm-none-eabi-gcc -mthumb -S -x c - -o
> -
> .cpu arm7tdmi
> .arch armv4t
> .fpu softvfp
> .eabi_attribute 20, 1
> .eabi_attribute 21, 1
> .eabi_attribute 23, 3
> .eabi_attribute 24, 1
> .eabi_attribute 25, 1
> .eabi_attribute 26, 1
> .eabi_attribute 30, 6
> .eabi_attribute 34, 0
> .eabi_attribute 18, 4
> .file   ""
> .text
> .align  1
> .global main
> .syntax unified
> .code   16
> .thumb_func
> .type   main, %function
> main:
> @ Function supports interworking.
> @ args = 0, pretend = 0, frame = 0
> @ frame_needed = 1, uses_anonymous_args = 0
> push{r7, lr}
> add r7, sp, #0
> movsr3, #0
> movsr0, r3
> mov sp, r7
> @ sp needed
> pop {r7}
> pop {r1}
> bx  r1
> .size   main, .-main
> .ident  "GCC: (Arch Repository) 12.1.0"
>
>

Re: gcc, arm, and thumbs mode

[Thomas Stüfe]

(cited the wrong JBS issue, the correct one is
https://bugs.openjdk.java.net/browse/JDK-8284997)

On Fri, May 27, 2022 at 3:12 PM Thomas Stüfe 
wrote:

> Hi,
>
> I am investigating the arm-specific problem
> https://bugs.openjdk.java.net/browse/JDK-8283326. It looks like the
> error is caused by arm- and thumb-code interleaving: GCC-compiled code, in
> thumb mode, calls into a static assembler subroutine that has been compiled
> into arm mode, but the caller uses the wrong call instruction and we call
> into SafeFetch without switching into arm mode (gcc-generated code uses bl
> instead of blx).
>
> This problem only happens if the OpenJDK was built with a GCC that itself
> was built with --with-mode=thumb. Without that option, GCC defaults to arm
> code generation, and that hides the error I describe above.
>
> My question is: Is this by design? It seems strange to leave this decision
> up to whoever built the toolchain. Should we not fix the arm/thumb decision
> at build time with either one of -mthumb or -marm?
>
> Thanks, Thomas
>
> P.S. I found one possible solution for my particular problem was to add
> `.type function` to the static assembler routine. That caused gcc to use
> the correct jump instruction (blx instead of bl). But I am not sure this is
> the best solution, maybe best would be to just use the same mode for all
> hotspot compilation units.
>

gcc, arm, and thumbs mode

[Thomas Stüfe]

Hi,

I am investigating the arm-specific problem
https://bugs.openjdk.java.net/browse/JDK-8283326. It looks like the
error is caused by arm- and thumb-code interleaving: GCC-compiled code, in
thumb mode, calls into a static assembler subroutine that has been compiled
into arm mode, but the caller uses the wrong call instruction and we call
into SafeFetch without switching into arm mode (gcc-generated code uses bl
instead of blx).

This problem only happens if the OpenJDK was built with a GCC that itself
was built with --with-mode=thumb. Without that option, GCC defaults to arm
code generation, and that hides the error I describe above.

My question is: Is this by design? It seems strange to leave this decision
up to whoever built the toolchain. Should we not fix the arm/thumb decision
at build time with either one of -mthumb or -marm?

Thanks, Thomas

P.S. I found one possible solution for my particular problem was to add
`.type function` to the static assembler routine. That caused gcc to use
the correct jump instruction (blx instead of bl). But I am not sure this is
the best solution, maybe best would be to just use the same mode for all
hotspot compilation units.

Re: RFR: 8284752: Zero does not build on Mac OS X due to missing os::current_thread_enable_wx implementation

[Thomas Stüfe]

Hi Magnus,

On Wed, Apr 13, 2022 at 1:23 PM Magnus Ihse Bursie <
magnus.ihse.bur...@oracle.com> wrote:

> On 2022-04-12 15:21, David Holmes wrote:
> > On Tue, 12 Apr 2022 11:31:03 GMT, Johannes Bechberger <
> d...@openjdk.java.net> wrote:
> >
> >> Adds an implementation of the missing method (guarded with
> `defined(AARCH64) && defined(__APPLE__)`) and fixes the compilation issues
> on Mac M1.
> > Looks good and trivial.
> >
> > It seems apparent that nobody has been building Zero in this environment.
> I'm a bit surprised at this. I thought macos/aarch64 was added to GHA
> some time ago? (And also to Oracle's internal CI pipeline)
>
>
GHAs build Linux Zero (hotspot only). I think nobody builds MacOs zero, or?
This was a MacOs specific bug (still good to have that fixed though).


> /Magnus
>
>
>

Re: Failing jdk11u AIX build on Adoptium servers

[Thomas Stüfe]

Hi Matthias,

On Thu, Mar 24, 2022 at 4:55 PM Baesken, Matthias 
wrote:

> Hi ,
>
>
>
> >My expected course of action is as follows:
> >- Bump the minimum requirements in our Supported Builds document
> >[https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms]
> >- Open an issue with the Adoptium group to change their build for jdk11
> and above to use the version 16 compiler.
>
> Are the xlc16 – related changes already all in jdk11u-dev now ?
>
>
>
> >probably we should change the minimum requirement of jdk12 too then.
>
>
>
> JDK12 does not have the harfbuzz 2.8.0  backport so probably no need to
> change the Supported Builds document.
>
>
oh, you are right. I just thought that it would be strange if jdk11 had a
newer compiler requirement than jdk12. But probably nobody cares for jdk12
on AIX.

Thanks, Thomas


>
>
> Best regards, Matthias
>
>
>
>

Re: Failing jdk11u AIX build on Adoptium servers

[Thomas Stüfe]

cc build-dev.

Hi Tyler, makes sense to me. Wrt the documentation, probably we should
change the minimum requirement of jdk12 too then.

Cheers, Thomas





On Thu, Mar 24, 2022 at 4:05 PM Tyler Steele  wrote:

> Hi all,
>
> In regards to the new Harfbuzz version bump (and the work done to prepare
> for it). And of special importance now that
> the Adoptium build of jdk11u-dev is
> failing [
> https://ci.adoptopenjdk.net/job/AIX-jdk11-dev-build/17/execution/node/51/log/],
> I wanted to suggest a course of
> action to the AIX-folks here to get suggestions before I make any changes.
>
> Early this week, I spent some time looking into this issue. My assessment
> is that AIX builds for jdk versions 11 and
> greater will need the at least xlc version 16.1 to support the Harfbuzz
> change. This is because full support for C++11
> with xlc did not arrive on AIX until that version (little-endian support
> was complete in 13.1).
>
> For reference:
> "IBM XL C++ for AIX
>  - Core language support status: C++11 partial in 13.1.3 and 16.1.0 (xlC
> frontend), complete in 16.1.0 (xlclang
> frontend)" [source: https://en.cppreference.com/w/cpp/compiler_support ]
>
> My expected course of action is as follows:
> - Bump the minimum requirements in our Supported Builds document
> [https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms]
> - Open an issue with the Adoptium group to change their build for jdk11
> and above to use the version 16 compiler.
>
> As I am still relatively new to AIX and OpenJDK development, so I value
> any suggestions for improvements, omissions you
> may have noticed, or generally other comments the team here may have.
>
> Tyler
>

Re: Supporting alternative toolchains on Windows

[Thomas Stüfe]

As Dalibor wrote, I would not expect too many performance surprises.

That said, a more pragmatic approach may be to create a shim layer for
visual studio compiler and linker, e.g. a fake "cl.exe" and "link.exe" that
translate VC++ options and paths to whatever toolchain you like. That way
you don't have to touch the OpenJDK make at all. I know it works in
principle since I have such a thing in the past, albeit for a different
product and a different target toolchain.

I would not be surprised if such a thing exists already. It seems such an
obvious idea.

Cheers, Thomas

On Fri, Mar 11, 2022 at 2:35 PM Julian Waters 
wrote:

> Darn, seems like it'll be much harder than I expected. Since multiple
> toolchains are supported for macOS and Linux, I assumed a slight patch
> would help get it to work on Windows. Looking through the stuff in make
> though, it appears a lot of the build system implicitly expects the
> compiler for Windows to always be Visual C++, which doesn't really help
> that much (Though the fact that we can exclude many versions of gcc, such
> as Cygwin's and old MinGW binaries helps a lot). The build process for the
> newer Windows ports of gcc are surprisingly similar to Visual C++ though
> (Eg rc can be swapped out for windres) so this might hopefully be something
> I can try exploring in the future (Gonna look a bit harder at make and
> write what I can find back to this mailing list in the meantime). It'd be
> interesting if benchmarks of the JVM compiled with different compilers on
> Windows can be compared side by side on the off chance this becomes a
> reality though
>
> best regards,
> Julian
>
> On Fri, Mar 11, 2022 at 9:16 PM Magnus Ihse Bursie <
> magnus.ihse.bur...@oracle.com> wrote:
>
> > On 2022-03-11 12:55, Julian Waters wrote:
> >
> > > Hi all,
> > >
> > > How feasible would it be/much effort would it require to support
> > compiling
> > > with alternate toolchains on Windows besides Visual C++ (like the
> Windows
> > > ports of clang and gcc) if we restrict the allowed toolchains to only
> > those
> > > that link against the ucrt? (Toolchains linking against the dated
> msvcrt
> > > would present too many issues to work with)
> >
> > That'd be a huge undertaking. And any such patch would only be accepted
> > into the code base if the organization behinded appeared trustworthy in
> > their long-term commitment to keeping it working.
> >
> > /Magnus
> >
> >
> >
>

Re: JDK-8282532

[Thomas Stüfe]

Hi Jules,

On Wed, Mar 2, 2022 at 1:29 PM Jules W.  wrote:

> Apologies for not creating a thread consulting the mailing list before
> submitting the PR, I'm still getting used to the process (And also thought
> this would create unnecessary noise for people within this list).
>

On the contrary, this is very much the encouraged behavior. It's never
wrong to ask around before doing a patch. Someone may already be doing it,
or people have different notions about how to solve the problem. Or whether
it is a problem to begin with.

And if you don't get answers, nobody cares enough and it is okay to proceed
with a PR.


>
> I haven't changed the html version of the documentation yet as I haven't
> really finished making changes to the markdown version, I do apologize for

the formatting errors though.
>
>
Do you refer to something you attached? Note that attachments are scraped
from mails in the ML.

Cheers, Thomas

Re: GHAs and precompiled headers

[Thomas Stüfe]

On Mon, Jan 24, 2022 at 10:31 AM Aleksey Shipilev  wrote:

> On 1/24/22 10:26 AM, Thomas Stüfe wrote:
> >
> >
> > On Mon, Jan 24, 2022 at 10:15 AM Aleksey Shipilev  <mailto:sh...@redhat.com>> wrote:
> >
> > On 1/24/22 8:43 AM, Thomas Stüfe wrote:
> >  > We generally build without --disable-precompiled-headers in GHAs,
> which
> >  > hides errors from missing includes. Since GHAs are very useful to
> test
> >  > builds on side platforms, would it not make sense to build without
> >  > precompiled headers? Or is that too costly?
> >
> > But... We *do* build Hotspot without PCH in "additional"
> configurations that test different
> > platforms:
> >
> >
> https://github.com/openjdk/jdk/blob/9bf6ffa19f1ea9efcadb3396d921305c9ec0b1d1/.github/workflows/submit.yml#L408-L432
> > <
> https://github.com/openjdk/jdk/blob/9bf6ffa19f1ea9efcadb3396d921305c9ec0b1d1/.github/workflows/submit.yml#L408-L432
> >
> >
> > Yes, but only Linux. Leaves out macOS and Windows. For people who mainly
> use Linux and rely on GHA
>
> Ah.
>
> The last I checked, dropping PCH on Windows made build times suffer a lot.
> Given how underpowered
> are the Windows and MacOS build nodes in GHA, I'd rather accept a small
> possibility of non-PCH
> breakage than slowness of generic GHA workflows.
>
>
Pity, but I guess it cannot be avoided. It hit me this weekend, but at
least I know now the GHA limitations.

Thanks, Thomas

Re: GHAs and precompiled headers

[Thomas Stüfe]

On Mon, Jan 24, 2022 at 10:15 AM Aleksey Shipilev  wrote:

> On 1/24/22 8:43 AM, Thomas Stüfe wrote:
> > We generally build without --disable-precompiled-headers in GHAs, which
> > hides errors from missing includes. Since GHAs are very useful to test
> > builds on side platforms, would it not make sense to build without
> > precompiled headers? Or is that too costly?
>
> But... We *do* build Hotspot without PCH in "additional" configurations
> that test different platforms:
>
>
> https://github.com/openjdk/jdk/blob/9bf6ffa19f1ea9efcadb3396d921305c9ec0b1d1/.github/workflows/submit.yml#L408-L432
>
>
Yes, but only Linux. Leaves out macOS and Windows. For people who mainly
use Linux and rely on GHA not to break the other two this is inconvenient.

..Thomas

GHAs and precompiled headers

[Thomas Stüfe]

Hi,

We generally build without --disable-precompiled-headers in GHAs, which
hides errors from missing includes. Since GHAs are very useful to test
builds on side platforms, would it not make sense to build without
precompiled headers? Or is that too costly?

Thanks, Thomas

Re: gtest test targets fail with "Unknown test selection: 'gtest'" on jdk15-17

[Thomas Stüfe]

Hi Abigail,

at some point (it may have been JDK 15, I am not sure atm) we stopped
including the google test framework in our repo and instead require the
builder to provide it externally. See configure option "--with-gtest". Must
point to a copy of the googletest framework (
https://github.com/google/googletest).

HTH, Cheers, Thomas

On Sun, Jan 16, 2022 at 9:01 PM Abigail G  wrote:

> Hello,
>
> I am attempting to run tests for openjdk15 to 17 using the make target
> test-hotspot-gtest, but after compiling the test files, it fails with
> this message:
>
>Unknown test selection: 'gtest'
>See doc/testing.[md|html] for help
>RunTests.gmk:541: *** Cannot continue.  Stop.
>make[2]: *** [make/Main.gmk:601: exploded-test-gtest] Error 2
>
>ERROR: Build failed for target 'test-hotspot-gtest' in configuration
>'linux-x86_64-server-release' (exit code 2)
>
>No indication of failed target found.
>Hint: Try searching the build log for '] Error'.
>Hint: See doc/building.html#troubleshooting for assistance.
>
>make[1]: *** [/builddir/jdk15u-jdk-15.0.3+3/make/Init.gmk:315: main]
>Error 2
>make: *** [/builddir/jdk15u-jdk-15.0.3+3/make/Init.gmk:186: test-
>hotspot-gtest] Error 2
>
> I have been able to successfully compile and run the tests for
> openjdk12 to 14 with the same test target, and, looking at the
> makefiles, it appears there has been no change in the relevant code
> between jdk14 and 15. What could be the issue here?
>
> Thanks in advance,
> Abigail G
>
>

Re: Does CDS archive generation work for crossbuilds?

[Thomas Stüfe]

Hi Alan,

On Sat, Oct 23, 2021 at 9:58 AM Alan Bateman 
wrote:

>
>
> On 23/10/2021 07:57, Thomas Stüfe wrote:
> > Hi,
> >
> > when I crossbuild (for linux aarch64, using a devkit, building on linux
> > x64), for some reason I don't
> > get the classes.jsa generated inside the images directory.
> >
> > My configure options:
> >
> >
> --with-devkit=/shared/projects/openjdk/devkits/x86_64-linux-gnu-to-aarch64-linux-gnu
> > --openjdk-target=aarch64-linux-gnu
> > --with-boot-jdk=/shared/projects/openjdk/jdks/sapmachine17
> >
> --with-build-jdk=/shared/projects/openjdk/jdk-jdk/output-release/images/jdk
> > --with-gtest=/shared/projects/openjdk/gtest/googletest
> > --with-debug-level=fastdebug
> >
> > The build jdk is a freshly build x64 release VM from the same source
> tree.
> >
> > Am I missing something obvious? Is CDS archive generation even supported
> > for crossbuilds?
> It needs the generate run-time to execute "java -Xshare:dump" so I don't
> expect so. hotspot-runtime-dev is probably the place to discuss the
> details. BTW: this came up recently in the context of the jlink plugin
> that generates the CDS archive. The plugin needed a check to ensure that
> the target platform matched the current platform as it could launch the
> target VM to create the dump.
>
>
Thinking for a second, probably it cannot work since we copy binary
structures verbatim to the archive; I guess the chance that they are binary
compatible between platforms is very small. But it should be easily
rectified by calling Xshare:dump on the target platform.

Thank you!

..Thomas


> -Alan
>

Does CDS archive generation work for crossbuilds?

[Thomas Stüfe]

Hi,

when I crossbuild (for linux aarch64, using a devkit, building on linux
x64), for some reason I don't
get the classes.jsa generated inside the images directory.

My configure options:

--with-devkit=/shared/projects/openjdk/devkits/x86_64-linux-gnu-to-aarch64-linux-gnu
--openjdk-target=aarch64-linux-gnu
--with-boot-jdk=/shared/projects/openjdk/jdks/sapmachine17
--with-build-jdk=/shared/projects/openjdk/jdk-jdk/output-release/images/jdk
--with-gtest=/shared/projects/openjdk/gtest/googletest
--with-debug-level=fastdebug

The build jdk is a freshly build x64 release VM from the same source tree.

Am I missing something obvious? Is CDS archive generation even supported
for crossbuilds?

Thanks, Thomas

Re: CMake replacing Autotools?

[Thomas Stüfe]

On Fri, Mar 19, 2021 at 2:39 PM  wrote:

> On 2021-03-19 03:14, Thomas Stüfe wrote:
> > On Fri, Mar 19, 2021 at 11:04 AM Andrew Haley  wrote:
> >
> >> On 3/19/21 9:22 AM, Thomas Stüfe wrote:
> >>>> 2. More choices to actually build the project: Use integrated build
> >>>> tools of IDEs (Visual Studio, Xcode) or use Ninja, which is faster
> than
> >>>> gmake
> >>>>
> >>> Is gmake really where we lose time? Did you analyze this or is this
> just
> >> an
> >>> assumption? I would have thought it's things like single threaded jmod,
> >>> jlink, and subprocess spawning.
> >> I'm sure it is. The other slow thing is linking HotSpot.
> >>
> > What is so slow with gmake? Rule processing?
> >
> > It also depends on the platform, I guess. Eg on Cygwin, the fork
> emulation
> > is extremely slow.
> >
> I have done pretty extensive work optimizing our build's performance
> over the years. There are many ways to measure performance. First we
> need to establish what kind of build we are even measuring.
>
> For a full images build ("make images"), on a reasonably sized machine
> (8-16 HV threads), we scale pretty well and use most CPUs most of the
> time. There isn't much additional concurrency to gain here. Obvious
> single threaded steps are hotspot linking and jlink. In such a build,
> Hotspot is mostly linked in parallel with all the Java compilation, so
> not an issue. Jlinking the JDK image does stick out as something we
> can't do much in parallel with, unless we also build the test or docs
> image. For a hotspot only build ("make hotspot"), then the hotspot
> linking will stick out as a single threaded step. Note that cmake/ninja
> will not help with any of this. Potential speed up from ninja is also
> very limited as the rules processing of our make scripts does not amount
> to any significant part of a full build.
>
> On Windows specifically, we do have an issue with fork being
> inefficient. We also have a less efficient file system making file
> operations more expensive in general. I have two big (though old)
> identical workstations, one with Windows and one Linux. Very rough
> numbers are 5 minutes for "make images" on the Linux machine and 10 on
> the Windows machine. These differences vary wildly on different hardware
> though. Using Windows native tools here would certainly help to some
> extent. OTOH, we have WSL, which is already considerably more performant
> than Cygwin (very rough numbers, maybe 8-9 minutes for the same build).
> The setup is a bit trickier than Cygwin, but once set up, it works
> really well in my experience.
>
> The area where ninja would provide the most benefit is for incremental
> builds, especially when very little work is actually needed, as it
> processes rules much faster than make. We have worked hard at making
> incremental builds as efficient and fast as possible, but our build is
> also pretty big so the time it takes is still noticeable, especially on
> Windows.
>
> All this said, when picking a build system, compatibility issues are the
> number one concern. If the support matrix of CMake does not completely
> cover the support matrix of OpenJDK, it's a no go to me. I would also be
> hesitant to be at the mercy of the platform support of a 3rd party when
> a new port of OpenJDK needs to be made.
>
> Regarding IDE integration, our build system is able to produce
> compile-commands.json which several IDEs know how to consume.
>
> Another big objection I have to this is the amount work required to
> rewrite the build system (again). I would expect a rewrite like this to
> be several man months, just for the OpenJDK (not counting forced
> downstream work for custom extensions to the build as well as all system
> currently interacting with the build, which I'm sure exist in more
> places than just Oracle).
>
> /Erik
>

Thanks for that extensive explanation! I have nothing to contribute here.
But would like to thank you guys for the work on the build system. It's
exceptionally stable and works very well.

Cheers, Thomas

Re: CMake replacing Autotools?

[Thomas Stüfe]

On Fri, Mar 19, 2021 at 11:04 AM Andrew Haley  wrote:

> On 3/19/21 9:22 AM, Thomas Stüfe wrote:
> >> 2. More choices to actually build the project: Use integrated build
> >> tools of IDEs (Visual Studio, Xcode) or use Ninja, which is faster than
> >> gmake
> >>
> > Is gmake really where we lose time? Did you analyze this or is this just
> an
> > assumption? I would have thought it's things like single threaded jmod,
> > jlink, and subprocess spawning.
>
> I'm sure it is. The other slow thing is linking HotSpot.
>

What is so slow with gmake? Rule processing?

It also depends on the platform, I guess. Eg on Cygwin, the fork emulation
is extremely slow.


> --
> Andrew Haley  (he/him)
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> https://keybase.io/andrewhaley
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>
>

Re: CMake replacing Autotools?

[Thomas Stüfe]

Hi Christoph,

On Fri, Mar 19, 2021 at 12:01 AM Christoph Grüninger 
wrote:

> Hi Magnus,
>
> for sure, I am going to do CMake cheerleading.
>
> I think OpenJDK would profit from a change to CMake:
>
> 1. Less tools required for building: CMake + gmake/Ninja/msbuild/Xcode
> instead of Autotools and all the related Linux/Unix tools, that are
> somewhat alien to Windows and OS X. Visual Studio even provides its own
> CMake.
>

Note that CMake is missing on a number of proprietary Unices (AIX, HPUX)
whereas Autotools are universally available and just work; on Windows via
Cygwin, on Mac via Fink/homebrew to Mac. So I think the argument is
actually the reverse.


>
> 2. More choices to actually build the project: Use integrated build
> tools of IDEs (Visual Studio, Xcode) or use Ninja, which is faster than
> gmake
>
>
Is gmake really where we lose time? Did you analyze this or is this just an
assumption? I would have thought it's things like single threaded jmod,
jlink, and subprocess spawning.

Cheers, Thomas

gtest-specific defines?

[Thomas Stüfe]

Hi,

Sometimes one needs access to hotspot internals only for the sake of
gtests. At the moment there is no define which tells me whether I build
with gtests or not, so I add those interfaces unconditionally, with a "only
for gtests" comment.

I wonder whether it makes sense to add a define which conditionally
switches on code if we build with --with-gtests.

The argument against that would be that we may not want gtest-specific
differences to creep in but rather test what we ship.

What do you think?

Cheers, Thomas

Re: [External] : Re: AArch64 OpenJDK bootstrap failure on head

[Thomas Stüfe]

(dropping Andrew and Aleksey)

On Tue, Feb 2, 2021 at 6:13 PM Ioi Lam  wrote:

>
>
> On 2/2/21 1:32 AM, Thomas Stüfe wrote:
>
>
>
> On Mon, Feb 1, 2021 at 10:11 PM Ioi Lam  wrote:
>
>> On 2/1/21 9:36 AM, Thomas Stüfe wrote:
>>
>> This does not solve the alignment problem, but I don't like that we
>> unconditionally print a message here since this is a non-fatal error. Also,
>> CDS mapping may fail for other reasons, for which we do not print
>> unconditionally. I think we should make this info log level:
>>
>> --- a/src/hotspot/share/memory/metaspaceShared.cpp
>> +++ b/src/hotspot/share/memory/metaspaceShared.cpp
>> @@ -1725,7 +1725,7 @@ MapArchiveResult
>> MetaspaceShared::map_archive(FileMapInfo* mapinfo, char* mapped
>>mapinfo->set_is_mapped(false);
>>
>>if (mapinfo->alignment() != (size_t)os::vm_allocation_granularity()) {
>> -log_error(cds)("Unable to map CDS archive --
>> os::vm_allocation_granularity() expected: " SIZE_FORMAT
>> +log_info(cds)("Unable to map CDS archive --
>> os::vm_allocation_granularity() expected: " SIZE_FORMAT
>> " actual: %d", mapinfo->alignment(),
>> os::vm_allocation_granularity());
>>  return MAP_ARCHIVE_OTHER_FAILURE;
>>}
>>
>> @ Ioi, does that make sense?
>>
>>
>> Yes, your fix makes sense.
>>
>>
> Thanks. https://github.com/openjdk/jdk/pull/2348
> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/2348__;!!GqivPVa7Brio!LRn8UjBQlnYs4XgafXHxU7RUqFZZx-Y19XBMcwy44BEjEXSIBS1GBrWKZ6pwcw$>
>
>
>> This issue is being address in
>> https://bugs.openjdk.java.net/browse/JDK-8236847. We will probably
>> unconditionally change the alignment to 64KB for AARCH64, as well as MacOS
>> (so that you can run a X64 JDK on M1 using Rosetta).
>>
>>
> I thought so too. I also see it being used for region-to-region alignment,
> where I believe page size would be sufficient?
>
>
> The problem in JDK-8236847
> <https://bugs.openjdk.java.net/browse/JDK-8236847> happens when you
> create a CDS archive on one machine, and use it on another, and the two
> machines have different page sizes.
>
>
Oh, I understand. Sorry, I was not clear enough.

What I meant was: os::vm_allocation_granularity() is the OS-dictated
alignment for the start address for memory mappings. We need to know it
wherever we want to predict the mapping address of a future mapping.
Outside of that, there is very little reason to use it instead of using
page size. WIthin the hotspot, it is used all over the place, and a number
of those usages are bewildering and possibly wrong. Hence my original
question, why use it for CDS alignment. But you answered the question, as
long as we mmap the cds regions individually via mmap, we need to do that
with allocation granularity.

Setting it hard wired to 64k sounds pragmatic.

Note that I created https://bugs.openjdk.java.net/browse/JDK-8253683 a
while ago to cleanup usages of os::vm_allocation_granularity(), but never
got along to do this.


> (a) linux/aarch64 can be configured to have either 4KB or 64KB page sizes
> (b) macOS/x64 uses 4KB, but macOS/aarch64 uses 64KB
>
> So if you create the archive on a machine with 4KB page size, your RW
> region may start at (64KB * N + 4KB), and this region cannot be mapped
> directly on a machine with 64KB sizes.
>
> My proposal is to always align the CDS regions by 64KB on these platforms,
> so they can always be mapped under all circumstances.
>
>
Makes perfect sense.


> Alternatives are: use read() instead of mmap; or, instead of mmaping the
> individual regions, mmap all of them at once (assuming that the first
> region, MC, is 64KB aligned). Either solution will reduce the possibility
> of sharing, and make the code more complicated.
>
>
I still think, as in earlier discussions, that mapping it in one go would
be the right way to do.


> Since the CDS archive is at least 10MB in size, adding 3 extra padding
> areas of up to 64KB each doesn't seem that outrageous in file size. There's
> no change in physical memory usage since we never touch the padding area.
>
>
I agree.


> Thanks
> - Ioi
>
>
>
Cheers, Thomas


> .:Thomas
>
> Thanks
>> - Ioi
>>
>> Cheers, Thomas
>>
>>
>> On Mon, Feb 1, 2021 at 6:20 PM Andrew Haley  wrote:
>>
>>> On 2/1/21 5:14 PM, Aleksey Shipilev wrote:
>>> > On 2/1/21 4:38 PM, Andrew Haley wrote:
>>> >> but that doesn't work either. Any ideas? I'm really stuck.
>>> >
>>> > Did you "make clean" after changing any of the configur

Re: AArch64 OpenJDK bootstrap failure on head

[Thomas Stüfe]

On Mon, Feb 1, 2021 at 10:11 PM Ioi Lam  wrote:

> On 2/1/21 9:36 AM, Thomas Stüfe wrote:
>
> This does not solve the alignment problem, but I don't like that we
> unconditionally print a message here since this is a non-fatal error. Also,
> CDS mapping may fail for other reasons, for which we do not print
> unconditionally. I think we should make this info log level:
>
> --- a/src/hotspot/share/memory/metaspaceShared.cpp
> +++ b/src/hotspot/share/memory/metaspaceShared.cpp
> @@ -1725,7 +1725,7 @@ MapArchiveResult
> MetaspaceShared::map_archive(FileMapInfo* mapinfo, char* mapped
>mapinfo->set_is_mapped(false);
>
>if (mapinfo->alignment() != (size_t)os::vm_allocation_granularity()) {
> -log_error(cds)("Unable to map CDS archive --
> os::vm_allocation_granularity() expected: " SIZE_FORMAT
> +log_info(cds)("Unable to map CDS archive --
> os::vm_allocation_granularity() expected: " SIZE_FORMAT
> " actual: %d", mapinfo->alignment(),
> os::vm_allocation_granularity());
>  return MAP_ARCHIVE_OTHER_FAILURE;
>}
>
> @ Ioi, does that make sense?
>
>
> Yes, your fix makes sense.
>
>
Thanks. https://github.com/openjdk/jdk/pull/2348


> This issue is being address in
> https://bugs.openjdk.java.net/browse/JDK-8236847. We will probably
> unconditionally change the alignment to 64KB for AARCH64, as well as MacOS
> (so that you can run a X64 JDK on M1 using Rosetta).
>
>
I thought so too. I also see it being used for region-to-region alignment,
where I believe page size would be sufficient?

.:Thomas

Thanks
> - Ioi
>
> Cheers, Thomas
>
>
> On Mon, Feb 1, 2021 at 6:20 PM Andrew Haley  wrote:
>
>> On 2/1/21 5:14 PM, Aleksey Shipilev wrote:
>> > On 2/1/21 4:38 PM, Andrew Haley wrote:
>> >> but that doesn't work either. Any ideas? I'm really stuck.
>> >
>> > Did you "make clean" after changing any of the configure files and/or
>> configure arguments? I.e. did
>> > AWTIcon32_java_icon16_png actually regenerate?
>>
>> Many times.
>>
>> > Prepending the build with "LOG=debug" would tell what cmdlines are used
>> at every step of build process.
>>
>> Sure, I can see what it's doing. I think this is actually a regression
>> caused by the Windows-AArch64 port. I'll do some bisecting.
>>
>> --
>> Andrew Haley  (he/him)
>> Java Platform Lead Engineer
>> Red Hat UK Ltd. <https://www.redhat.com
>> <https://urldefense.com/v3/__https://www.redhat.com__;!!GqivPVa7Brio!MzT8lD4heOPkWgVBap3cDC2aM4W8zJ1wWS_-PVlTdPwr96wHRafdO7zjS2x2qQ$>
>> >
>> https://keybase.io/andrewhaley
>> <https://urldefense.com/v3/__https://keybase.io/andrewhaley__;!!GqivPVa7Brio!MzT8lD4heOPkWgVBap3cDC2aM4W8zJ1wWS_-PVlTdPwr96wHRafdO7wP-EZNow$>
>> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>>
>>
>

Re: AArch64 OpenJDK bootstrap failure on head

[Thomas Stüfe]

This does not solve the alignment problem, but I don't like that we
unconditionally print a message here since this is a non-fatal error. Also,
CDS mapping may fail for other reasons, for which we do not print
unconditionally. I think we should make this info log level:

--- a/src/hotspot/share/memory/metaspaceShared.cpp
+++ b/src/hotspot/share/memory/metaspaceShared.cpp
@@ -1725,7 +1725,7 @@ MapArchiveResult
MetaspaceShared::map_archive(FileMapInfo* mapinfo, char* mapped
   mapinfo->set_is_mapped(false);

   if (mapinfo->alignment() != (size_t)os::vm_allocation_granularity()) {
-log_error(cds)("Unable to map CDS archive --
os::vm_allocation_granularity() expected: " SIZE_FORMAT
+log_info(cds)("Unable to map CDS archive --
os::vm_allocation_granularity() expected: " SIZE_FORMAT
" actual: %d", mapinfo->alignment(),
os::vm_allocation_granularity());
 return MAP_ARCHIVE_OTHER_FAILURE;
   }

@ Ioi, does that make sense?

Cheers, Thomas


On Mon, Feb 1, 2021 at 6:20 PM Andrew Haley  wrote:

> On 2/1/21 5:14 PM, Aleksey Shipilev wrote:
> > On 2/1/21 4:38 PM, Andrew Haley wrote:
> >> but that doesn't work either. Any ideas? I'm really stuck.
> >
> > Did you "make clean" after changing any of the configure files and/or
> configure arguments? I.e. did
> > AWTIcon32_java_icon16_png actually regenerate?
>
> Many times.
>
> > Prepending the build with "LOG=debug" would tell what cmdlines are used
> at every step of build process.
>
> Sure, I can see what it's doing. I think this is actually a regression
> caused by the Windows-AArch64 port. I'll do some bisecting.
>
> --
> Andrew Haley  (he/him)
> Java Platform Lead Engineer
> Red Hat UK Ltd. 
> https://keybase.io/andrewhaley
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>
>

Re: RFR: 8258477: Pre-submit testing using GitHub Actions should merge changes from target branch

[Thomas Stüfe]

On Wed, Dec 16, 2020 at 3:20 PM Robin Westberg 
wrote:

> Hi Thomas,
>
> > On 16 Dec 2020, at 14:54, Thomas Stüfe  wrote:
> >
> > Hi Robin,
> >
> > does this mean tests won't run on branches which cannot be merged with
> the
> > assumed target branch?
>
> No, if there’s a problem with doing the merge the tests will simply
> continue without doing it, and just use the commit in the pull request
> as-is.
>
> Best regards,
> Robin


Yes, that makes sense. Thank you!

..Thomas


>
> >
> > Thanks, Thomas
> >
> >
> > On Wed, Dec 16, 2020 at 11:55 AM Robin Westberg <
> rwestb...@openjdk.java.net>
> > wrote:
> >
> >> Normally when running GitHub Actions on a pull request, what is checked
> >> out is the merge of the pull request with the latest changes on the
> target
> >> branch. This ensure that what is tested is as close as possible to what
> >> will actually be the result of integrating said pull request.
> >>
> >> In our use of GitHub Actions, we don't run directly on pull requests but
> >> instead allow contributors to run them in their own personal forks. In
> that
> >> context, there is no notion of a target branch. However, we can infer
> the
> >> target project and branch by encoding this in the workflow file. This
> >> allows us to perform the same merge manually, and achieve the same
> >> behaviour.
> >>
> >> The change also adds an option to disable this automated merge when
> >> launching the workflow manually, which could be useful when
> investigating
> >> unexpected test failures.
> >>
> >> Note that downstream projects picking up this change will have to adapt
> >> the workflow file to keep using these actions for pre-submit testing.
> This
> >> has been a common request from downstream projects, but could also be
> done
> >> in a separate change (or not at all).
> >>
> >> -
> >>
> >> Commit messages:
> >> - Allow opting out of automated merge on manual runs
> >> - Initial implementation
> >>
> >> Changes: https://git.openjdk.java.net/jdk/pull/1801/files
> >> Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1801&range=00
> >>  Issue: https://bugs.openjdk.java.net/browse/JDK-8258477
> >>  Stats: 117 lines in 1 file changed: 116 ins; 0 del; 1 mod
> >>  Patch: https://git.openjdk.java.net/jdk/pull/1801.diff
> >>  Fetch: git fetch https://git.openjdk.java.net/jdk
> >> pull/1801/head:pull/1801
> >>
> >> PR: https://git.openjdk.java.net/jdk/pull/1801
> >>
>
>

Re: RFR: 8258477: Pre-submit testing using GitHub Actions should merge changes from target branch

[Thomas Stüfe]

Hi Robin,

does this mean tests won't run on branches which cannot be merged with the
assumed target branch?

Thanks, Thomas


On Wed, Dec 16, 2020 at 11:55 AM Robin Westberg 
wrote:

> Normally when running GitHub Actions on a pull request, what is checked
> out is the merge of the pull request with the latest changes on the target
> branch. This ensure that what is tested is as close as possible to what
> will actually be the result of integrating said pull request.
>
> In our use of GitHub Actions, we don't run directly on pull requests but
> instead allow contributors to run them in their own personal forks. In that
> context, there is no notion of a target branch. However, we can infer the
> target project and branch by encoding this in the workflow file. This
> allows us to perform the same merge manually, and achieve the same
> behaviour.
>
> The change also adds an option to disable this automated merge when
> launching the workflow manually, which could be useful when investigating
> unexpected test failures.
>
> Note that downstream projects picking up this change will have to adapt
> the workflow file to keep using these actions for pre-submit testing. This
> has been a common request from downstream projects, but could also be done
> in a separate change (or not at all).
>
> -
>
> Commit messages:
>  - Allow opting out of automated merge on manual runs
>  - Initial implementation
>
> Changes: https://git.openjdk.java.net/jdk/pull/1801/files
>  Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1801&range=00
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8258477
>   Stats: 117 lines in 1 file changed: 116 ins; 0 del; 1 mod
>   Patch: https://git.openjdk.java.net/jdk/pull/1801.diff
>   Fetch: git fetch https://git.openjdk.java.net/jdk
> pull/1801/head:pull/1801
>
> PR: https://git.openjdk.java.net/jdk/pull/1801
>

gh actions fail on linux x64 when fetching libsound

[Thomas Stüfe]

Hi,

all my linux x64 gh builds fail with:

"E: Failed to fetch
http://azure.archive.ubuntu.com/ubuntu/pool/main/a/alsa-lib/libasound2-dev_1.2.2-2.1ubuntu2.1_amd64.deb
 404  Not Found [IP: 52.147.219.192 80]"

https://github.com/tstuefe/jdk/runs/1452221121

Does anyone have an idea?

Thanks, Thomas

Re: RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow

[Thomas Stüfe]

Hi Bernhard,

just some drive-by comments.

- 40-50 min builds seem to be excessive for just the hotspot build, do you
know what exactly takes that long? Is this for release and debug each or
both combined?

- Is it worth the cycles to build both release *and* debug? How probable is
it that a non-win-aarch-dev will break one but not the other? I presume
developers on the windows aarch project will have tested the build locally
before pushing.

- fixpath: Having the gh actions download a patch from your personal repos
seems odd

I feel at some point we need to have a talk about the growing number of
platforms in github actions. E.g. which platforms should be mandatory green
before pushing.

I also am unsure about the associated cost for each developer (I know atm
everyone has a free quota, but that is not limitless. If it is eaten up,
strictly speaking one would not be able to push anymore since no submit
tests can be ran until the quota is refilled).

Cheers, Thomas

On Mon, Nov 23, 2020 at 10:41 AM Bernhard Urban-Forster <
bur...@openjdk.java.net> wrote:

> This adds the cross-compiled build only, as no Windows+Arm64 machines are
> available on GitHub Action that we could use to run the tests.
>
> Due to cross-compilation a build JDK is required. Initially I added EA
> builds to be downloaded from https://jdk.java.net/16/ and used for that,
> but then I saw how @shipiliv attempted it for the linux cross-compilation
> builds in https://github.com/openjdk/jdk/pull/1147.  That is using the
> JDK image produced by the x64 variant. This however add more stress to the
> "critical path", as now two more jobs depend on the x64 build first.
>
> Let's see how it works out in the long-run. A Windows+AArch64 build takes
> 40-50min.
>
> -
>
> Commit messages:
>  - 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow
>
> Changes: https://git.openjdk.java.net/jdk/pull/1379/files
>  Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1379&range=00
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8256657
>   Stats: 164 lines in 1 file changed: 163 ins; 0 del; 1 mod
>   Patch: https://git.openjdk.java.net/jdk/pull/1379.diff
>   Fetch: git fetch https://git.openjdk.java.net/jdk
> pull/1379/head:pull/1379
>
> PR: https://git.openjdk.java.net/jdk/pull/1379
>

Re: RFR: 8256414: add optimized build to submit workflow [v2]

[Thomas Stüfe]

Hi Igor, thanks for the update and the reference. Yes, makes sense to test
it automatically as long as it’s still there.

Cheers, Thomas

On Mon 16. Nov 2020 at 21:55,  wrote:

> Hi Thomas,
>
> There is indeed was a discussion about the future of optimized flavor, and
> the consensus was to remove it. We, however, haven’t done that yet
> (8183287[*] is filed for that work). meanwhile, people still use optimized
> builds from time to time so we need to ensure they are at least buildable.
>
> [*]: https://bugs.openjdk.java.net/browse/JDK-8183287
>
> — Igor
>
> On Nov 16, 2020, at 12:43 PM, Thomas Stüfe 
> wrote:
>
> 
>
> Hi,
>
> just curious, was the optimized build not earmarked for removal? I dimly
> remember reading discussions about this.
>
> Thanks, Thomas
>
> On Mon, Nov 16, 2020 at 9:35 PM Daniel D.Daugherty <
> dcu...@openjdk.java.net> wrote:
>
>> On Mon, 16 Nov 2020 19:29:59 GMT, Igor Ignatyev 
>> wrote:
>>
>> >> Marked as reviewed by shade (Reviewer).
>> >
>> > Thanks for the reviews, folks. the build looks green, integrating.
>>
>> @iignatev - did you also change Mach5? I don't have workflow builds
>> enabled
>> by default since I typically do Mach5 builds...
>>
>> -
>>
>> PR: https://git.openjdk.java.net/jdk/pull/1233
>>
>

Re: RFR: 8256414: add optimized build to submit workflow [v2]

[Thomas Stüfe]

Hi,

just curious, was the optimized build not earmarked for removal? I dimly
remember reading discussions about this.

Thanks, Thomas

On Mon, Nov 16, 2020 at 9:35 PM Daniel D.Daugherty 
wrote:

> On Mon, 16 Nov 2020 19:29:59 GMT, Igor Ignatyev 
> wrote:
>
> >> Marked as reviewed by shade (Reviewer).
> >
> > Thanks for the reviews, folks. the build looks green, integrating.
>
> @iignatev - did you also change Mach5? I don't have workflow builds enabled
> by default since I typically do Mach5 builds...
>
> -
>
> PR: https://git.openjdk.java.net/jdk/pull/1233
>

Re: Why do we export "JVM_handle_xxx_signal"?

[Thomas Stüfe]

Thanks for checking, Ioi. I think I'll remove the export and rename the
functions.

Cheers, Thomas

On Fri, Oct 30, 2020 at 7:12 AM Ioi Lam  wrote:

> I have no idea, but this symbol has been exported since we moved the
> HotSpot source code from SCCS to Mercurial in 2008. It's probably
> vestige from the early days of Java.
>
>
> http://hg.openjdk.java.net/jdk7/modules/hotspot/annotate/9646293b9637/make/linux/makefiles/mapfile-vers-product#l244
>
> I checked all .so files in our JDK build and no one uses
> JVM_handle_linux_signal. I think it's probably safe to hide it. We
> should probably drop the JVM_ prefix, since this function is not
> declared in jvm.h.
>
> Thanks
> - Ioi
>
> On 10/29/20 9:02 AM, Thomas Stüfe wrote:
> > Hi,
> >
> > Does anyone know why we explicitly export JVM_handle_bsd_signal and
> > JVM_handle_linux_signal (the latter also accidentally from symbols-aix)?
> >
> > These functions are not even the real signal handler, just an internal
> > function; the signal handler is "javaSignalHandler", but that one is not
> > exported...
> >
> > Thanks, Thomas
>
>

Why do we export "JVM_handle_xxx_signal"?

[Thomas Stüfe]

Hi,

Does anyone know why we explicitly export JVM_handle_bsd_signal and
JVM_handle_linux_signal (the latter also accidentally from symbols-aix)?

These functions are not even the real signal handler, just an internal
function; the signal handler is "javaSignalHandler", but that one is not
exported...

Thanks, Thomas

Re: RFR(S) : 8245610 : remove in-tree copy on gtest

[Thomas Stüfe]

On Mon, Jun 8, 2020 at 3:14 PM Erik Joelsson 
wrote:

> On 2020-06-07 23:56, Thomas Stüfe wrote:
> > Also, it may be me, but where do I find the official build
> > documentation? Googling "building openjdk" gives me a number of hits,
> > neither one seems official - the top hit seems to be one from Magnus'
> > private cr directory :) but since it does not mention google test at
> > all I don't think this is recent.
> >
> The official building doc is found in the source tree in
> doc/building.[md|html].
>
> /Erik
>

Thank you Erik.

Re: RFR(S) : 8245610 : remove in-tree copy on gtest

[Thomas Stüfe]

Taking this up again, why could we just not roll back this change if it was
ill advised?

Also, it may be me, but where do I find the official build documentation?
Googling "building openjdk" gives me a number of hits, neither one seems
official - the top hit seems to be one from Magnus' private cr directory :)
but since it does not mention google test at all I don't think this is
recent.

Thanks, Thomas

On Mon, Jun 1, 2020 at 6:01 PM Thomas Stüfe  wrote:

> Hi Magnus,
>
> On Mon, Jun 1, 2020 at 1:35 PM Magnus Ihse Bursie <
> magnus.ihse.bur...@oracle.com> wrote:
>
>>
>>
>> On 2020-05-29 07:04, Thomas Stüfe wrote:
>>
>> Hi Igor,
>>
>> thank you for taking the time to answer my grumblings.
>>
>> Yes, comparison with jtreg is a bit crooked - it is not needed to get a
>> valid build. But jtreg is also maintained in the official OpenJDK
>> repositories; I can clone codetools/jtreg and have the correct version.
>> With gtests, I have to know which version OpenJDK needs, which is not the
>> latest version, and have to download that from outside our repositories.
>> Getting it wrong version may yield me difficult to analyze build errors
>> (plattform zoo, handicapped C++ compilers etc). Also, updating to a new
>> gtest version will now involve a lot more communicatio (a version check in
>> configure would help with that).
>>
>> But this is a minor complaint, I could live with that. I most dislike the
>> fact that I have to specify this option *every single time*, and that
>> omitting it is objectively wrong and may give me a false sense of security.
>> Omitting it gives no warning, but if my changes break gtests I will only
>> notice it hours later when jdk/submit results are back.
>>
>> Yes, I can hide this behind an alias or script, but I think this is the
>> wrong way. You guys did such a good job in making the build dead easy:
>> first time, it tells me exactly which Debian packets to install, I always
>> loved that special touch. I specify boot jdk and maybe debug level and I am
>> done. In fact, the build is so easy that until recently I did not even know
>> we had a build documentation :) The new --with-gtest option is a jarring
>> break from that.
>>
>> I agree, Thomas. The patch went in too fast, without proper exploration
>> of alternatives, or how this would affect the usability of the build.
>>
>> One of the main goals of the build system has been that it should be easy
>> to build, and the system should be "self-instructing", so that if a
>> dependency is missing, this should be made clear, and a suitable suggestion
>> for correction should be printed.
>>
>> The gtest removal fails on both these accounts. :(
>>
>> I think we should change configure so that gtest is a required
>> dependency, unless specifically disabled. We can look  for gtest in
>> standard locations, like /usr/src/googletest, where it is installed by e.g.
>> "sudo apt install googletest".
>>
>
> I fear that we are more reliant on a specific version of googletest than
> is the case with standard libraries. Just a gut feeling, but the fact that
> we cannot use the latest googletest version is a strong indicator. So using
> the OS-provided version may be the wrong one, which may be a constant
> source of annoying build errors.
>
> Installation instructions, such as this apt command, must also be added.
>> (I'd appreciate feedback on what the package is called on RH/Fedora!) We
>> could also consider checking for an environment variable, similar to how
>> the boot JDK looks at JAVA_HOME, so you can set up a non-standard path in
>> your environment, and do not have to pass it to configure as a flag all the
>> time.
>>
>> And we also need to fix the RunTest framework, as René pointed out, so
>> that if you try to run gtests without the gtest library, you need to get a
>> proper error message that describes the step you need to take to be able to
>> run gtest tests.
>>
>> /Magnus
>>
>>
>>
> Thanks, Thomas
>
>
>> In my mind, gtest is in the same ballpark as the freetype library on
>> Windows. That has always been a bit annoying but that was only Windows.
>> Something you cannot rely the OS library mechanism to deliver but have to
>> download and place yourself and tell the build about it.
>>
>> I wonder whether we can find a compromise somehow. Maybe something like
>> an agreed on structure and place for a "companion directory", containing
>> build dependencies like these. Its location could be by convention in a
&g

Re: RFR(S) : 8245610 : remove in-tree copy on gtest

[Thomas Stüfe]

Hi Magnus,

On Mon, Jun 1, 2020 at 1:35 PM Magnus Ihse Bursie <
magnus.ihse.bur...@oracle.com> wrote:

>
>
> On 2020-05-29 07:04, Thomas Stüfe wrote:
>
> Hi Igor,
>
> thank you for taking the time to answer my grumblings.
>
> Yes, comparison with jtreg is a bit crooked - it is not needed to get a
> valid build. But jtreg is also maintained in the official OpenJDK
> repositories; I can clone codetools/jtreg and have the correct version.
> With gtests, I have to know which version OpenJDK needs, which is not the
> latest version, and have to download that from outside our repositories.
> Getting it wrong version may yield me difficult to analyze build errors
> (plattform zoo, handicapped C++ compilers etc). Also, updating to a new
> gtest version will now involve a lot more communicatio (a version check in
> configure would help with that).
>
> But this is a minor complaint, I could live with that. I most dislike the
> fact that I have to specify this option *every single time*, and that
> omitting it is objectively wrong and may give me a false sense of security.
> Omitting it gives no warning, but if my changes break gtests I will only
> notice it hours later when jdk/submit results are back.
>
> Yes, I can hide this behind an alias or script, but I think this is the
> wrong way. You guys did such a good job in making the build dead easy:
> first time, it tells me exactly which Debian packets to install, I always
> loved that special touch. I specify boot jdk and maybe debug level and I am
> done. In fact, the build is so easy that until recently I did not even know
> we had a build documentation :) The new --with-gtest option is a jarring
> break from that.
>
> I agree, Thomas. The patch went in too fast, without proper exploration of
> alternatives, or how this would affect the usability of the build.
>
> One of the main goals of the build system has been that it should be easy
> to build, and the system should be "self-instructing", so that if a
> dependency is missing, this should be made clear, and a suitable suggestion
> for correction should be printed.
>
> The gtest removal fails on both these accounts. :(
>
> I think we should change configure so that gtest is a required dependency,
> unless specifically disabled. We can look  for gtest in standard locations,
> like /usr/src/googletest, where it is installed by e.g. "sudo apt install
> googletest".
>

I fear that we are more reliant on a specific version of googletest than is
the case with standard libraries. Just a gut feeling, but the fact that we
cannot use the latest googletest version is a strong indicator. So using
the OS-provided version may be the wrong one, which may be a constant
source of annoying build errors.

Installation instructions, such as this apt command, must also be added.
> (I'd appreciate feedback on what the package is called on RH/Fedora!) We
> could also consider checking for an environment variable, similar to how
> the boot JDK looks at JAVA_HOME, so you can set up a non-standard path in
> your environment, and do not have to pass it to configure as a flag all the
> time.
>
> And we also need to fix the RunTest framework, as René pointed out, so
> that if you try to run gtests without the gtest library, you need to get a
> proper error message that describes the step you need to take to be able to
> run gtest tests.
>
> /Magnus
>
>
>
Thanks, Thomas


> In my mind, gtest is in the same ballpark as the freetype library on
> Windows. That has always been a bit annoying but that was only Windows.
> Something you cannot rely the OS library mechanism to deliver but have to
> download and place yourself and tell the build about it.
>
> I wonder whether we can find a compromise somehow. Maybe something like
> an agreed on structure and place for a "companion directory", containing
> build dependencies like these. Its location could be by convention in a
> clear relation to the OpenJDK sources (e.g. just beside it) and configure
> would look for the libraries in there by default. Like this:
>
> - openjdk-source
>- configure
>- Makefile
>- ..
> - openjdk-builddeps
>   - google-test
>   - freetype
>   - ..
>
> This would also lend itself very well to a maintained repository of build
> dependencies somewhere (not necessarily maintained by Oracle). We would
> have similar ease of use as with platform libraries, which by default are
> also expected in a standard place in the file system.
>
> Thanks, Thomas
>
>
> On Fri, May 29, 2020 at 5:20 AM Igor Ignatyev 
> wrote:
>
>> Hi Thomas,
>>
>> I regret that this patch made you unhappier. I fully agree that all
>> hotspot develope

Re: RFR(S) : 8245610 : remove in-tree copy on gtest

[Thomas Stüfe]

Hi Igor,

thank you for taking the time to answer my grumblings.

Yes, comparison with jtreg is a bit crooked - it is not needed to get a
valid build. But jtreg is also maintained in the official OpenJDK
repositories; I can clone codetools/jtreg and have the correct version.
With gtests, I have to know which version OpenJDK needs, which is not the
latest version, and have to download that from outside our repositories.
Getting it wrong version may yield me difficult to analyze build errors
(plattform zoo, handicapped C++ compilers etc). Also, updating to a new
gtest version will now involve a lot more communicatio (a version check in
configure would help with that).

But this is a minor complaint, I could live with that. I most dislike the
fact that I have to specify this option *every single time*, and that
omitting it is objectively wrong and may give me a false sense of security.
Omitting it gives no warning, but if my changes break gtests I will only
notice it hours later when jdk/submit results are back.

Yes, I can hide this behind an alias or script, but I think this is the
wrong way. You guys did such a good job in making the build dead easy:
first time, it tells me exactly which Debian packets to install, I always
loved that special touch. I specify boot jdk and maybe debug level and I am
done. In fact, the build is so easy that until recently I did not even know
we had a build documentation :) The new --with-gtest option is a jarring
break from that.

In my mind, gtest is in the same ballpark as the freetype library on
Windows. That has always been a bit annoying but that was only Windows.
Something you cannot rely the OS library mechanism to deliver but have to
download and place yourself and tell the build about it.

I wonder whether we can find a compromise somehow. Maybe something like
an agreed on structure and place for a "companion directory", containing
build dependencies like these. Its location could be by convention in a
clear relation to the OpenJDK sources (e.g. just beside it) and configure
would look for the libraries in there by default. Like this:

- openjdk-source
   - configure
   - Makefile
   - ..
- openjdk-builddeps
  - google-test
  - freetype
  - ..

This would also lend itself very well to a maintained repository of build
dependencies somewhere (not necessarily maintained by Oracle). We would
have similar ease of use as with platform libraries, which by default are
also expected in a standard place in the file system.

Thanks, Thomas


On Fri, May 29, 2020 at 5:20 AM Igor Ignatyev 
wrote:

> Hi Thomas,
>
> I regret that this patch made you unhappier. I fully agree that all
> hotspot developers are to always build *and* run gtest tests, yet not all
> people who work on OpenJDK project are hotspot developers.
>
> I also believe that all OpenJDK developers are to run tests related to the
> area they are changing. The vast majority of such tests are jtreg tests.
> And jtreg, for better or worse, is a counterexample to your last paragraph
> -- it's an essential part of OpenJDK, but it doesn't come in a form of well
> established library, and none of known to me platforms have jtreg in their
> package manager, so you do have to manually download jtreg and specify its
> location one way or another. I understand that there is a difference b/w
> gtest and jtreg, as jtreg isn't really need at build time. However the
> proper way to run any of OpenJDK tests is by `make test` and for it to work
> you need to either executed configure w/ --with-jtreg or specify JT_HOME on
> each invocation of `make test`, where the latter is a preferred way. From
> that point of view, gtest and jtreg aren't different, you need to download
> both manually and specify their location at configure-time.
>
> with that being said, I truly understand the inconvenience it causes to
> you, yet given you need to download gtest only once and it's fairly easy to
> hide `with-gtest` behind a script or an alias like configure_openjdk='bash
> ./configure --with-gtest=$GTEST_HOME', I hope it won't cause problems for
> you and won't discourage you in anyways.
>
> Thanks,
> -- Igor
>
> On May 28, 2020, at 12:31 AM, Thomas Stüfe 
> wrote:
>
> Hi,
>
> I know the boat has sailed, I missed this one. But I am a bit unhappy
> about this.
>
> I always build with gtests; I think it makes no sense to not build with
> gtest. Even if you do not want to run the gtests (and it makes sense to
> always do since its a good first-base validity test), hotspot developers
> should always build them since changes in the hotspot sources may break
> hotspot gtests. hotspot source and gtests are a unity.
>
> So if it makes no sense to not build gtest, I should not need a special
> option to specify gtest location - I'd argue that the default case

Re: RFR(S) : 8245610 : remove in-tree copy on gtest

[Thomas Stüfe]

Hi,

I know the boat has sailed, I missed this one. But I am a bit unhappy about
this.

I always build with gtests; I think it makes no sense to not build with
gtest. Even if you do not want to run the gtests (and it makes sense to
always do since its a good first-base validity test), hotspot developers
should always build them since changes in the hotspot sources may break
hotspot gtests. hotspot source and gtests are a unity.

So if it makes no sense to not build gtest, I should not need a special
option to specify gtest location - I'd argue that the default case should
not require special options.

The JBS issue states that "it can be treated like any other external
dependencies" but this comparison does not hold - almost all other
dependencies come in the form of well established libraries with standard
headers and standard installation locations which can be coded as default
values. On a recent mainstream platform I do not have to specify any paths
to libraries to build OpenJDK. This is now different, I always have to
manually download gtests and specify gtest location. This is regrettable.

Thanks, Thomas


On Tue, May 26, 2020 at 2:27 PM Magnus Ihse Bursie <
magnus.ihse.bur...@oracle.com> wrote:

>
>
> On 2020-05-25 19:53, Igor Ignatyev wrote:
> > Hi Magnus,
> >
> >> On May 25, 2020, at 7:46 AM, Magnus Ihse Bursie
> >>  >> > wrote:
> >>
> >> Looks basically good to me.
> > thanks for your review!
> >>
> >> We need to document the dependency on gtest, and how to retrieve it.
> >> I recommend you add a section next to the JTReg information (under
> >> the "## Running Tests" heading) in doc/building.md. I think you
> >> should include basically the same information as you did in your
> >> follow-up mail, that was informative and clear.
> > that's a good suggestion, I've added a small paragraph to 'Running
> > Tests' in doc/building.md and regenerated corresponding .html file --
> > http://cr.openjdk.java.net/~iignatyev/8245610/webrev.doc/
> > please let me know if you think something should be added or reworded.
> Looks great! Thank you.
>
> /Magnus
> >
> >>
> >> I assume the most suitable replacement for developers who are used to
> >> add a '--disable-hotspot-gtest' to e.g. a pre-definied jib
> >> configuration is to now use '--without-gtest'. This will need to be
> >> communicated, perhaps to both hotspot-dev and jdk-dev.
> > sure, after this gets integrated, I'll let both hotspot-dev and
> > jdk-dev aliases know which changes might be required to enable/disable
> > hotspot gtest tests compilation.
> >
> > Thanks.
> > -- Igor
> >
> >>
> >> /Magnus
> >>
> >> On 2020-05-22 20:12, Igor Ignatyev wrote:
> >>> http://cr.openjdk.java.net/~iignatyev/8245610/webrev.00/
>  132 lines changed: 80 ins; 36 del; 16 mod
> >>> http://cr.openjdk.java.net/~iignatyev/8245610/webrev.00%2bremoval/
>  57482 lines changed: 80 ins; 57385 del; 17 mod;
> >>> Hi all,
> >>>
> >>> could you please review this small (if you ignore removal part)
> >>> patch which removes in-tree copy of gtest (test/fmw/gtest) and
> >>> updates makefiles to use one provided thru an added configure option
> >>> `--with-gtest`? the previously used configure option
> >>> `--enable-hotspot-gtest` gets depricated.
> >>>
> >>> the patch also compiles gtest and gmock source code into a static
> >>> library so they can be compiled w/ all warnings disabled.
> >>>
> >>> from JBS:
>  w/ JEP 381 (JDK-8241787 / JDK-8244224) being integrated, all
>  compilers used by OpenJDK became supported by gtest out-of-box, so
>  there is no need to have our copy of gtest framework (including
>  gmock) within OpenJDK source tree. instead, it can be treated like
>  any other external dependencies, and a pointer to the directory w/
>  gtest code can be passed via configure options.
> >>>
> >>> JBS: https://bugs.openjdk.java.net/browse/JDK-8245610
> >>> webrevs:
> >>>  - http://cr.openjdk.java.net/~iignatyev/8245610/webrev.00/
> >>> (meaningful changes)
> >>>  -
> >>> http://cr.openjdk.java.net/~iignatyev/8245610/webrev.00%2bremoval/
> >>> (all changes)
> >>> testing:
> >>> - gtest tests on {linux,windows,macosx}-x64;
> >>> - tier[1-5] builds which include but not limited to linux-aarch64,
> >>> linux-arm32, linux-x64-zero
> >>>
> >>> Thanks,
> >>> -- Igor
> >>>
> >>
> >
>
>

Re: RFR: JDK-8244966: Add .vscode to .hgignore

[Thomas Stüfe]

LGTM.

Cheers, Thomas

On Sat, May 16, 2020 at 1:34 AM Lemmond, Dan  wrote:

> Hi,
>
> This is my first review request so apologies for any missteps or
> inconsistencies.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8244966
> Webrev: http://cr.openjdk.java.net/~phh/8244966/webrev.01/
>
> Please review this change that adds the .vscode directory to .hgignore,
> similar to the .idea directory. This directory is generated by various
> Visual Studio Code plugins during development.
>
> Testing:
> Local testing done, verified that .vscode/ no longer shows up when running
> hg status.
>
> Thanks,
> Dan
>

Re: RFR: 8199138: Add RISC-V support to Zero

[Thomas Stüfe]

Looks still good.

..Thomas

On Tue 7. Apr 2020 at 01:20, John Paul Adrian Glaubitz <
glaub...@physik.fu-berlin.de> wrote:

> Hello!
>
> On 4/6/20 8:09 PM, John Paul Adrian Glaubitz wrote:
> > I have reduced the complexity of the patch as some of the changes from
> > the previous change set are not necessary, in particular the changes
> > to config.{guess,sub}, the definition of EM_RISCV (which is already
> defined
> > by the Linux kernel headers now provided the kernel is recent enough).
>
> After checking various kernel headers of older but still supported
> enterprise
> Linux distributions such as SLE-12 and RHEL-7, I think it's probably better
> to include the redundant EM_RISCV definition to avoid build problems on
> these
> platforms [1]. I just want to be on the safe side.
>
> The build changes are unchanged.
>
> Adrian
>
> > [1] http://cr.openjdk.java.net/~glaubitz/8199138/webrev.03/
>
> --
>  .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer - glaub...@debian.org
> `. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
>   `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
>

Re: RFR: 8199138: Add RISC-V support to Zero

[Thomas Stüfe]

Hi Adrian, looks good to me.

Cheers, Thomas

On Mon, Apr 6, 2020, 20:11 John Paul Adrian Glaubitz <
glaub...@physik.fu-berlin.de> wrote:

> Hello!
>
> Please review this small change which adds basic support for the riscv64
> target for Linux/Zero [1].
>
> I have reduced the complexity of the patch as some of the changes from
> the previous change set are not necessary, in particular the changes
> to config.{guess,sub}, the definition of EM_RISCV (which is already defined
> by the Linux kernel headers now provided the kernel is recent enough).
>
> Additionally, I'm leaving the change for os::get_summary_cpu_info() out
> as I would like to clean up this code a bit first - it's rather
> inconsistent
> considering whether the arch override is used for Hotspot arches only or
> also for Zero arches.
>
> Thanks,
> Adrian
>
> > [1] http://cr.openjdk.java.net/~glaubitz/8199138/webrev.02/
>
> --
>  .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer - glaub...@debian.org
> `. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
>   `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
>

Re: RFR [11u] 8232572: Add hooks for custom output dir in Bundles.gmk

[Thomas Stüfe]

LGTM.

..Thomas

On Mon, Mar 9, 2020 at 2:57 PM Langer, Christoph 
wrote:

> Hi,
>
> please review this Oracle parity backport.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8232572
> Original Change: https://hg.openjdk.java.net/jdk/jdk/rev/ae0af9fb3dbb
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8232572.11u/
>
> I had to manually bring some hunks to the right position in the file. The
> $$(call MakeTargetDir) does not yet exist as it was only introduced with
> JDK-8211724. So I just add the LogWarn call as first command in the recipe.
>
> Thanks and best regards
> Christoph
>
>

Re: JLI_Launch has mangled name in jli.dll when building for win32

[Thomas Stüfe]

Hi Robert,

This means that the function is using the __stdcall calling convention. I
do not know whether this is correct or not, but caller and callee have to
agree on the calling convention used, so the prototype for that function
used when compiling the caller code must have been declared with __stdcall
too for this to work.

This is a 32bit Windows specialty. No other platform, including 64 bit
Windows, has different calling conventions.

Hope that helps,

Cheers, Thomas


On Thu, Jul 11, 2019, 09:34 Robert Lichtenberger <
r.lichtenber...@synedra.com> wrote:

> I have tried to use the jpackage EA version in combination with the
> win32-build of Java 12 from AdoptOpenJDK.
> I was able to build the jpackage EA from the sandbox and built an image
> from the win32 Java 12 build, which then was successfully combined using
> jpackage's --runtime-image option. (BTW I was able to produce working
> packages for win64, mac and linux in that way).
>
> Now when I tried to start the win32 package I get an error message:
> Failed to locate JLI_Launch.
>
> I digged into the jpackage/launcher code and found out that
> runtime/bin/jli.dll is loaded and the function JLI_Launch is called.
>
> But the jli.dll in the win32-build of Java 12 from AdoptOpenJDK has a
> name mangled version  of JLI_Launch in it (_JLI_Launch@56) which is (I
> think) the reason that the JVM cannot be started.
>
> I've since tried to build Java 12 myself and the effect is reproducible.
> When I build for win64, the jli.dll is correct, when I build
> --with-target-bits=32 from the exact same sources the jli.dll has the
> mangled name.
>
> Other pre-built JDKs (e.g. Liberica JDK) have the same "problem".
>
> Any ideas/hints as to what could cause the problem would be most welcome
> :-).
>
>
>

Re: AIX xlc16 options langlvl=c99vla / langlvl=noredefmac is not supported

[Thomas Stüfe]

I think this is fine for 14.

Cheers, Thomas

On Mon, Jul 8, 2019 at 9:23 AM Baesken, Matthias 
wrote:

> Hello,   when building   jdk/jdk  on AIX  (using the current default
> xlc16)   I get a ton of warnings  :
>
> warning: 1540-5200 The option "langlvl=c99vla" is not supported.
> warning: 1540-5200 The option "langlvl=noredefmac" is not supported.
>
>
> The 2  options seem  to be gone with the current xlc16  / xlclang++  .   I
> would like to remove them -  any objections ?
> (removing them might be bad for older xlc  versions but I think we
> probably  cannot compile with older versions  any more  ) .
>
>
> Best regards, Matthias
>

Re: RFR: [XS] 8227171: provide function names in native stack trace on aix with xlc16

[Thomas Stüfe]

It's trivial IMHO. Ship it.

On Thu, Jul 4, 2019, 09:30 Baesken, Matthias 
wrote:

> Hello, any more reviews ?
>
> May I push the change ?
>
>
>
> Best regards, Matthias
>
>
>
>
>
>
>
> *From:* Thomas Stüfe 
> *Sent:* Mittwoch, 3. Juli 2019 14:09
> *To:* Baesken, Matthias 
> *Cc:* build-dev@openjdk.java.net; ppc-aix-port-...@openjdk.java.net
> *Subject:* Re: RFR: [XS] 8227171: provide function names in native stack
> trace on aix with xlc16
>
>
>
> Looks good and trivial. Thanks for fixing.
>
>
>
> ..Thomas
>
>
>
> On Wed, Jul 3, 2019 at 2:02 PM Baesken, Matthias 
> wrote:
>
> Hello please review this small fix for AIX (needed  with  the new xlc16) .
>
> Currently  we miss function names in the hs_err file  :
>
> 0x000115098660 - 0x09000f629a4c libjvm.so:: function>+0x6c (C++ saves_lr stores_bc gpr_saved:4 )
> 0x0001150986f0 - 0x0900113594bc libjvm.so:: function>+0x95c (C++ saves_lr stores_bc gpr_saved:18 )
>
> The output contains "nameless function"  instead of the  (member) function
> names .
> Looks like this unwanted behavior  was introduced when switching to the
> xlc16  compiler.
>
> With xlc16, it looks like we have to  configure  the traceback table
> generation to get the output we need.
>
> The -qtbtable=full   setting can be used for this _
>
>
> https://www.ibm.com/support/knowledgecenter/SSGH4D_16.1.0/com.ibm.xlf161.aix.doc/compiler_ref/tbtable.html
>
>
>
> Bug / webrev :
>
> https://bugs.openjdk.java.net/browse/JDK-8227171
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8227171.0/
>
>
> Thanks, Matthias
>
>

Re: RFR: [XS] 8227171: provide function names in native stack trace on aix with xlc16

[Thomas Stüfe]

Looks good and trivial. Thanks for fixing.

..Thomas

On Wed, Jul 3, 2019 at 2:02 PM Baesken, Matthias 
wrote:

> Hello please review this small fix for AIX (needed  with  the new xlc16) .
>
> Currently  we miss function names in the hs_err file  :
>
> 0x000115098660 - 0x09000f629a4c libjvm.so:: function>+0x6c (C++ saves_lr stores_bc gpr_saved:4 )
> 0x0001150986f0 - 0x0900113594bc libjvm.so:: function>+0x95c (C++ saves_lr stores_bc gpr_saved:18 )
>
> The output contains "nameless function"  instead of the  (member) function
> names .
> Looks like this unwanted behavior  was introduced when switching to the
> xlc16  compiler.
>
> With xlc16, it looks like we have to  configure  the traceback table
> generation to get the output we need.
>
> The -qtbtable=full   setting can be used for this _
>
>
> https://www.ibm.com/support/knowledgecenter/SSGH4D_16.1.0/com.ibm.xlf161.aix.doc/compiler_ref/tbtable.html
>
>
>
> Bug / webrev :
>
> https://bugs.openjdk.java.net/browse/JDK-8227171
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8227171.0/
>
>
> Thanks, Matthias
>
>

Re: [8u] RFR: 8210761: libjsig is being compiled without optimization

[Thomas Stüfe]

On Wed, Jun 26, 2019 at 2:27 PM Severin Gehwolf  wrote:

> On Wed, 2019-06-26 at 14:21 +0200, Thomas Stüfe wrote:
> >
> > One curious question though about the original bug, was this really a
> > measurable performance issue? I would have thought that libjsig is
> > not invoked often enough for this to make a difference..
>
> It's a matter of missed optimizations. We've got tooling in RHEL 8
> which checks for native libraries which aren't being optimized. This
> was one of the flagged libs throughout all of OpenJDK. While it
> probably isn't needed for performance reasons, it's probly also just an
> oversight that opt flags didn't get passed properly for this library.
> It shouldn't do any harm.
>

Thanks for clarifying!

Cheers, Thomas


> Thanks,
> Severin
>
>

Re: [8u] RFR: 8210761: libjsig is being compiled without optimization

[Thomas Stüfe]

I'd be okay with doing this on AIX if Christoph is.

One curious question though about the original bug, was this really a
measurable performance issue? I would have thought that libjsig is not
invoked often enough for this to make a difference..

Cheers, Thomas



On Wed, Jun 26, 2019 at 2:05 PM Andrew John Hughes 
wrote:

>
>
> On 25/06/2019 10:04, Severin Gehwolf wrote:
> > Hi,
> >
> > Could somebody please review this 8u backport? The build system in 8 is
> > different, especially hotspot makefiles. Note, libjsig.so is part of
> > the hotspot build. The patch is different in 8 (over 11) due to this
> > reason. This is a Linux-only change and should be low risk. Thoughts?
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8210761
> > webrev:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210761/jdk8/01/webrev/
> >
> > Change in JDK 11:
> http://hg.openjdk.java.net/jdk-updates/jdk11u/rev/e7aa96dc0b1e
> >
> > Testing: Manual build of slowdebug/fastdebug/release on x86_64.
> > Fastdebug/release builds receive the -O3 flag, slowdebug not. As
> > expected.
> >
> > Thanks,
> > Severin
> >
>
> The 11u fix applies to all 'unix' platforms, as far as I can see. Should
> the 8u equivalent not also be applied to the solaris, bsd and aix
> subdirectories as well for consistency?
>
> Thanks,
> --
> Andrew :)
>
> Senior Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
>
> PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
> Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
> https://keybase.io/gnu_andrew
>
>

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[Thomas Stüfe]

Hi David,

Thank you for doing this, this looks all very good.

I wish though we had a clear whitelist of features to use or blacklist for
features to avoid. Most developers do not use Windows as a primary
platform, so it will always be a surprise whether Windows breaks in submit
tests.

I am also (a bit) concerned about C99 features creeping in which would
prevent verbatim backporting of patches to older releases. But let’s see if
that is really a problem in practice.

Thanks, Thomas


On Tue 21. May 2019 at 02:58, David Holmes  wrote:

> Thank you everyone for taking a look at this.
>
> Here is version 2:
>
> http://cr.openjdk.java.net/~dholmes/8224087/webrev.v2/
>
> Changes:
> - set c99 rather than gnu99
> - Volker's change for xlc to match gcc and clang
> - added short note to build doc (can do wiki later)
> - cosmetic change of name to make variable based on other feedback
> during the C++14 discussion
>
> Thanks,
> David
>
> On 20/05/2019 5:40 pm, David Holmes wrote:
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
> > webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/
> >
> > The need to remove a for-loop declaration expression to appease gcc 4.8
> > annoyed me enough to investigate setting C99 as our minimum allow
> > C-language level when compiling. It turned out to be a lot more complex
> > a situation than I thought due to toolchain quirks. See lots of details
> > in the bug report.
> >
> > To summarise the changes:
> > - gcc: force to -std=gnu99
> > - clang force to -std=gnu99
> > - Solaris studio - no effective change
> > - Visual Studio - no change
> > - xlc - no effective change (but we use the explicit flag rather than
> > accepting it as default)
> >
> > I've checked how this works with all the toolchains except xlc as I have
> > no access to that. Some assistance from someone who can verify the
> > correctness on xlc would be appreciated.
> >
> > Thanks,
> > David
>

Re: RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[Thomas Stüfe]

On Fri, Apr 5, 2019 at 10:51 AM David Holmes 
wrote:

> Hi Thomas,
>
> On 5/04/2019 6:23 pm, Thomas Stüfe wrote:
> > Hi,
> >
> > sorry, just sniping in from the side.
> >
> > About the motivation: the original intent of Yasumasas patch was to
> > reduce the length of the event log messages.
> >
> > I have a patch out there for RFR, since ~2 weeks or so, which largely
> > solves this problem:
> >
> > https://bugs.openjdk.java.net/browse/JDK-8220762
> >
> > The patch abolishes fixed-length string records in the event log in
> > favor of variable length strings, and therefore uses the event log
> > buffer much more efficiently. Truncation could still happen but is very
> > unlikely.
> >
> > The patch is out since some time and has no reviews yet (Yasumasa agreed
> > to review), I did not press it since we are all very busy. But, it would
> > solve this problem, and maybe we do not need this fix at all.
>
> It wouldn't solve the current problem - which is the invalidation of the
> precompiled header file. But after it is fixed we could regress
> Yasumasa's change which would fix the current problem.
>
> Cheers,
> David
>

Thanks. Okay, I agree.

In general, it would be useful to have a variant of __FILE__ which only
contains the filename.

..Thomas


>
> > Unless we want a generic solution to problems like this.
> >
> > Cheers, Thomas
> >
> >
> >
> >
> >
> > On Fri, Apr 5, 2019 at 8:00 AM David Holmes  > <mailto:david.hol...@oracle.com>> wrote:
> >
> > On 5/04/2019 3:23 pm, Ioi Lam wrote:
> >  >
> >  >
> >  > On 4/4/19 10:08 PM, David Holmes wrote:
> >  >> Adding back build-dev
> >  >>
> >  >> On 5/04/2019 2:41 pm, Ioi Lam wrote:
> >  >>> #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
> >  >>>
> >  >>> This make me a little uneasy, as it could potential point past
> the
> >  >>> end of the string.
> >  >>
> >  >> The intent of course is that that is impossible:
> >  >> - _FILE_ is an absolute file path within the repo:
> > /something/repo/file
> >  >> - FILE_MACRO_OFFSET gets you from / to the top-level repo
> directory
> >  >> leaving "file"
> >  >>
> >  >> so it has to be in bounds.
> >  >>
> >  >>> For safety, Is it possible to get some sort of assert to make
> sure
> >  >>> that __FILE__ always has more than FILE_MACRO_OFFSET characters?
> >  >>>
> >  >>> Maybe we can add a static object in macros.hpp with a
> constructor
> >  >>> that puts __FILE__ into a list, and then we can walk the list
> when
> >  >>> the VM starts up? E.g.
> >  >>>
> >  >>>  ...
> >  >>>
> >  >>>  #ifdef ASSERT
> >  >>>  static ThisFileChecker __this_file_checker(__FILE__);
> >  >>>  #endif
> >  >>>
> >  >>>  #endif // SHARE_UTILITIES_MACROS_HPP
> >  >>
> >  >> So basically you're not trusting the compiler and build system
> > to be
> >  >> correct here. :)
> >  >
> >  > I am sure the build system is correct . but it's complicated.
> >  >
> >  > BTW, we actually have generated sources that can live outside of
> the
> >  > source repo. And with luck, their names can actually be shorter
> than
> >  > FILE_MACRO_OFFSET.
> >
> > Excellent point! repo sources and generated sources need not be in
> the
> > same tree, so you cannot use one "offset" to handle them both.
> >
> > David
> > -
> >
> >
> >  > $ cd /tmp/mybld
> >  > $ find . -name \*.cpp
> >  > ./hotspot/variant-server/support/adlc/ad_x86_expand.cpp
> >  > ./hotspot/variant-server/support/adlc/ad_x86_pipeline.cpp
> >  > ./hotspot/variant-server/support/adlc/ad_x86_format.cpp
> >  > ./hotspot/variant-server/support/adlc/dfa_x86.cpp
> >  > ./hotspot/variant-server/support/adlc/ad_x86_misc.cpp
> >  > ./hotspot/variant-server/support/adlc/ad_x86_gen.cpp
> >  > ./hotspot/variant-server/support/adlc/ad_x86.cpp
> >  > ./hotspot/variant-server/support/adlc/ad_x86_peephole.cpp
&

Re: RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[Thomas Stüfe]

p.s.: Review is at hs-runtime-dev:
https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-March/033150.html
.

On Fri, Apr 5, 2019 at 10:23 AM Thomas Stüfe 
wrote:

> Hi,
>
> sorry, just sniping in from the side.
>
> About the motivation: the original intent of Yasumasas patch was to reduce
> the length of the event log messages.
>
> I have a patch out there for RFR, since ~2 weeks or so, which largely
> solves this problem:
>
> https://bugs.openjdk.java.net/browse/JDK-8220762
>
> The patch abolishes fixed-length string records in the event log in favor
> of variable length strings, and therefore uses the event log buffer much
> more efficiently. Truncation could still happen but is very unlikely.
>
> The patch is out since some time and has no reviews yet (Yasumasa agreed
> to review), I did not press it since we are all very busy. But, it would
> solve this problem, and maybe we do not need this fix at all.
>
> Unless we want a generic solution to problems like this.
>
> Cheers, Thomas
>
>
>
>
>
> On Fri, Apr 5, 2019 at 8:00 AM David Holmes 
> wrote:
>
>> On 5/04/2019 3:23 pm, Ioi Lam wrote:
>> >
>> >
>> > On 4/4/19 10:08 PM, David Holmes wrote:
>> >> Adding back build-dev
>> >>
>> >> On 5/04/2019 2:41 pm, Ioi Lam wrote:
>> >>> #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
>> >>>
>> >>> This make me a little uneasy, as it could potential point past the
>> >>> end of the string.
>> >>
>> >> The intent of course is that that is impossible:
>> >> - _FILE_ is an absolute file path within the repo: /something/repo/file
>> >> - FILE_MACRO_OFFSET gets you from / to the top-level repo directory
>> >> leaving "file"
>> >>
>> >> so it has to be in bounds.
>> >>
>> >>> For safety, Is it possible to get some sort of assert to make sure
>> >>> that __FILE__ always has more than FILE_MACRO_OFFSET characters?
>> >>>
>> >>> Maybe we can add a static object in macros.hpp with a constructor
>> >>> that puts __FILE__ into a list, and then we can walk the list when
>> >>> the VM starts up? E.g.
>> >>>
>> >>>  ...
>> >>>
>> >>>  #ifdef ASSERT
>> >>>  static ThisFileChecker __this_file_checker(__FILE__);
>> >>>  #endif
>> >>>
>> >>>  #endif // SHARE_UTILITIES_MACROS_HPP
>> >>
>> >> So basically you're not trusting the compiler and build system to be
>> >> correct here. :)
>> >
>> > I am sure the build system is correct . but it's complicated.
>> >
>> > BTW, we actually have generated sources that can live outside of the
>> > source repo. And with luck, their names can actually be shorter than
>> > FILE_MACRO_OFFSET.
>>
>> Excellent point! repo sources and generated sources need not be in the
>> same tree, so you cannot use one "offset" to handle them both.
>>
>> David
>> -
>>
>>
>> > $ cd /tmp/mybld
>> > $ find . -name \*.cpp
>> > ./hotspot/variant-server/support/adlc/ad_x86_expand.cpp
>> > ./hotspot/variant-server/support/adlc/ad_x86_pipeline.cpp
>> > ./hotspot/variant-server/support/adlc/ad_x86_format.cpp
>> > ./hotspot/variant-server/support/adlc/dfa_x86.cpp
>> > ./hotspot/variant-server/support/adlc/ad_x86_misc.cpp
>> > ./hotspot/variant-server/support/adlc/ad_x86_gen.cpp
>> > ./hotspot/variant-server/support/adlc/ad_x86.cpp
>> > ./hotspot/variant-server/support/adlc/ad_x86_peephole.cpp
>> > ./hotspot/variant-server/support/adlc/ad_x86_clone.cpp
>> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_expand.cpp
>> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_pipeline.cpp
>> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_format.cpp
>> > ./hotspot/variant-server/gensrc/adfiles/dfa_x86.cpp
>> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_misc.cpp
>> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_gen.cpp
>> > ./hotspot/variant-server/gensrc/adfiles/ad_x86.cpp
>> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_peephole.cpp
>> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_clone.cpp
>> > ./hotspot/variant-server/gensrc/jvmtifiles/jvmtiEnter.cpp
>> >
>> ./hotspot/variant-server/gensrc/jvmtifiles/bytecodeInterpreterWithChecks.cpp
>>
>> >
>> > ./hotspot/variant-server/

Re: RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[Thomas Stüfe]

Hi,

sorry, just sniping in from the side.

About the motivation: the original intent of Yasumasas patch was to reduce
the length of the event log messages.

I have a patch out there for RFR, since ~2 weeks or so, which largely
solves this problem:

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

The patch abolishes fixed-length string records in the event log in favor
of variable length strings, and therefore uses the event log buffer much
more efficiently. Truncation could still happen but is very unlikely.

The patch is out since some time and has no reviews yet (Yasumasa agreed to
review), I did not press it since we are all very busy. But, it would solve
this problem, and maybe we do not need this fix at all.

Unless we want a generic solution to problems like this.

Cheers, Thomas





On Fri, Apr 5, 2019 at 8:00 AM David Holmes  wrote:

> On 5/04/2019 3:23 pm, Ioi Lam wrote:
> >
> >
> > On 4/4/19 10:08 PM, David Holmes wrote:
> >> Adding back build-dev
> >>
> >> On 5/04/2019 2:41 pm, Ioi Lam wrote:
> >>> #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
> >>>
> >>> This make me a little uneasy, as it could potential point past the
> >>> end of the string.
> >>
> >> The intent of course is that that is impossible:
> >> - _FILE_ is an absolute file path within the repo: /something/repo/file
> >> - FILE_MACRO_OFFSET gets you from / to the top-level repo directory
> >> leaving "file"
> >>
> >> so it has to be in bounds.
> >>
> >>> For safety, Is it possible to get some sort of assert to make sure
> >>> that __FILE__ always has more than FILE_MACRO_OFFSET characters?
> >>>
> >>> Maybe we can add a static object in macros.hpp with a constructor
> >>> that puts __FILE__ into a list, and then we can walk the list when
> >>> the VM starts up? E.g.
> >>>
> >>>  ...
> >>>
> >>>  #ifdef ASSERT
> >>>  static ThisFileChecker __this_file_checker(__FILE__);
> >>>  #endif
> >>>
> >>>  #endif // SHARE_UTILITIES_MACROS_HPP
> >>
> >> So basically you're not trusting the compiler and build system to be
> >> correct here. :)
> >
> > I am sure the build system is correct . but it's complicated.
> >
> > BTW, we actually have generated sources that can live outside of the
> > source repo. And with luck, their names can actually be shorter than
> > FILE_MACRO_OFFSET.
>
> Excellent point! repo sources and generated sources need not be in the
> same tree, so you cannot use one "offset" to handle them both.
>
> David
> -
>
>
> > $ cd /tmp/mybld
> > $ find . -name \*.cpp
> > ./hotspot/variant-server/support/adlc/ad_x86_expand.cpp
> > ./hotspot/variant-server/support/adlc/ad_x86_pipeline.cpp
> > ./hotspot/variant-server/support/adlc/ad_x86_format.cpp
> > ./hotspot/variant-server/support/adlc/dfa_x86.cpp
> > ./hotspot/variant-server/support/adlc/ad_x86_misc.cpp
> > ./hotspot/variant-server/support/adlc/ad_x86_gen.cpp
> > ./hotspot/variant-server/support/adlc/ad_x86.cpp
> > ./hotspot/variant-server/support/adlc/ad_x86_peephole.cpp
> > ./hotspot/variant-server/support/adlc/ad_x86_clone.cpp
> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_expand.cpp
> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_pipeline.cpp
> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_format.cpp
> > ./hotspot/variant-server/gensrc/adfiles/dfa_x86.cpp
> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_misc.cpp
> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_gen.cpp
> > ./hotspot/variant-server/gensrc/adfiles/ad_x86.cpp
> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_peephole.cpp
> > ./hotspot/variant-server/gensrc/adfiles/ad_x86_clone.cpp
> > ./hotspot/variant-server/gensrc/jvmtifiles/jvmtiEnter.cpp
> >
> ./hotspot/variant-server/gensrc/jvmtifiles/bytecodeInterpreterWithChecks.cpp
>
> >
> > ./hotspot/variant-server/gensrc/jvmtifiles/jvmtiEnterTrace.cpp
> >
> >>
> >> Would it suffice to put a static assert in a common header, like
> >> macros.hpp, that verifies the length of _FILE_ or is that not
> >> available statically?
> >>
> > I don't know how a static assert would work. That's why I suggested the
> > static object with a constructor.
> >
> > Thanks
> > - Ioi
> >
> >> Cheers,
> >> David
> >>
> >>>
> >>> Thanks
> >>> - Ioi
> >>>
> >>>
> >>> On 4/4/19 9:04 PM, David Holmes wrote:
>  Hi Erik,
> 
>  Build and hotspot changes seem okay.
> 
>  Thanks,
>  David
> 
>  On 5/04/2019 8:03 am, Erik Joelsson wrote:
> >
> > On 2019-04-04 14:26, Kim Barrett wrote:
> >>
> >> OK, I can do that.
> >>
> >>
> --
>
> >>
> >> src/hotspot/share/utilities/macros.hpp
> >>   645 #if FILE_MACRO_OFFSET
> >>   646 #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
> >>   647 #else
> >>   648 #define THIS_FILE __FILE__
> >>   649 #endif
> >>
> >> Is the "#if FILE_MACRO_OFFSET" an intentional test for 0, or is this
> >> an implicit test for "defined"?
> >>
> >> If the form

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8217707: JNICALL declaration breaks Splash screen functions

[Thomas Stüfe]

Looks fine.

Cheers, Thomas

On Thu, Mar 28, 2019 at 5:11 PM Alexey Ivanov 
wrote:

> Any volunteers for review?
>
> On 24/03/2019 19:18, Alexey Ivanov wrote:
> > Hi,
> >
> > Please review the fix for jdk 13.
> >
> > bug: https://bugs.openjdk.java.net/browse/JDK-8217707
> > webrev: http://cr.openjdk.java.net/~aivanov/8217707/webrev.0/
> >
> > Description:
> > Splash screen functionality is broken in 32 bit Windows. It's because
> > the functions in splashscreen.dll are exported with decorated names
> > now, yet they're looked up by the undecorated names.
> >
> > The easiest way to reproduce the problem is to run SwingSet2:
> > java -jar SwingSet2.jar
> >
> >
> > Fix:
> > The fix removes JNICALL (__stdcall) declarations from splash screen
> > functions. Thus the functions are exported undecorated.
> >
> > I've run SplashScreen jtreg tests, all tests pass.
> >
> > This is a follow-up fix to
> > https://bugs.openjdk.java.net/browse/JDK-8201226
> > https://bugs.openjdk.java.net/browse/JDK-8202476
> > which re-enabled 32 bit Windows post removal of mapfiles / compiler
> > options.
> >
> >
> > Thank you in advance.
> >
> > Regards,
> > Alexey
>
>

Re: [PATCH] Support for building using WSL (Windows Subsystem for Linux) on Windows

[Thomas Stüfe]

Neat! Thank you.

..Thomas

On Wed, Jan 16, 2019 at 5:36 PM Erik Joelsson 
wrote:

> Build support is in and you should be able to follow the documentation in
> doc/building.md. Test support is still waiting on a jtreg release and a
> patch to use it, but even with that, there are tests (at least shell tests)
> that will not work properly.
>
> /Erik
> On 2019-01-16 00:49, Thomas Stüfe wrote:
>
> Hi guys,
>
> Just wanted to know what the state is. Did you already add the support for
> WSL or is this still WIP? If it should work, is there a documentation
> somewhere I could follow?
>
> Thank you!
>
> Thomas
>
>
> On Sat, Dec 22, 2018 at 4:55 AM Andrew Luo <
> andrewluotechnolog...@outlook.com> wrote:
>
>> Just wanted to update the thread with the issues we discovered with WSL
>> while adding support to the OpenJDK build system.  I reported these issues
>> to the WSL team for all except for one of the bugs, which I'm still
>> investigating.
>>
>> GenerateCurrencyData.java - issue with Properties.load(System.in):
>> https://github.com/Microsoft/WSL/issues/3723
>> Issue with directly calling cmd.exe to transform long path to short path:
>> https://github.com/Microsoft/WSL/issues/3724
>> Calling from Linux to Win32 with untransformable Linux paths in a WSLENV
>> path environment variable causes extra output:
>> https://github.com/Microsoft/WSL/issues/3725
>> Spp.java - still investigating this
>>
>> Thanks,
>>
>> -Andrew
>>
>> -Original Message-
>> From: Erik Joelsson 
>> Sent: Thursday, December 20, 2018 1:51 AM
>> To: Andrew Luo ; Magnus Ihse Bursie <
>> magnus.ihse.bur...@oracle.com>
>> Cc: build-dev@openjdk.java.net
>> Subject: Re: [PATCH] Support for building using WSL (Windows Subsystem
>> for Linux) on Windows
>>
>> I've updated and two builds in a row have now succeeded. I will keep
>> running, but it does seem likely that the new version has fixed the issue.
>>
>> /Erik
>>
>> On 2018-12-20 09:44, Erik Joelsson wrote:
>> > Hello,
>> >
>> > On 2018-12-19 19:40, Andrew Luo wrote:
>> >> Hi Erik,
>> >>
>> >> Which target are you using (make exploded-image?)?  I never saw this
>> >> error while building on my machine (I've built about 10 times now,
>> >> I'm on Windows 10 1809 for what it's worth).  Perhaps I can try to
>> >> reproduce this on my system as well...
>> >
>> > The target doesn't really matter that much, it's failing when building
>> > java modules, so exploded-image should reproduce it. I have built
>> > successfully as well, so this only happens intermittently. Here is the
>> > environment string from my system:
>> >
>> > WSL version Ubuntu 16.04.4 LTS #471-Microsoft Fri Dec 07 20:04:00 PST
>> > 2018 4.4.0-17134-Microsoft (on Windows build 10.0.17134.471)
>> >
>> > In System about, it identifies itself as Windows 10 Pro 1803, so that
>> > looks older than yours. I will see if I can update.
>> >
>> > I should also note that deleting build output is not necessary (and
>> > probably not affecting) success or failure on rebuild. From what I can
>> > see what happens is: make runs the find command to find all java
>> > source files and puts that list of files as prerequisites to the java
>> > compile rule, then when evaluating the rule, it sometimes fails to
>> > resolve a file. This would seem like a bug in the filesystem to me.
>> >
>> > /Erik
>> >
>> >> Thanks,
>> >>
>> >> -Andrew
>> >>
>> >> -Original Message-
>> >> From: Erik Joelsson 
>> >> Sent: Wednesday, December 19, 2018 8:28 AM
>> >> To: Andrew Luo ; Magnus Ihse
>> >> Bursie 
>> >> Cc: build-dev@openjdk.java.net
>> >> Subject: Re: [PATCH] Support for building using WSL (Windows
>> >> Subsystem for Linux) on Windows
>> >>
>> >> I'm now seeing intermittent build failures that look like this:
>> >>
>> >> make[3]: *** No rule to make target
>> >> '/mnt/d/erik/jdk-sandbox/open/src/java.security.jgss/share/classes/su
>> >> n/security/krb5/internal/TGSReq.java',
>> >>
>> >> needed by
>> >>
>> '/mnt/d/erik/jdk-sandbox/build/windows-x86_64-server-release/jdk/modules/java.security.jgss/_the.java.security.jgss_batch'.
>>
>> >>
>>

Re: [PATCH] Support for building using WSL (Windows Subsystem for Linux) on Windows

[Thomas Stüfe]

Hi guys,

Just wanted to know what the state is. Did you already add the support for
WSL or is this still WIP? If it should work, is there a documentation
somewhere I could follow?

Thank you!

Thomas


On Sat, Dec 22, 2018 at 4:55 AM Andrew Luo <
andrewluotechnolog...@outlook.com> wrote:

> Just wanted to update the thread with the issues we discovered with WSL
> while adding support to the OpenJDK build system.  I reported these issues
> to the WSL team for all except for one of the bugs, which I'm still
> investigating.
>
> GenerateCurrencyData.java - issue with Properties.load(System.in):
> https://github.com/Microsoft/WSL/issues/3723
> Issue with directly calling cmd.exe to transform long path to short path:
> https://github.com/Microsoft/WSL/issues/3724
> Calling from Linux to Win32 with untransformable Linux paths in a WSLENV
> path environment variable causes extra output:
> https://github.com/Microsoft/WSL/issues/3725
> Spp.java - still investigating this
>
> Thanks,
>
> -Andrew
>
> -Original Message-
> From: Erik Joelsson 
> Sent: Thursday, December 20, 2018 1:51 AM
> To: Andrew Luo ; Magnus Ihse Bursie <
> magnus.ihse.bur...@oracle.com>
> Cc: build-dev@openjdk.java.net
> Subject: Re: [PATCH] Support for building using WSL (Windows Subsystem for
> Linux) on Windows
>
> I've updated and two builds in a row have now succeeded. I will keep
> running, but it does seem likely that the new version has fixed the issue.
>
> /Erik
>
> On 2018-12-20 09:44, Erik Joelsson wrote:
> > Hello,
> >
> > On 2018-12-19 19:40, Andrew Luo wrote:
> >> Hi Erik,
> >>
> >> Which target are you using (make exploded-image?)?  I never saw this
> >> error while building on my machine (I've built about 10 times now,
> >> I'm on Windows 10 1809 for what it's worth).  Perhaps I can try to
> >> reproduce this on my system as well...
> >
> > The target doesn't really matter that much, it's failing when building
> > java modules, so exploded-image should reproduce it. I have built
> > successfully as well, so this only happens intermittently. Here is the
> > environment string from my system:
> >
> > WSL version Ubuntu 16.04.4 LTS #471-Microsoft Fri Dec 07 20:04:00 PST
> > 2018 4.4.0-17134-Microsoft (on Windows build 10.0.17134.471)
> >
> > In System about, it identifies itself as Windows 10 Pro 1803, so that
> > looks older than yours. I will see if I can update.
> >
> > I should also note that deleting build output is not necessary (and
> > probably not affecting) success or failure on rebuild. From what I can
> > see what happens is: make runs the find command to find all java
> > source files and puts that list of files as prerequisites to the java
> > compile rule, then when evaluating the rule, it sometimes fails to
> > resolve a file. This would seem like a bug in the filesystem to me.
> >
> > /Erik
> >
> >> Thanks,
> >>
> >> -Andrew
> >>
> >> -Original Message-
> >> From: Erik Joelsson 
> >> Sent: Wednesday, December 19, 2018 8:28 AM
> >> To: Andrew Luo ; Magnus Ihse
> >> Bursie 
> >> Cc: build-dev@openjdk.java.net
> >> Subject: Re: [PATCH] Support for building using WSL (Windows
> >> Subsystem for Linux) on Windows
> >>
> >> I'm now seeing intermittent build failures that look like this:
> >>
> >> make[3]: *** No rule to make target
> >> '/mnt/d/erik/jdk-sandbox/open/src/java.security.jgss/share/classes/su
> >> n/security/krb5/internal/TGSReq.java',
> >>
> >> needed by
> >>
> '/mnt/d/erik/jdk-sandbox/build/windows-x86_64-server-release/jdk/modules/java.security.jgss/_the.java.security.jgss_batch'.
>
> >>
> >> Stop.
> >>
> >> The particular file that's missing varies, and deleting the output
> >> dir for that module and rebuild works. The common pattern seems to be
> >> upper case letters in the file name of the source file.
> >>
> >> I will investigate some more.
> >>
> >> /Erik
> >>
> >> On 2018-12-19 06:18, Erik Joelsson wrote:
> >>> I can also report that on the Windows 10 machine I'm testing this
> >>> on, "make bundles" on Cygwin took 12m56s and in WSL 8m39s, so a
> >>> pretty big improvement!
> >>>
> >>> /Erik
> >>>
> >>> On 2018-12-19 03:45, Erik Joelsson wrote:
>  Hello,
> 
>  On 2018-12-19 00:19, Erik Joelsson wrote:
> > Hello Andrew,
> >
> > On 2018-12-18 12:45, Andrew Luo wrote:
> >> Hi Erik/Magnus,
> >>
> >> I've attached my latest changes:
> >>
> >> 1. Fixed a file I forgot to revert in my previous change while
> >> trying something out...
> >> 2. Added information about case sensitivity of the OpenJDK build
> >> directory (yes, I did use the make target to generate the HTML
> >> file) 3. Fixed Cygwin (hopefully, I don't have a Cygwin
> >> environment to verify this)
> > I will take this patch for a spin and see.
> >
>  After applying a fix for
>  https://bugs.openjdk.java.net/browse/JDK-8215635, I managed to
>  build everything as well. I pushed some minor adjustments to make
>  Cygwin work too.
> 
> >>

Re: RFR(XS): 8214063: [AIX] Disable symbol visibility flags

[Thomas Stüfe]

On Tue, Nov 27, 2018 at 4:35 PM Volker Simonis  wrote:
>
> Hi,
>
> can I please have a review for the following trivial change which
> simply disables the symbol visibility flags on AIX:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8214063/
> https://bugs.openjdk.java.net/browse/JDK-8214063
>

-# On AIX/xlc we need at least xlc 13.1 for the symbol hiding (see JDK-8214063)
+# On AIX/xlc we need at least xlc 13.1 for the symbol hiding (see
JDK-8214063). We do not yet support xlc 13, so lets not pass
qvisibility=hidden

> Change "8202322: AIX: symbol visibility flags not support on xlc 12.1"
> [1] blindly introduced these flags for all xlC compiler versions >
> 12.1 without ever testing it (which should not have happened).

Dude you are off. Did you mean "8200178 - Remove mapfiles for JDK
native libraries"? Matthias just removed the flag conditionally for
xlc 12.1

> Now
> that people are starting to really use xlC 13 it turns out that there
> is more to do than just enabling the flags. This future work is
> covered by "8204541: Correctly support AIX xlC 13.1 symbol visibility
> flags".
>
> Thank you and best regards,
> Volker
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8202322
> [2] https://bugs.openjdk.java.net/browse/JDK-8204541

Cheers Thomas

Re: RFR(S): 8214343: Handle the absence of Xrandr more generically

[Thomas Stüfe]

Looks good Volker. I agree, NO_XRANDR is reasonable and better than HAVE_XRANDR.

I have to say, the amount of work this took is insane for the size of
the problem involved.

Cheers, Thomas

On Tue, Nov 27, 2018 at 2:54 PM Volker Simonis  wrote:
>
> Hi,
>
> can I please have a review for the following trivial change which
> handles the absence of Xrandr more generically:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8214343/
> https://bugs.openjdk.java.net/browse/JDK-8214343
>
> Change JDK-8213944 fixed the build on AIX (which has no Xrandr) by
> conditionally excluding the relevant parts on AIX with the help of
> preprocessor defines. On the mailing lists the wish was expressed to
> handle the absence of Xrandr more generically during the configure
> step, so here comes the corresponding change.
>
> In contrast to the suggestions on the previous mail thread I define
> "NO_XRENDER" if we're configuring on AIX. This avoids clobbering all
> the other platforms which support Xrandr with yet another command line
> define of the form "-DHAVE_XRANDR". Instead, only the corresponding
> compiler options on AIX will now contain "-DNO_XRANDR". Other
> platforms which don't support Xrandr can now easily define NO_XRANDR
> as well.
>
> Thank you and best regards,
> Volker

Re: [OpenJDK 2D-Dev] RFR(XS): 8213944: Fix AIX build after the removal of Xrandr.h and add a configure check for it

[Thomas Stüfe]

Side note, I wondered whether IBM does anything original in their
openj9 (the jdk parts they forked off OpenJDK) since they are
targetting AIX too and must be having the same build error. But I
cannot find anything they did:

---
thomas@t450:/shared/projects/openjdk/j9/openj9-openjdk-jdk$ git log --
src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c
commit 32743992c57206a56b7c1e08f6b870458a481c94
Author: simonis 
Date:   Thu Nov 22 09:44:02 2018 +0100

8213944: Fix AIX build after the removal of Xrandr.h and add a
configure check for it
Reviewed-by: shade, erikj, stuefe, ihse, goetz

commit 5ca5123bc3859e335c1173a5969bde487e6b5ad4
Author: prr 
Date:   Wed Oct 31 16:58:37 2018 -0700

8210863: Remove Xrandr include files from JDK sources
Reviewed-by: serb
---

So, I wonder, they just live with a broken openj9 build for 22 days?

..Thomas


On Mon, Nov 26, 2018 at 9:37 PM Phil Race  wrote:
>
> Well .. I see this was pushed whilst I was on vacation.
> I would not have voted for the fix in its current form. The
> "HAVE_XRANDR" would
> have been  less bad. If (say) HPUX has the same issue you
> haven't really helped them with an AIX specific change in the source file.
> It would have been better to confine all of that decision making to the
> make files.
>
> Can we re-do this ? And since it'll be less urgent the follow up can be
> pushed to jdk/client.
>
> But I really don't get why AIX doesn't have XRANDR.
> I understood that it might not have GDK - which is why - you may remember -
> we checked into that when looking at options for the Robot imported sources
> and deferred that, but XRANDR ??? I think this is an AIX "bug" which should
> be reported to IBM.
>
> -phil.
>
>
> On 11/20/18 10:13 AM, Volker Simonis wrote:
> > Thanks everybody for the reviews.
> >
> > If nobody raises a "Veto" (Phil?) I plan to push this fix tomorrow in
> > its current form.
> >
> > I've also run it through the submit repo and got an error on Windows
> > for the test "runtime/modules/JVMDefineModule.java" which seems
> > completely unrelated to my change which only touches the X11
> > implementation on Unix. Can somebody please confirm that?
> >
> > [Mach5] mach5-one-simonis-JDK-8213944-20181120-1629-11082: FAILED,
> > Failed tests: 1
> >
> > runtime/modules/JVMDefineModule.java tier1 windows-x64-debug othervm
> > driver ExitCode: -1073741819
> >
> > Mach5 Tasks Results Summary
> >
> > UNABLE_TO_RUN: 0
> > FAILED: 0
> > EXECUTED_WITH_FAILURE: 1
> > KILLED: 0
> > PASSED: 75
> > NA: 0
> >
> > Thank you and best regards,
> > Volker
> >
> > On Tue, Nov 20, 2018 at 12:05 PM Magnus Ihse Bursie
> >  wrote:
> >>
> >>
> >> On 2018-11-19 18:56, Volker Simonis wrote:
> >>> Hi Phil,
> >>>
> >>> I'd like to kindly ask you to suggest how we can proceed with this issue.
> >>>
> >>> As I wrote before, Xrandr is not officially supported on AIX and there
> >>> are no official packages available for it. There are some OpenSource
> >>> sites for AIX which provide Xrandr, but they are all not compatible
> >>> with the default native libraries (e.g. the open source Xrandr package
> >>> depends on another open source package of Xrender.so.1 but the system
> >>> only provides Xrender.so.0 natively). We can't compile the whole JDK
> >>> (i.e. libawt_xawt.so) against some open source package of Xrender.so.1
> >>> because that simply won't be available on the majority of systems.
> >>>
> >>> Remember that forcing people to install these open-source packages
> >>> even as a build dependency is like expecting Linux users to install
> >>> some non-official packages just to be able to build. Especially in
> >>> corporate environments that's not easy. Moreover the benefit would be
> >>> really minimal, because the Xrandr functionality won't be available at
> >>> runtime anyway.
> >>>
> >>> So to cut a long story short, I see two options:
> >>>
> >>> 1. Go with my current patch (ugly but efficient)
> >>>
> >>> 2. Check-in in an AIX specific version of XRander.h/randr.h under
> >>> src/java.desktop/aix (OK for me, but that would actually negate the
> >>> initial purpose of "8210863: Remove Xrandr include files")
> >>>
> >>> Do you have a better proposal?
> >> I think the change look good, and I vote for strategy 1. As Thomas
> >> suggested, if the AIX ifdefs look bad we can create a new define, but
> >> I'm not sure that's really helpful - after all, it's just on AIX we
> >> currently have no r&r. Having a define would mostly be needed if it was
> >> multiple OSes, or similar more complex situations, that would have/not
> >> have the r&r extension.
> >>
> >> Yet another solution, to get rid of the ifdefs, is to move the relevant
> >> Xranrd dependent functions into a new, separate file, like
> >> awt_GraphicsEnv_randr.c, and then in the build exclude it on AIX (or,
> >> perhaps if it's worth the trouble, on all platforms where configure did
> >> not find Xrandr).
> >>
> >> /Magnus
> >>
> >>> Thank you and best regards,
> >>> Volker
> >>>
>

Re: bash completion for make targets

[Thomas Stüfe]

:) Thank you Magnus!
On Wed, Nov 21, 2018 at 1:03 PM Magnus Ihse Bursie
 wrote:
>
> On 2018-11-20 14:30, Thomas Stüfe wrote:
>
> > Hi Magnus,
> >
> > The problem did vanish for me. Tab completion works again in all my
> > configurations. On zero, which I do not build that often, I had to run
> > reconfigure.
> >
> > Sorry, I lost track of this one. I am not sure when it started working
> > again. I even synced before Severins change, but it worked with that
> > version too, so either this was a local fluke or it was fixed before
> > 8213736.
> Great that it works again. I don't care where it was fixed, only that it
> works. :) Sometimes this gets stuck on incorrect updates of the
> generated targets list in configurations as well, so it might have
> actually "broke" at one point but not been discovered, since the target
> list was not regenerated. And similarly "fixed" at some later point but
> still not been discovered as fixed, due to a bad cache. If you ever get
> trouble with tab expansion again, make sure to start by cleaning out the
> entire build directory (if only temporary with "mv build build-old").
>
> /Magnus
> >
> > Cheers, Thomas
> > On Tue, Nov 20, 2018 at 12:56 PM Magnus Ihse Bursie
> >  wrote:
> >> On 2018-11-08 12:20, Thomas Stüfe wrote:
> >>> On Thu, Nov 8, 2018 at 12:13 PM Magnus Ihse Bursie
> >>>  wrote:
> >>>> On 2018-11-08 12:10, Thomas Stüfe wrote:
> >>>>
> >>>>> Hi all,
> >>>>>
> >>>>> This may be a stupid question, and not that important, but bash
> >>>>> completion for targets like "clean" and "images" stopped working for
> >>>>> me. "reconfigure" still works thouugh. Did anything change?
> >>>>>
> >>>>> Best Regards, Thomas
> >>>> Not intentionally. It can be that there were too many targets. Does the
> >>>> recent fix for JDK-8213338 help?
> >>>>
> >>> Unfortunately no.
> >> Bummer. :-( Is it better after JDK-8213736? This too affected target
> >> handling.
> >>
> >> If not, does "make print-targets" work properly for you? Does it work
> >> differently if you have zero, exactly one, many configurations? (This is
> >> one of the complications for make tab expansions -- theoretically
> >> different configurations can have different targets available to them.)
> >>
> >> /Magnus
> >>
> >>> ..Thomas
> >>>
> >>>> /Magnus
>

Re: RFR(XS): 8214120: [REDO] Fix sun.awt.nativedebug on X11 platforms

[Thomas Stüfe]

Hi Volker,

looks good to me. Lets wait for jdk-submit though - especially since
you changed linking on Windows too by adding JNIEXPORT.

Gruss Thomas
On Tue, Nov 20, 2018 at 6:13 PM Volker Simonis  wrote:
>
> Hi,
>
> so here's a reworked version of my previous change:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8214120/
> https://bugs.openjdk.java.net/browse/JDK-8214120
>
> In addition to my initial change (which unfortunately broke the
> Solaris build) the new version has to JNIEXPORT the following methods
> from libawt.so otherwise they won't be visible in libawt_xawt.so on
> some platforms which make symbols local by default (e.g. Solaris):
>
> DAssert_Impl
> DTrace_PrintFunction
> DTrace_VPrint
> DTrace_VPrintln
>
> Local tests on Linux/AIX and Solaris succeeded. The patch is currently
> building in the submit repo.
>
> Thank you and best regards,
> Volker

Re: bash completion for make targets

[Thomas Stüfe]

Hi Magnus,

The problem did vanish for me. Tab completion works again in all my
configurations. On zero, which I do not build that often, I had to run
reconfigure.

Sorry, I lost track of this one. I am not sure when it started working
again. I even synced before Severins change, but it worked with that
version too, so either this was a local fluke or it was fixed before
8213736.

Cheers, Thomas
On Tue, Nov 20, 2018 at 12:56 PM Magnus Ihse Bursie
 wrote:
>
> On 2018-11-08 12:20, Thomas Stüfe wrote:
> > On Thu, Nov 8, 2018 at 12:13 PM Magnus Ihse Bursie
> >  wrote:
> >> On 2018-11-08 12:10, Thomas Stüfe wrote:
> >>
> >>> Hi all,
> >>>
> >>> This may be a stupid question, and not that important, but bash
> >>> completion for targets like "clean" and "images" stopped working for
> >>> me. "reconfigure" still works thouugh. Did anything change?
> >>>
> >>> Best Regards, Thomas
> >> Not intentionally. It can be that there were too many targets. Does the
> >> recent fix for JDK-8213338 help?
> >>
> > Unfortunately no.
> Bummer. :-( Is it better after JDK-8213736? This too affected target
> handling.
>
> If not, does "make print-targets" work properly for you? Does it work
> differently if you have zero, exactly one, many configurations? (This is
> one of the complications for make tab expansions -- theoretically
> different configurations can have different targets available to them.)
>
> /Magnus
>
> >
> > ..Thomas
> >
> >> /Magnus
>

Re: (trivial, urgent) RFR(xxs): 8214075: [BACKOUT] 8214007: Fix sun.awt.nativedebug on X11 platforms

[Thomas Stüfe]

Thanks guys. I pushed.

..Thomas
On Mon, Nov 19, 2018 at 10:21 PM David Holmes  wrote:
>
> On 20/11/2018 7:17 am, Thomas Stüfe wrote:
> > p.s. does this need a jdk-submit run?
>
> No. Please push. If there are any issues I'll follow up.
>
> Thanks,
> David
>
> > I'm about to call it a day, and since jdk-submit takes 4-6 hours, that
> > would mean someone else would have to push the fix.
> >
> > ..Thomas
> > On Mon, Nov 19, 2018 at 10:13 PM Thomas Stüfe  
> > wrote:
> >>
> >> Hi all,
> >>
> >> may I please have a quick review. We want to backout 8214007 for now
> >> since it breaks Solaris.
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8214075
> >>
> >> http://cr.openjdk.java.net/~stuefe/webrevs/8214075-backout-8214007/webrev.00/webrev/
> >>
> >> Thanks, Thomas

Re: (trivial, urgent) RFR(xxs): 8214075: [BACKOUT] 8214007: Fix sun.awt.nativedebug on X11 platforms

[Thomas Stüfe]

p.s. does this need a jdk-submit run?

I'm about to call it a day, and since jdk-submit takes 4-6 hours, that
would mean someone else would have to push the fix.

..Thomas
On Mon, Nov 19, 2018 at 10:13 PM Thomas Stüfe  wrote:
>
> Hi all,
>
> may I please have a quick review. We want to backout 8214007 for now
> since it breaks Solaris.
>
> https://bugs.openjdk.java.net/browse/JDK-8214075
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8214075-backout-8214007/webrev.00/webrev/
>
> Thanks, Thomas

(trivial, urgent) RFR(xxs): 8214075: [BACKOUT] 8214007: Fix sun.awt.nativedebug on X11 platforms

[Thomas Stüfe]

Hi all,

may I please have a quick review. We want to backout 8214007 for now
since it breaks Solaris.

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

http://cr.openjdk.java.net/~stuefe/webrevs/8214075-backout-8214007/webrev.00/webrev/

Thanks, Thomas

Re: [OpenJDK 2D-Dev] RFR(XS): 8213944: Fix AIX build after the removal of Xrandr.h and add a configure check for it

[Thomas Stüfe]

FWIW I am very much in favor of (1) and if aesthetics are a problem in
awt_GraphicsEnv.c, a "HAVE_XRANDR" define would make things a bit
prettier...

just my 2c.

On Mon, Nov 19, 2018 at 6:58 PM Volker Simonis  wrote:
>
> Hi Phil,
>
> I'd like to kindly ask you to suggest how we can proceed with this issue.
>
> As I wrote before, Xrandr is not officially supported on AIX and there
> are no official packages available for it. There are some OpenSource
> sites for AIX which provide Xrandr, but they are all not compatible
> with the default native libraries (e.g. the open source Xrandr package
> depends on another open source package of Xrender.so.1 but the system
> only provides Xrender.so.0 natively). We can't compile the whole JDK
> (i.e. libawt_xawt.so) against some open source package of Xrender.so.1
> because that simply won't be available on the majority of systems.
>
> Remember that forcing people to install these open-source packages
> even as a build dependency is like expecting Linux users to install
> some non-official packages just to be able to build. Especially in
> corporate environments that's not easy. Moreover the benefit would be
> really minimal, because the Xrandr functionality won't be available at
> runtime anyway.
>
> So to cut a long story short, I see two options:
>
> 1. Go with my current patch (ugly but efficient)
>
> 2. Check-in in an AIX specific version of XRander.h/randr.h under
> src/java.desktop/aix (OK for me, but that would actually negate the
> initial purpose of "8210863: Remove Xrandr include files")
>
> Do you have a better proposal?
>
> Thank you and best regards,
> Volker
>
> On Fri, Nov 16, 2018 at 11:22 AM Volker Simonis
>  wrote:
> >
> > On Thu, Nov 15, 2018 at 6:01 PM Philip Race  wrote:
> > >
> > > PS I am not sure why xrandr headers would not be available for AIX.
> > > They are a standard part of the xdistribution.
> > >
> >
> > I'm not an X11 guru, but as far as I understand, xrandr is an
> > extension and as such it doesn't have to be supported by every
> > implementation. To the best of my knowledge (I've just started another
> > poll among some experts) AIX doesn't support Xrandr and does not have
> > the corresponding headers.
> >
> > > If true, think what you are going to have to do is add a
> > > --with-xrandr-include option
> > > and provide it that way.
> > >
> >
> > What if there are no standard Xrandr headers on a platform? Do you
> > really want to force users to get them from some dubious sources just
> > for building the OpenJDK? Sorry, but I don't think that's a good
> > solution. Than I'd rather prefer the ugly ifdefs (or I check the
> > headers back in again in an AIX-specific directory :)
> >
> > Thank you and best regards,
> > Volker
> >
> > > -phil.
> > >
> > > On 11/15/18, 8:55 AM, Philip Race wrote:
> > > > Hmm. I don't like the ifdefs.
> > > >
> > > > Xrandr is a requirement for the build. If its not there at runtime
> > > > that's OK.
> > > >
> > > > -phil.
> > > >
> > > > On 11/15/18, 8:06 AM, Volker Simonis wrote:
> > > >> Hi,
> > > >>
> > > >> can I please have a review for the following small change:
> > > >>
> > > >> http://cr.openjdk.java.net/~simonis/webrevs/2018/8213944/
> > > >> https://bugs.openjdk.java.net/browse/JDK-8213944
> > > >>
> > > >> Change JDK-8210863 removed the Xrandr.h/randr.h headers from the
> > > >> OpenJDK sources but forgot to add a configure check for the Xrandr
> > > >> extension which is now a build dependency.
> > > >>
> > > >> The change also broke the AIX build. AIX never supported Xrandr, but
> > > >> that was only detected at runtime, when the JDK was unable to
> > > >> dynamically load libXrand.so. Now, without Xrandr.h/randr.h in the
> > > >> source tree any more, we have to conditionally compile some parts of
> > > >> src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c such
> > > >> that it doesn't require the definitions and declarations from
> > > >> Xrandr.h/randr.h any more.
> > > >>
> > > >> Thank you and best regards,
> > > >> Volker

Re: RFR: 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported - was : RE: bin/idea.sh and Cygwin

[Thomas Stüfe]

pushed.
On Wed, Nov 14, 2018 at 9:11 AM Baesken, Matthias
 wrote:
>
> HI Michal ,  tested on my Windows 10 machine ,  works nicely !
>
> Best regards, Matthias
>
>
> > -Original Message-
> > From: Michal Vala 
> > Sent: Montag, 12. November 2018 05:25
> > To: Thomas Stüfe 
> > Cc: Erik Joelsson ; Baesken, Matthias
> > ; Chris Hegarty ;
> > build-dev ; Maurizio Cimadamore
> > 
> > Subject: Re: RFR: 8213591 : running bin/idea.sh in Cygwin: generated project
> > cannot be imported - was : RE: bin/idea.sh and Cygwin
> >
> > Hi Thomas,
> >
> > thanks! I've tested on Windows 2012, vs2013.
> >
> > Anyone with latest Windows 10 to test this?
> >
> > Also I'd like to ask someone to sponsor this, as I'm just an author.
> >
> >
>

Re: RFR: 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported - was : RE: bin/idea.sh and Cygwin

[Thomas Stüfe]

Hi Michal,

I can sponsor for you.

@Matthias: yould you test on your Box too if this patch works?

Best Regards, Thomas

On Mon, Nov 12, 2018 at 5:24 AM Michal Vala  wrote:
>
> Hi Thomas,
>
> thanks! I've tested on Windows 2012, vs2013.
>
> Anyone with latest Windows 10 to test this?
>
> Also I'd like to ask someone to sponsor this, as I'm just an author.
>
>
> On 11/9/18 7:09 PM, Thomas Stüfe wrote:
> > Hi Michal,
> >
> > I tested this and it now works nicely for me (win7, vs2017, with
> > current jdk/jdk).
> >
> > Change looks fine to me to.
> >
> > Best Regards, Thomas
> >
> > On Fri, Nov 9, 2018 at 7:23 PM Michal Vala  wrote:
> >>
> >> I got valid idea project even with empty JT_HOME as placeholder was 
> >> correctly
> >> replaced by empty string. Sure that it's not acceptable.
> >>
> >> Anyway, JT_HOME should be only variable that can be empty.
> >>
> >> new webrev: 
> >> http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.02/
> >>
> >> On 11/9/18 5:42 PM, Thomas Stüfe wrote:
> >>> On Fri, Nov 9, 2018 at 5:35 PM Thomas Stüfe  
> >>> wrote:
> >>>>
> >>>> Hi Michal,
> >>>>
> >>>> does not yet work for me. I get cygpath Usage output:
> >>>>
> >>>> $ bash  ./bin/idea.sh
> >>>> Usage: cygpath (-d|-m|-u|-w|-t TYPE) [-f FILE] [OPTION]... NAME...
> >>>>  cygpath [-c HANDLE]
> >>>>  cygpath [-ADHOPSW]
> >>>>  cygpath [-F ID]
> >>>>
> >>>> Convert Unix and Windows format paths, or output system path information
> >>>> ...
> >>>>
> >>>> Cheers, Thomas
> >>>
> >>> add_replacement "###JTREG_HOME###" "`cygpath -am $JT_HOME`"
> >>>
> >>> seems to be the culprit.
> >>>
> >>> JT_HOME is empty, and I never did set that before (I usually work on
> >>> Linux though).
> >>>
> >>> I think the problem is that in this expression:
> >>>
> >>>  if [ "x$CYGPATH" = "x" ]; then
> >>>  ..
> >>>  else
> >>>  ..
> >>>  fi
> >>>
> >>> the non-windows path does not require the variables to be set. Whereas
> >>> calling "cygpath -am" without an argument is an error which leads to
> >>> the usage output.
> >>>
> >>> ..Thomas
> >>>
> >>>>
> >>>> On Fri, Nov 9, 2018 at 6:09 PM Michal Vala  wrote:
> >>>>>
> >>>>> You're right, sorry. Updated webrev:
> >>>>> http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.01/
> >>>>>
> >>>>> On 11/9/18 5:42 PM, Erik Joelsson wrote:
> >>>>>> Hello Michal,
> >>>>>>
> >>>>>> It looks like the "dirname" calls are omitted in the cygpath case, so 
> >>>>>> BUILD_DIR
> >>>>>> ends up pointing to the spec file instead of the directory the file is 
> >>>>>> in.
> >>>>>>
> >>>>>> /Erik
> >>>>>>
> >>>>>>
> >>>>>> On 2018-11-09 05:58, Michal Vala wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> I've looked into this. Please review the patch:
> >>>>>>> http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.00/
> >>>>>>>
> >>>>>>> On 11/9/18 9:29 AM, Baesken, Matthias wrote:
> >>>>>>>> Hello , I opened
> >>>>>>>>
> >>>>>>>> 8213591 :   running bin/idea.sh in Cygwin: generated project cannot 
> >>>>>>>> be imported
> >>>>>>>>
> >>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8213591
> >>>>>>>>
> >>>>>>>> for  the reported issue .
> >>>>>>>>
> >>>>>>>> Best regards, Matthias
> >>>>>>>>
> >>>>>>>>> -Original Message-
> >>>>>>>>> From: Erik Joelsson 
> >>>>>>>>> Sent: Donnerstag, 8. November 2018 18

Re: RFR: 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported - was : RE: bin/idea.sh and Cygwin

[Thomas Stüfe]

Hi Michal,

I tested this and it now works nicely for me (win7, vs2017, with
current jdk/jdk).

Change looks fine to me to.

Best Regards, Thomas

On Fri, Nov 9, 2018 at 7:23 PM Michal Vala  wrote:
>
> I got valid idea project even with empty JT_HOME as placeholder was correctly
> replaced by empty string. Sure that it's not acceptable.
>
> Anyway, JT_HOME should be only variable that can be empty.
>
> new webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.02/
>
> On 11/9/18 5:42 PM, Thomas Stüfe wrote:
> > On Fri, Nov 9, 2018 at 5:35 PM Thomas Stüfe  wrote:
> >>
> >> Hi Michal,
> >>
> >> does not yet work for me. I get cygpath Usage output:
> >>
> >> $ bash  ./bin/idea.sh
> >> Usage: cygpath (-d|-m|-u|-w|-t TYPE) [-f FILE] [OPTION]... NAME...
> >> cygpath [-c HANDLE]
> >> cygpath [-ADHOPSW]
> >> cygpath [-F ID]
> >>
> >> Convert Unix and Windows format paths, or output system path information
> >> ...
> >>
> >> Cheers, Thomas
> >
> >add_replacement "###JTREG_HOME###" "`cygpath -am $JT_HOME`"
> >
> > seems to be the culprit.
> >
> > JT_HOME is empty, and I never did set that before (I usually work on
> > Linux though).
> >
> > I think the problem is that in this expression:
> >
> > if [ "x$CYGPATH" = "x" ]; then
> > ..
> > else
> > ..
> > fi
> >
> > the non-windows path does not require the variables to be set. Whereas
> > calling "cygpath -am" without an argument is an error which leads to
> > the usage output.
> >
> > ..Thomas
> >
> >>
> >> On Fri, Nov 9, 2018 at 6:09 PM Michal Vala  wrote:
> >>>
> >>> You're right, sorry. Updated webrev:
> >>> http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.01/
> >>>
> >>> On 11/9/18 5:42 PM, Erik Joelsson wrote:
> >>>> Hello Michal,
> >>>>
> >>>> It looks like the "dirname" calls are omitted in the cygpath case, so 
> >>>> BUILD_DIR
> >>>> ends up pointing to the spec file instead of the directory the file is 
> >>>> in.
> >>>>
> >>>> /Erik
> >>>>
> >>>>
> >>>> On 2018-11-09 05:58, Michal Vala wrote:
> >>>>> Hi,
> >>>>>
> >>>>> I've looked into this. Please review the patch:
> >>>>> http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.00/
> >>>>>
> >>>>> On 11/9/18 9:29 AM, Baesken, Matthias wrote:
> >>>>>> Hello , I opened
> >>>>>>
> >>>>>> 8213591 :   running bin/idea.sh in Cygwin: generated project cannot be 
> >>>>>> imported
> >>>>>>
> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8213591
> >>>>>>
> >>>>>> for  the reported issue .
> >>>>>>
> >>>>>> Best regards, Matthias
> >>>>>>
> >>>>>>> -Original Message-
> >>>>>>> From: Erik Joelsson 
> >>>>>>> Sent: Donnerstag, 8. November 2018 18:05
> >>>>>>> To: Baesken, Matthias ; Chris Hegarty
> >>>>>>> ; 'build-dev@openjdk.java.net'  >>>>>>> d...@openjdk.java.net>; maurizio.cimadam...@oracle.com
> >>>>>>> Subject: Re: bin/idea.sh and Cygwin
> >>>>>>>
> >>>>>>> A patch fixing idea.sh so that it works on Windows would certainly be
> >>>>>>> welcome.
> >>>>>>>
> >>>>>>> /Erik
> >>>>>>>
> >>>>>>>
> >>>>>>> On 2018-11-08 05:12, Baesken, Matthias wrote:
> >>>>>>>> Hi  Chris ,  thanks for the info .
> >>>>>>>>
> >>>>>>>> However I found out that replacing the   /cygdrive/C/ with C:/ in   
> >>>>>>>> the
> >>>>>>> top-level  xml/imlfiles in the  ".idea"  - folder
> >>>>>>>> makes   IntelliJ   happy,  I could then  open  the project 
> >>>>>>>> successfully
> >>>>>>>> from
> >>>>>

Re: RFR: 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported - was : RE: bin/idea.sh and Cygwin

[Thomas Stüfe]

On Fri, Nov 9, 2018 at 5:35 PM Thomas Stüfe  wrote:
>
> Hi Michal,
>
> does not yet work for me. I get cygpath Usage output:
>
> $ bash  ./bin/idea.sh
> Usage: cygpath (-d|-m|-u|-w|-t TYPE) [-f FILE] [OPTION]... NAME...
>cygpath [-c HANDLE]
>cygpath [-ADHOPSW]
>cygpath [-F ID]
>
> Convert Unix and Windows format paths, or output system path information
> ...
>
> Cheers, Thomas

  add_replacement "###JTREG_HOME###" "`cygpath -am $JT_HOME`"

seems to be the culprit.

JT_HOME is empty, and I never did set that before (I usually work on
Linux though).

I think the problem is that in this expression:

   if [ "x$CYGPATH" = "x" ]; then
   ..
   else
   ..
   fi

the non-windows path does not require the variables to be set. Whereas
calling "cygpath -am" without an argument is an error which leads to
the usage output.

..Thomas

>
> On Fri, Nov 9, 2018 at 6:09 PM Michal Vala  wrote:
> >
> > You're right, sorry. Updated webrev:
> > http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.01/
> >
> > On 11/9/18 5:42 PM, Erik Joelsson wrote:
> > > Hello Michal,
> > >
> > > It looks like the "dirname" calls are omitted in the cygpath case, so 
> > > BUILD_DIR
> > > ends up pointing to the spec file instead of the directory the file is in.
> > >
> > > /Erik
> > >
> > >
> > > On 2018-11-09 05:58, Michal Vala wrote:
> > >> Hi,
> > >>
> > >> I've looked into this. Please review the patch:
> > >> http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.00/
> > >>
> > >> On 11/9/18 9:29 AM, Baesken, Matthias wrote:
> > >>> Hello , I opened
> > >>>
> > >>> 8213591 :   running bin/idea.sh in Cygwin: generated project cannot be 
> > >>> imported
> > >>>
> > >>> https://bugs.openjdk.java.net/browse/JDK-8213591
> > >>>
> > >>> for  the reported issue .
> > >>>
> > >>> Best regards, Matthias
> > >>>
> > >>>> -Original Message-
> > >>>> From: Erik Joelsson 
> > >>>> Sent: Donnerstag, 8. November 2018 18:05
> > >>>> To: Baesken, Matthias ; Chris Hegarty
> > >>>> ; 'build-dev@openjdk.java.net'  > >>>> d...@openjdk.java.net>; maurizio.cimadam...@oracle.com
> > >>>> Subject: Re: bin/idea.sh and Cygwin
> > >>>>
> > >>>> A patch fixing idea.sh so that it works on Windows would certainly be
> > >>>> welcome.
> > >>>>
> > >>>> /Erik
> > >>>>
> > >>>>
> > >>>> On 2018-11-08 05:12, Baesken, Matthias wrote:
> > >>>>> Hi  Chris ,  thanks for the info .
> > >>>>>
> > >>>>> However I found out that replacing the   /cygdrive/C/ with C:/ in   
> > >>>>> the
> > >>>> top-level  xml/imlfiles in the  ".idea"  - folder
> > >>>>>makes   IntelliJ   happy,  I could then  open  the project 
> > >>>>> successfully
> > >>>>> from
> > >>>> IntelliJ  .
> > >>>>>
> > >>>>> So I guess a  couple  of"cygpath  -aw"  -calls  at the right 
> > >>>>> places  in
> > >>>>> the
> > >>>> project generation   might   fix  the idea.sh   based project file
> > >>>> generation on
> > >>>> Cygwin   (without postprocessing).
> > >>>>> Any comments on this ?
> > >>>>>
> > >>>>> Or is there another  way  to get   .idea/-files  that open "out of the
> > >>>>> box"   ?
> > >>>>>
> > >>>>>
> > >>>>> Best regards, Matthias
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>> -Original Message-
> > >>>>>> From: Chris Hegarty 
> > >>>>>> Sent: Donnerstag, 8. November 2018 12:52
> > >>>>>> To: Baesken, Matthias ; 'build-
> > >>>>>> d...@openjdk.java.net' ;
> > >>>>>> maurizio.cimadam...@oracle.com
> > >>>>>> Subject: Re: bin/idea.sh and Cygwin
> > >>>>>>
> > >>>>>> Matthias,
> > >>>>>>
> > >>>>>> On 08/11/18 11:45, Baesken, Matthias wrote:
> > >>>>>>> Hello, I tried to use bin/idea.sh  with Cygwin to generate project 
> > >>>>>>> files
> > >>>> for
> > >>>>>> IDEA  IntelliJ Community .
> > >>>>>>> The  project file generation seems to work and outputs the   .idea -
> > >>>> folder
> > >>>>>> with lots of xml files in it .
> > >>>>>>> However  ,  when opening the project from IDEA,  it fails  with  a
> > >>>> message :
> > >>>>>>>
> > >>>>>>> VCS root  configuration  problems  -
> > >>>>>>>
> > >>>>>>> The directory  \cygdrive\C\hg\open\jdk\jdk6 is registered as a 
> > >>>>>>> hg4idea
> > >>>> root
> > >>>>>> but no hg4idea  repositories were found  there .
> > >>>>>>> C.\hg\open\jdk\jdk6
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> Could it be that  the Cygwin-paths  in the generated xml-files 
> > >>>>>>> confuse
> > >>>> the
> > >>>>>> IDEA intelliJ IDE ?
> > >>>>>>
> > >>>>>> Certainly looks like it.
> > >>>>>>
> > >>>>>>> Has anybody ever used it successfully  with Cygwin/ Windows  ?
> > >>>>>>> ( or with some other UNIX shell/toolset for Windows) ?
> > >>>>>> I have not tried. I use it successfully on macOS and Linux.
> > >>>>>>
> > >>>>>> -Chris.
> > >>>
> > >>
> > >
> >
> > --
> > Michal Vala
> > OpenJDK QE
> > Red Hat Czech

Re: RFR: 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported - was : RE: bin/idea.sh and Cygwin

[Thomas Stüfe]

Hi Michal,

does not yet work for me. I get cygpath Usage output:

$ bash  ./bin/idea.sh
Usage: cygpath (-d|-m|-u|-w|-t TYPE) [-f FILE] [OPTION]... NAME...
   cygpath [-c HANDLE]
   cygpath [-ADHOPSW]
   cygpath [-F ID]

Convert Unix and Windows format paths, or output system path information
...

Cheers, Thomas

On Fri, Nov 9, 2018 at 6:09 PM Michal Vala  wrote:
>
> You're right, sorry. Updated webrev:
> http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.01/
>
> On 11/9/18 5:42 PM, Erik Joelsson wrote:
> > Hello Michal,
> >
> > It looks like the "dirname" calls are omitted in the cygpath case, so 
> > BUILD_DIR
> > ends up pointing to the spec file instead of the directory the file is in.
> >
> > /Erik
> >
> >
> > On 2018-11-09 05:58, Michal Vala wrote:
> >> Hi,
> >>
> >> I've looked into this. Please review the patch:
> >> http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.00/
> >>
> >> On 11/9/18 9:29 AM, Baesken, Matthias wrote:
> >>> Hello , I opened
> >>>
> >>> 8213591 :   running bin/idea.sh in Cygwin: generated project cannot be 
> >>> imported
> >>>
> >>> https://bugs.openjdk.java.net/browse/JDK-8213591
> >>>
> >>> for  the reported issue .
> >>>
> >>> Best regards, Matthias
> >>>
>  -Original Message-
>  From: Erik Joelsson 
>  Sent: Donnerstag, 8. November 2018 18:05
>  To: Baesken, Matthias ; Chris Hegarty
>  ; 'build-dev@openjdk.java.net'   d...@openjdk.java.net>; maurizio.cimadam...@oracle.com
>  Subject: Re: bin/idea.sh and Cygwin
> 
>  A patch fixing idea.sh so that it works on Windows would certainly be
>  welcome.
> 
>  /Erik
> 
> 
>  On 2018-11-08 05:12, Baesken, Matthias wrote:
> > Hi  Chris ,  thanks for the info .
> >
> > However I found out that replacing the   /cygdrive/C/ with C:/ in   the
>  top-level  xml/imlfiles in the  ".idea"  - folder
> >makes   IntelliJ   happy,  I could then  open  the project 
> > successfully
> > from
>  IntelliJ  .
> >
> > So I guess a  couple  of"cygpath  -aw"  -calls  at the right places 
> >  in
> > the
>  project generation   might   fix  the idea.sh   based project file
>  generation on
>  Cygwin   (without postprocessing).
> > Any comments on this ?
> >
> > Or is there another  way  to get   .idea/-files  that open "out of the
> > box"   ?
> >
> >
> > Best regards, Matthias
> >
> >
> >
> >
> >> -Original Message-
> >> From: Chris Hegarty 
> >> Sent: Donnerstag, 8. November 2018 12:52
> >> To: Baesken, Matthias ; 'build-
> >> d...@openjdk.java.net' ;
> >> maurizio.cimadam...@oracle.com
> >> Subject: Re: bin/idea.sh and Cygwin
> >>
> >> Matthias,
> >>
> >> On 08/11/18 11:45, Baesken, Matthias wrote:
> >>> Hello, I tried to use bin/idea.sh  with Cygwin to generate project 
> >>> files
>  for
> >> IDEA  IntelliJ Community .
> >>> The  project file generation seems to work and outputs the   .idea -
>  folder
> >> with lots of xml files in it .
> >>> However  ,  when opening the project from IDEA,  it fails  with  a
>  message :
> >>>
> >>> VCS root  configuration  problems  -
> >>>
> >>> The directory  \cygdrive\C\hg\open\jdk\jdk6 is registered as a hg4idea
>  root
> >> but no hg4idea  repositories were found  there .
> >>> C.\hg\open\jdk\jdk6
> >>>
> >>>
> >>> Could it be that  the Cygwin-paths  in the generated xml-files confuse
>  the
> >> IDEA intelliJ IDE ?
> >>
> >> Certainly looks like it.
> >>
> >>> Has anybody ever used it successfully  with Cygwin/ Windows  ?
> >>> ( or with some other UNIX shell/toolset for Windows) ?
> >> I have not tried. I use it successfully on macOS and Linux.
> >>
> >> -Chris.
> >>>
> >>
> >
>
> --
> Michal Vala
> OpenJDK QE
> Red Hat Czech

Re: windows cygwin version test too strict

[Thomas Stüfe]

Please ignore all my mails. My repository was just off. I did a fresh
re-clone, now all is fine.

Thanks, Thomas
On Fri, Nov 9, 2018 at 4:13 PM Thomas Stüfe  wrote:
>
> ..bypassing this version check yields a similar error later on, where
> a more modern GNUmake is not accepted:
>
> configure: Found GNU make at /usr/bin/make, however this is not
> version 3.81 or later. (it is: GNU Make 4.2.1). Ignoring.
> configure: error: Cannot find GNU make 3.81 or newer! Please put it in
> the path, or add e.g. MAKE=/opt/gmake3.81/make as argument to
> configure.
>
> ..Thomas
> On Fri, Nov 9, 2018 at 4:09 PM Thomas Stüfe  wrote:
> >
> > Hi all,
> >
> > When running configure on Windows, I get now:
> >
> > configure: Your cygwin is too old. You are running 2.11.2(0.329/5/3),
> > but at least cygwin 1.7 is required. Please upgrade.
> >
> > When looking at basics_windows.m4:
> >
> > CYGWIN_VERSION_OK=`$ECHO $CYGWIN_VERSION | $GREP ^1.7.`
> > if test "x$CYGWIN_VERSION_OK" = x; then
> >   AC_MSG_NOTICE([Your cygwin is too old. You are running
> > $CYGWIN_VERSION, but at least cygwin 1.7 is required. Please
> > upgrade.])
> >   AC_MSG_ERROR([Cannot continue])
> > fi
> >
> > So, we explicitly need version 1.7? Higher won't do?
> >
> > Thanks, Thomas

Re: windows cygwin version test too strict

[Thomas Stüfe]

..bypassing this version check yields a similar error later on, where
a more modern GNUmake is not accepted:

configure: Found GNU make at /usr/bin/make, however this is not
version 3.81 or later. (it is: GNU Make 4.2.1). Ignoring.
configure: error: Cannot find GNU make 3.81 or newer! Please put it in
the path, or add e.g. MAKE=/opt/gmake3.81/make as argument to
configure.

..Thomas
On Fri, Nov 9, 2018 at 4:09 PM Thomas Stüfe  wrote:
>
> Hi all,
>
> When running configure on Windows, I get now:
>
> configure: Your cygwin is too old. You are running 2.11.2(0.329/5/3),
> but at least cygwin 1.7 is required. Please upgrade.
>
> When looking at basics_windows.m4:
>
> CYGWIN_VERSION_OK=`$ECHO $CYGWIN_VERSION | $GREP ^1.7.`
> if test "x$CYGWIN_VERSION_OK" = x; then
>   AC_MSG_NOTICE([Your cygwin is too old. You are running
> $CYGWIN_VERSION, but at least cygwin 1.7 is required. Please
> upgrade.])
>   AC_MSG_ERROR([Cannot continue])
> fi
>
> So, we explicitly need version 1.7? Higher won't do?
>
> Thanks, Thomas

windows cygwin version test too strict

[Thomas Stüfe]

Hi all,

When running configure on Windows, I get now:

configure: Your cygwin is too old. You are running 2.11.2(0.329/5/3),
but at least cygwin 1.7 is required. Please upgrade.

When looking at basics_windows.m4:

CYGWIN_VERSION_OK=`$ECHO $CYGWIN_VERSION | $GREP ^1.7.`
if test "x$CYGWIN_VERSION_OK" = x; then
  AC_MSG_NOTICE([Your cygwin is too old. You are running
$CYGWIN_VERSION, but at least cygwin 1.7 is required. Please
upgrade.])
  AC_MSG_ERROR([Cannot continue])
fi

So, we explicitly need version 1.7? Higher won't do?

Thanks, Thomas

Re: bash completion for make targets

[Thomas Stüfe]

On Thu, Nov 8, 2018 at 12:13 PM Magnus Ihse Bursie
 wrote:
>
> On 2018-11-08 12:10, Thomas Stüfe wrote:
>
> > Hi all,
> >
> > This may be a stupid question, and not that important, but bash
> > completion for targets like "clean" and "images" stopped working for
> > me. "reconfigure" still works thouugh. Did anything change?
> >
> > Best Regards, Thomas
> Not intentionally. It can be that there were too many targets. Does the
> recent fix for JDK-8213338 help?
>

Unfortunately no.

..Thomas

> /Magnus

bash completion for make targets

[Thomas Stüfe]

Hi all,

This may be a stupid question, and not that important, but bash
completion for targets like "clean" and "images" stopped working for
me. "reconfigure" still works thouugh. Did anything change?

Best Regards, Thomas

Re: RFR(XS): 8213515: Improve freetype detection on linux/ppc64/ppc64le/s390x

[Thomas Stüfe]

+1

Gruss Thomas
On Thu, Nov 8, 2018 at 10:25 AM Volker Simonis  wrote:
>
> Hi,
>
> can I please have a review for the following trivial enhancement of
> the freetype detection on linux/ppc64/ppc64le/s390x:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8213515/
> https://bugs.openjdk.java.net/browse/JDK-8213515
>
> On some more "exotic" Linux platforms, libfreetype.so is found under
> /usr/lib64/libfreetype.so even if the platform only supports 64-bits.
> However, we currently only check for libfreetype.so under "/usr/lib"
> and "/usr/lib/-linux-gnu". Another check under "/usr/lib64" does
> no harm and greatly improves the usability on the mentioned platforms.
>
> Thank you and best regards,
> Volker

Re: RFR: JDK-8213428: Add a no preocmpiled header Linux build to builds-tier1 and jdk-submit

[Thomas Stüfe]

Hi Erik,

I cannot comment on the fix, but thanks for doing this!

Best Regards, Thomas

On Tue, Nov 6, 2018 at 7:06 PM Erik Joelsson  wrote:
>
> A recent discussion on build-dev brought out the need for jdk-submit to
> include a build with precompiled headers disabled. We currently have
> such builds in builds-tier2, which is not part of jdk-submit. In tier2
> now, we actually build all slowdebug builds without pch, which is
> overkill on platform coverage and not as useful as fastdebug builds.
>
> My suggestion is then to add a specialized linux build of just hotspot,
> fastdebug and disabled pch to builds-tier1 and remove the disabling of
> pch on the slowdebug builds in tier2.
>
> This open part adds the specialized build config to the jib configuration.
>
> Webrev: http://cr.openjdk.java.net/~erikj/8213428/webrev.01/
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8213428
>
> /Erik
>

Re: RFR: JDK-8213339 Update precompiled.hpp with headers based on current frequency

[Thomas Stüfe]

Hi David,

On Sun, Nov 4, 2018 at 11:09 PM David Holmes  wrote:
>
> On 4/11/2018 5:27 PM, Magnus Ihse Bursie wrote:
> >
> >> 3 nov. 2018 kl. 23:24 skrev David Holmes :
> >>
> >>> On 3/11/2018 10:09 PM, David Holmes wrote:
> >>> Looks okay - thanks for doing all the experiments! (Though I'm still 
> >>> curious what happens if you recompile individual header files :) ).
> >>
> >> s/recompile/precompile/  :)
> >
> > What do you mean? To have more than one set of PCH? As far as I know, all 
> > compilers we use only support a single PCH per source file to compile. So 
> > while you could have a PCH tailored per source file, that would only make 
> > sense in a scenario were you recompile all source files (but no header 
> > files) often.
>
> The GCC description for PCH describes it as a per-file header file
> optimization. You can precompile each .hpp file and if it is located in
> the same directory as the .hpp (or is on an include directory ahead of
> the .hpp) then the PCH file will be used instead. So rather than
> precompiling precompiled.hpp and having source files include it, you
> would precompile each individual (chosen) header file and have that in
> the special PCH include directory, and they would then be included where
> a source file asked for the corresponding .hpp file.
>

I assume precompiling a header means precompiling it itself and any
header it includes, no? In that case, would precompiling each header
individually not cause us to implicitly precompile included headers
many times (e.g. globalDefinitions.hpp)?

Thanks, Thomas

> That would also remove the problem of things not compiling with PCH
> disabled as a single set of includes would be used for both PCH and
> non-PCH building.
>
> I don't know if the other compilers support PCH in a similar manner to gcc.
>
> Cheers,
> David
>
> > /Magnus
> >
> >>
> >> David
> >>
> >>>   25 // Precompiled headers are turned off for Sun Studio,
> >>> May as well change to Solaris Studio if you're going to fix the typo :)
> >>> Thanks.
> >>> David
>  On 3/11/2018 7:06 PM, Magnus Ihse Bursie wrote:
>  The reasons for the current set of files included in precompiled.hpp is 
>  somewhat lost in the mists of history. However, it is clear that it is 
>  not optimal.
> 
>  This patch replaces the current set with a new set, based on how often a 
>  header file is included in a C++ file. This selection contains all 
>  header files that are included by at least 130 C++ files. Testing has 
>  shown that this is around the optimal value -- include many more, and 
>  too many "innocent" files get hurt by unneeded work, leave out many 
>  more, and we miss out on optimization possibilities.
> 
>  The same set turned out to work well for both clang and gcc. However, 
>  files named "*.inline.hpp" did hurt rather than help performance, so 
>  those have been left out. For visual studio, the same set was also 
>  optimal, as long as the inline files were included. Presumably, visual 
>  studio is better than gcc/clang on handling precompiled headers 
>  containing inlined code.
> 
>  Here are some rough comparisons from our internal CI system, for 
>  building the target "hotspot" from scratch.
> 
>  macosx-x64:
>  old: 00:05:00
>  new: 00:03:47
> 
>  linux-x64:
>  old: 00:05:43
>  new: 00:04:51
> 
>  windows-x64:
>  old: 00:05:18
>  new: 00:04:33
> 
>  linux-aarch64:
>  old: 00:07:57
>  new: 00:03:48
> 
>  Bug: https://bugs.openjdk.java.net/browse/JDK-8213339
>  WebRev: 
>  http://cr.openjdk.java.net/~ihse/JDK-8213339-update-precompiled-headers/webrev.01
> 
>  /Magnus
> >

Re: Stop using precompiled headers for Linux?

[Thomas Stüfe]

Hi Magnus,

your winning variant gives me a nice boost on my thinkpad:

pch, standard:
real17m52.367s
user52m20.730s
sys 4m53.711s

pch, your variant:
real15m0.514s
user46m6.466s
sys 2m38.371s

(non-pch is ~19-20 minutes WTC)

With those numbers, I might start using pch again on low powered machines.

.. Thomas



On Fri, Nov 2, 2018 at 12:14 PM Magnus Ihse Bursie
 wrote:
>
>
> On 2018-11-02 11:39, Magnus Ihse Bursie wrote:
> > On 2018-11-02 00:53, Ioi Lam wrote:
> >> Maybe precompiled.hpp can be periodically (weekly?) updated by a
> >> robot, which parses the dependencies files generated by gcc, and pick
> >> the most popular N files?
> > I think that's tricky to implement automatically. However, I've done
> > more or less, that, and I've got some wonderful results! :-)
>
> Ok, I'm done running my tests.
>
> TL;DR: I've managed to reduce wall-clock time from 2m 45s (with pch) or
> 2m 23s (without pch), to 1m 55s. The cpu time spent went from 52m 27s
> (with pch) or 55m 30s (without pch) to 41m 10s. This is a huge gain for
> our automated builds! And a clear improvement even for the ordinary
> developer.
>
> The list of included header files is reduced to just 37. The winning
> combination was to include all header files that was included in more
> than 130 different files, but to exclude all files with the name
> "*.inline.hpp". Hopefully, a further gain of not pulling in the
> *.inline.hpp files is that the risk of pch/non-pch failures will diminish.
>
> However, these 37 files in turn pull in an additional 201 header files.
> Of these, three are *.inline.hpp:
> share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp,
> os_cpu/linux_x86/bytes_linux_x86.inline.hpp and
> os_cpu/linux_x86/copy_linux_x86.inline.hpp. This looks like a problem
> with the header files to me.
>
> With some exceptions (mostly related to JFR), these additional 200 files
> have "generic" looking names (like share/gc/g1/g1_globals.hpp), which
> indicate to me that it is reasonable to have them in this list, just as
> the list of the original 37 tended to be quite general and high-level
> includes. However, some files (like
> share/jfr/instrumentation/jfrEventClassTransformer.hpp) has maybe leaked
> in where they should not really be. It might be worth letting a hotspot
> engineer spend some cycles to check up these files and see if anything
> can be improved.
>
> Caveats: I have only run this on my local linux build with the default
> server JVM configuration. Other machines will have different sweet
> spots. Other JVM variants/feature combinations will have different sweet
> spots. And, most importantly, I have not tested this at all on Windows.
> Nevertheless, I'm almost prepared to suggest a patch that uses this
> selection of files if running on gcc, just as is, because of the speed
> improvements I measured.
>
> And some data:
>
> Here is my log from my runs. The "on or above" means the cutoff I used
> for how many files that needed to include the files that were selected.
> As you can see, there is not much difference between cutoffs between
> 130-150, or (without the inline files) between 110 and 150. (There were
> a lot of additional inline files in the positions below 130.) With all
> other equal, I'd prefer a solution with fewer files. That is less likely
> to go bad.
>
> real2m45.623s
> user52m27.813s
> sys5m27.176s
> hotspot with original pch
>
> real2m23.837s
> user55m30.448s
> sys3m39.739s
> hotspot without pch
>
> real1m59.533s
> user42m50.019s
> sys3m0.893s
> hotspot new pch on or above 250
>
> real1m58.937s
> user42m18.994s
> sys3m0.245s
> hotspot new pch on or above 200
>
> real2m0.729s
> user42m16.636s
> sys2m57.125s
> hotspot new pch on or above 170
>
> real1m58.064s
> user42m9.618s
> sys2m57.635s
> hotspot new pch on or above 150
>
> real1m58.053s
> user42m9.796s
> sys2m58.732s
> hotspot new pch on or above 130
>
> real2m3.364s
> user42m54.818s
> sys3m2.737s
> hotspot new pch on or above 100
>
> real2m6.698s
> user44m30.434s
> sys3m12.015s
> hotspot new pch on or above 70
>
> real2m0.598s
> user41m17.810s
> sys2m56.258s
> hotspot new pch on or above 150 without inline
>
> real1m55.981s
> user41m10.076s
> sys2m51.983s
> hotspot new pch on or above 130 without inline
>
> real1m56.449s
> user41m10.667s
> sys2m53.808s
> hotspot new pch on or above 110 without inline
>
> And here is the "winning" list (which I declared as "on or above 130,
> without inline"). I encourage everyone to try this on their own system,
> and report back the results!
>
> #ifndef DONT_USE_PRECOMPILED_HEADER
> # include "classfile/classLoaderData.hpp"
> # include "classfile/javaClasses.hpp"
> # include "classfile/systemDictionary.hpp"
> # include "gc/shared/collectedHeap.hpp"
> # include "gc/shared/gcCause.hpp"
> # include "logging/log.hpp"
> # include "memory/allo

Re: Stop using precompiled headers for Linux?

[Thomas Stüfe]

On Thu, Nov 1, 2018 at 12:05 PM Magnus Ihse Bursie
 wrote:
>
> On 2018-11-01 11:54, Aleksey Shipilev wrote:
> > On 11/01/2018 11:43 AM, Magnus Ihse Bursie wrote:
> >> But then again, it might just signal that the list of headers included in 
> >> the PCH is no longer
> >> optimal. If it used to be the case that ~100 header files were so 
> >> interlinked, that changing any of
> >> them caused recompilation of all files that included it and all the other 
> >> 100 header files on the
> >> PCH list, then there was a net gain for using PCH and no "punishment".
> >>
> >> But nowadays this list might be far too large. Perhaps there's just only a 
> >> core set of say 20 header
> >> files that are universally (or almost universally) included, and that's 
> >> all that should be in the
> >> PCH list then. My guess is that, with a proper selection of header files, 
> >> PCH will still be a benefit.
> > I agree. This smells like inefficient PCH list. We can improve that, but I 
> > think that would be a
> > lower priority, given the abundance of CPU power we use to compile Hotspot. 
> > In my mind, the decisive
> > factor for disabling PCH is to keep proper includes at all times, without 
> > masking it with PCH. Half
> > of the trivial bugs I submit against hotspot are #include differences that 
> > show up in CI that builds
> > without PCH.
> >
> > So this is my ideal world:
> >   a) Efficient PCH list enabled by default for development pleasure;
> >   b) CIs build without PCH all the time (jdk-submit tier1 included!);
> >
> > Since we don't yet have (a), and (b) seems to be tedious, regardless how 
> > many times both Red Hat and
> > SAP people ask for it, disabling PCH by default feels like a good fallback.
>
> Should just CI builds default to non-PCH, or all builds (that is, should
> "configure" default to non-PCH on linux)? Maybe the former is better --
> one thing that the test numbers here has not shown is if incremental
> recompiles are improved by PCH. My gut feeling is that they really
> should -- once you've created your PCH, subsequent recompiles will be
> faster.

That would only be true as long as you just change cpp files, no? As
soon as you touch a header which is included in precompiled.hpp you
are worse off than without pch.

> So the developer default should perhaps be to keep PCH, and we
> should only configure the CI builds to do without PCH.

CI without pch would be better than nothing. But seeing how clunky and
slow jdk-submit is (and how often there are problems), I rather fail
early in my own build than waiting for jdk-submit to tell me something
went wrong (well, that is why I usually build nonpch, like Ioi does).

Just my 5 cent.

..Thomas
>
> /Magnus
>
>
> >
> > -Aleksey
> >
>

Re: Stop using precompiled headers for Linux?

[Thomas Stüfe]

It would help already if Oracle would disable precompiled headers for
the submit test builds.

..Thomas
On Tue, Oct 30, 2018 at 6:26 PM Ioi Lam  wrote:
>
> Is there any advantage of using precompiled headers on Linux? It's on by
> default and we keep having breakage where someone would forget to add
> #include. The latest instance is JDK-8213148.
>
> I just turn on precompiled headers explicitly in all my builds. I don't
> see any difference in build time (at least not significant enough for me
> to bother).
>
> Should we disable it by default on Linux?
>
> Thanks
>
> - Ioi
>
>

Re: RFC: JEP JDK-8208089: Implement C++14 Language Features

[Thomas Stüfe]

Hi Kim,
On Mon, Oct 8, 2018 at 8:32 PM Kim Barrett  wrote:
>
> > On Oct 8, 2018, at 6:48 AM, Thomas Stüfe  wrote:
> >
> > Hi Kim,
> >
> > is this JEP only about C++14 features or shall we discuss older
> > features too? The reason I am asking is that I would like us to
> > officially endorse namespaces. Not inline namespaces, just plain old
> > namespaces.
>
> I would like to keep this JEP focused on C++11/14 usage.
>
> However, it presently describes a process for recording and updating
> HotSpot usage.  I think that process is pretty close to the de facto
> process, which is rarely used successfully because it's not written
> down anywhere.
>
> We could extract that part of the JEP out, formalize, discuss, agree,
> and record it.  Then you could propose a change and there would be a
> process for dealing with the proposal, rather than having it slide
> into oblivion because we don't know how to proceed.  And the JEP could
> be made a bit smaller because it could just refer to that process.
>

Yes, I would like that. It would be really good to have a process in
place to preserve and actually live a consensus about what features we
use.

We have the Hotspot Style Guide, but we let it become woefully
outdated in the face of new developments. I always smile when I read
the "Be sparing with templates." remark - that ship has sailed long
ago.

---

A small remark to the text of your JEP:

"As a rule of thumb, permitting features which simplify writing, and
especially reading, code should be encouraged."

I would like to add to that that simplifying build error analysis
should also be a goal. That directly influences programmer
productivity. We had some cases in the recent past where we spent a
lot of time scratching our heads over walls of template related
compiler errors.

(not sure how to reach this goal though, since this seems to be a
problem inherent in using C++ templates).

Best Regards, Thomas

Re: RFC: JEP JDK-8208089: Implement C++14 Language Features

[Thomas Stüfe]

Hi Kim,

is this JEP only about C++14 features or shall we discuss older
features too? The reason I am asking is that I would like us to
officially endorse namespaces. Not inline namespaces, just plain old
namespaces.

HotSpot makes very limited use of namespaces.

Not really true, we already use them. E.g. in metaspace coding, I used
them to keep the global name space clean and to keep internals
internal. This was met with positive reviews, and it works on all
toolchains, so compiler support should not be a problem. Using
namespaces, we could get slowly replace the "AllStatic" classes, which
are namespaces in all but name. In contrast to classes, namespaces can
be spread over multiple files and compilation units, and allow for
cleaner separation of internal and external coding.

It also would allow us to get rid the middle-of-header-platform-inclusions:

For example, today we have:

[os.hpp]
class os: AllStatic {

(platform independent, outward facing os:: functions)
#include "os_linux.hpp"
>> (Inner class "Linux" with platform specific os functions)
...
}

Not only is the inclusion in the middle of a class terrifying, it also
means the shared, outward facing os:: namespace contains class Linux
and lots of platform specific internals.

With namespaces one could:

[os.hpp]
namespace os {

(platform independent, outward facing os:: functions)

}

[os_linux.hpp]
namespace os {
namespace Linux {
(linux specific os functions)
}
}

I think this is way cleaner, and keeps platform specifics from
including files which only care for the shared os interface.

--

Note that I would prefer forbidding the "using" directive for callers
of namespace functions, but rather force them to spell out the
namespace:

So, instead of this:

using os;
jlong m = available_memory();

I would prefer this, which is our current practice with AllStatic childs:

jlong m = os::available_memory();

The latter form would keep the code grepable.

Best Regards, Thomas
On Wed, Oct 3, 2018 at 9:13 PM Kim Barrett  wrote:
>
> I've submitted a JEP for
>
> (1) enabling the use of C++14 Language Features when building the JDK,
>
> (2) define a process for deciding and documenting which new features
> can be used or are forbidden in HotSpot code,
>
> (3) provide an initial list of permitted and forbidden new features.
>
> https://bugs.openjdk.java.net/browse/JDK-8208089
>

Re: RFR : 8211146 : fix problematic elif-tests after recent gcc warning changes Werror=undef

[Thomas Stüfe]

Thanks for clarifying!

Thomas

On Wed, Sep 26, 2018, 14:20 Baesken, Matthias 
wrote:

> Hi Thomas, I think your understanding is correct :
>
> "#ifdef "
>
> or
>
> "#if  defined()"
>
> Are ok while
>
> "#if "
>
>  or
>
> "#elif "
>
> Lead to the compile error on gcc with current warnings.
>
> Best regards, Matthias
>
>
> > >>
> > > This looks okay to me although I could imagine this issue coming back
> again
> > > with ongoing edits that add "ifdef __solaris" or similar.
> > >
> >
> > If I understand this problem correctly (and I might not) the problem
> > is with "#if ", not with "#ifdef  > switch>".
> >
> > ..thomas
> >
> > > -Alan
>

Re: RFR : 8211146 : fix problematic elif-tests after recent gcc warning changes Werror=undef

[Thomas Stüfe]

On Wed, Sep 26, 2018 at 11:51 AM, Alan Bateman  wrote:
>
>
> On 26/09/2018 10:24, Baesken, Matthias wrote:
>>
>>
>> Hello, please review this small build fix .
>>
>> After the recent changes  to  the gcc compile flags   with  8211029,
>> most of our  Linux builds stopped working .
>>
>> Error :
>>
>> === Output from failing command(s) repeated here ===
>>
>> * For target support_native_java.base_libnet_DatagramPacket.o:
>>
>> In file included from
>> /OpenJDK/8210319/jdk/src/java.base/share/native/libnet/net_util.h:31:0,
>>
>> from
>> /OpenJDK/8210319/jdk/src/java.base/share/native/libnet/DatagramPacket.c:27:
>>
>> /OpenJDK/8210319/jdk/src/java.base/unix/native/libnet/net_util_md.h:50:7:
>> error: "__solaris__" is not defined [-Werror=undef]
>>
>> #elif __solaris__
>>
>>^
>>
>> After looking into it, it seems  that  the
>> jdk/src/java.base/unix/native/libnet/net_util_md.h compile error is only
>> triggered on older Linux OS  (e.g. SLES11).
>>
>> Probably that’s why it was not seen at Oracle  after  puhsing  after
>> 8211029   .
>>
>> Some greps  showed me a number of similar problematic  #elif-checks for
>> OS, I adjusted them too .
>>
>> Bug / webrev :
>>
>> https://bugs.openjdk.java.net/browse/JDK-8211146
>>
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8211146.0/
>> 
>>
>>
> This looks okay to me although I could imagine this issue coming back again
> with ongoing edits that add "ifdef __solaris" or similar.
>

If I understand this problem correctly (and I might not) the problem
is with "#if ", not with "#ifdef ".

..thomas

> -Alan

Re: RFR: 8210647: libsaproc is being compiled without optimization.

[Thomas Stüfe]

A fix is waiting for review: https://bugs.openjdk.java.net/browse/JDK-8210836

Best Regards, Thomas

On Tue, Sep 18, 2018 at 5:09 PM, Anthony Scarpino
 wrote:
> I encountered this too when I build with ‘make’; however if I build through 
> JIB the error didn’t occur.
>
> Tony
>
>> On Sep 18, 2018, at 2:08 AM, Thomas Stüfe  wrote:
>>
>> Hi Severin,
>>
>> I get reproducable build errors with your fix on my machine (Ubuntu 16.4).
>>
>> When I build release (no special build options), I get:
>>
>> /shared/projects/openjdk/jdk-jdk/source/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c:
>> In function ‘read_exec_segments’:
>> /shared/projects/openjdk/jdk-jdk/source/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c:798:7:
>> error: ignoring return value of ‘pread’, declared with attribute
>> warn_unused_result [-Werror=unused-result]
>>   pread(ph->core->exec_fd, interp_name, exec_php->p_filesz,
>> exec_php->p_offset);
>>   ^
>>
>> When I remove your change (reset opt to NONE) error goes away. I have
>> no idea why optimization influences warning level though. Any ideas?
>>
>> (Of course, one could just fix the warning, but I wonder if I am the
>> only one with this problem)
>>
>> Best Regards, Thomas
>>
>>
>>
>> On Mon, Sep 17, 2018 at 6:25 AM, Sharath Ballal
>>  wrote:
>>> Hi Severin,
>>>
>>> Looks good to me.
>>>
>>>
>>> Thanks,
>>> Sharath (not a Reviewer)
>>>
>>>
>>> -Original Message-
>>> From: Severin Gehwolf [mailto:sgehw...@redhat.com]
>>> Sent: Friday, September 14, 2018 7:04 PM
>>> To: build-dev; serviceability-dev
>>> Subject: RFR: 8210647: libsaproc is being compiled without optimization.
>>>
>>> Hi,
>>>
>>> Could I please get a review of this one-liner fix. It changes optimization 
>>> of libsaproc from -O0 to -O3 (as per Magnus' suggestion).
>>> I've run servicability tests and haven't seen any new failures.
>>> Thoughts?
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210647
>>> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210647/webrev.01/
>>>
>>> Thanks,
>>> Severin
>>>
>>>
>

Re: RFR: 8210647: libsaproc is being compiled without optimization.

[Thomas Stüfe]

Hi Severin,

On Tue, Sep 18, 2018 at 11:24 AM, Severin Gehwolf  wrote:
> Hi Thomas,
>
> On Tue, 2018-09-18 at 11:08 +0200, Thomas Stüfe wrote:
>> Hi Severin,
>>
>> I get reproducable build errors with your fix on my machine (Ubuntu 16.4).
>>
>> When I build release (no special build options), I get:
>>
>> /shared/projects/openjdk/jdk-jdk/source/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c:
>> In function ‘read_exec_segments’:
>> /shared/projects/openjdk/jdk-jdk/source/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c:798:7:
>> error: ignoring return value of ‘pread’, declared with attribute
>> warn_unused_result [-Werror=unused-result]
>>pread(ph->core->exec_fd, interp_name, exec_php->p_filesz,
>> exec_php->p_offset);
>>^
>>
>> When I remove your change (reset opt to NONE) error goes away. I have
>> no idea why optimization influences warning level though. Any ideas?
>
> It's rather strange, yes.
>
>> (Of course, one could just fix the warning, but I wonder if I am the
>> only one with this problem)
>
> It's this (you are not the only one):
> https://bugs.openjdk.java.net/browse/JDK-8210836
>
> Would --disable-warnings-as-errors work for you?
>
> We have to have this switched on for our distro builds, as there is
> rarely a clean build unless you manage to have the blessed compiler
> versions available, don't build with system libs, etc.
>
> Sorry about this!
>

No problem. I can wait until this is solved.

Thanks, Thomas

> Cheers,
> Severin
>
>> Best Regards, Thomas
>>
>>
>>
>> On Mon, Sep 17, 2018 at 6:25 AM, Sharath Ballal
>>  wrote:
>> > Hi Severin,
>> >
>> > Looks good to me.
>> >
>> >
>> > Thanks,
>> > Sharath (not a Reviewer)
>> >
>> >
>> > -Original Message-
>> > From: Severin Gehwolf [mailto:sgehw...@redhat.com]
>> > Sent: Friday, September 14, 2018 7:04 PM
>> > To: build-dev; serviceability-dev
>> > Subject: RFR: 8210647: libsaproc is being compiled without optimization.
>> >
>> > Hi,
>> >
>> > Could I please get a review of this one-liner fix. It changes optimization 
>> > of libsaproc from -O0 to -O3 (as per Magnus' suggestion).
>> > I've run servicability tests and haven't seen any new failures.
>> > Thoughts?
>> >
>> > Bug: https://bugs.openjdk.java.net/browse/JDK-8210647
>> > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210647/webrev.01/
>> >
>> > Thanks,
>> > Severin
>> >
>> >
>

Re: RFR: 8210647: libsaproc is being compiled without optimization.

[Thomas Stüfe]

Hi Severin,

I get reproducable build errors with your fix on my machine (Ubuntu 16.4).

When I build release (no special build options), I get:

/shared/projects/openjdk/jdk-jdk/source/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c:
In function ‘read_exec_segments’:
/shared/projects/openjdk/jdk-jdk/source/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c:798:7:
error: ignoring return value of ‘pread’, declared with attribute
warn_unused_result [-Werror=unused-result]
   pread(ph->core->exec_fd, interp_name, exec_php->p_filesz,
exec_php->p_offset);
   ^

When I remove your change (reset opt to NONE) error goes away. I have
no idea why optimization influences warning level though. Any ideas?

(Of course, one could just fix the warning, but I wonder if I am the
only one with this problem)

Best Regards, Thomas



On Mon, Sep 17, 2018 at 6:25 AM, Sharath Ballal
 wrote:
> Hi Severin,
>
> Looks good to me.
>
>
> Thanks,
> Sharath (not a Reviewer)
>
>
> -Original Message-
> From: Severin Gehwolf [mailto:sgehw...@redhat.com]
> Sent: Friday, September 14, 2018 7:04 PM
> To: build-dev; serviceability-dev
> Subject: RFR: 8210647: libsaproc is being compiled without optimization.
>
> Hi,
>
> Could I please get a review of this one-liner fix. It changes optimization of 
> libsaproc from -O0 to -O3 (as per Magnus' suggestion).
> I've run servicability tests and haven't seen any new failures.
> Thoughts?
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8210647
> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210647/webrev.01/
>
> Thanks,
> Severin
>
>

Re: defpath extension won't work with jdk/submit

[Thomas Stüfe]

Thank you Tim!

Should I find time, I may look at it too more closely. In the
meantime, your workaround works fine, thanks.

Have nice vacations!

..Thomas

On Mon, Sep 3, 2018 at 5:32 PM, Tim Bell  wrote:
> On 09/03/18 07:25, Thomas Stüfe wrote:
>>
>> Hi guys,
>>
>> this is still not resolved. I am still unable to do hg defpath -du
>>  on jdk-submit:
>>
>> thomas@mainframe /shared/projects/openjdk/jdk-submit/source $ hg
>> defpath -du stuefe
>> abort: http://hg.openjdk.java.net/jdk/submit: Repository not found
>> No hgrc files updated
>>
>> Is defpath not supported anymore?
>
>
> It is in the code-tools project, so it is supported by the community.
>   http://openjdk.java.net/projects/code-tools/
>
> I got as far as reproducing the problem you reported.  I spent time looking
> at strace and tcpdump packet traces, but did not reach any conclusions.
> What is most puzzling is that two other mercurial repos work fine with
> defpath.py:
>
> Results of creating fresh clones and running 'hg defpath -du tbell':
>
> works: http://hg.openjdk.java.net/jdk/sandbox
> works: http://hg.openjdk.java.net/jdk/submit11
> fails: http://hg.openjdk.java.net/jdk/submit
>
> I am heading out today to catch a flight for vacation.  Until I return on
> the 12'th I am going to have to suggest you take the workaround of editing
> your jdk/submit/.hg/hgrc file by hand.  It pains me to say this, but I don't
> have a better solution today.
>
> For jdk/submit, you want the paths section of your .hg/hgrc file to look
> like this, with your OpenJDK username in place of mine ('tbell'):
>
> [paths]
> default = http://hg.openjdk.java.net/jdk/submit
> default-push = ssh://tb...@hg.openjdk.java.net/jdk/submit
>
>
> Tim
>
>
>
>> Thank you,
>>
>> Best Regards, Thomas
>>
>>
>> On Wed, Aug 29, 2018 at 8:49 AM, Thomas Stüfe 
>> wrote:
>>>
>>> Adding the push path manually to hgrc works fine, so I use that as a
>>> workaround.
>>>
>>> Thanks, Thomas
>>>
>>> On Wed, Aug 29, 2018 at 8:01 AM, Thomas Stüfe 
>>> wrote:
>>>>
>>>> Thank you Tim. Happy to know it is not my setup. Lets see what ops says.
>>>>
>>>> Best, Thomas
>>>>
>>>> On Wed, Aug 29, 2018 at 1:58 AM, Tim Bell  wrote:
>>>>>
>>>>> Thomas:
>>>>>
>>>>>> when trying to run hg defpath to set the paths to the jdk12 submit
>>>>>> repo (http://hg.openjdk.java.net/jdk/submit), it fails:
>>>>>>
>>>>>> thomas@t450:/shared/projects/openjdk/jdk-submit/source$ hg defpath -du
>>>>>> stuefe
>>>>>> abort: http://hg.openjdk.java.net/jdk/submit: Repository not found
>>>>>> No hgrc files updated
>>>>>
>>>>>
>>>>>
>>>>> Adding the ops list  in case others have seen
>>>>> this.
>>>>>
>>>>>> But it works for the submit repo for jdk11
>>>>>> (http://hg.openjdk.java.net/jdk/submit11/), and there it updates the
>>>>>> .hg/hgrc just fine.
>>>>>
>>>>>
>>>>>
>>>>> FWIW, I see the same thing.  I don't have an explanation yet.
>>>>>
>>>>> I created fresh clones and then tried 'hg defpath -du tbell':
>>>>>
>>>>> works: http://hg.openjdk.java.net/jdk/sandbox
>>>>> works: http://hg.openjdk.java.net/jdk/submit11
>>>>> fails: http://hg.openjdk.java.net/jdk/submit
>>>>>
>>>>>> Not sure what the problem is. Both URLs are reachable fine in a
>>>>>> browser.
>>>>>>
>>>>>> Any ideas?
>>>>>
>>>>>
>>>>>
>>>>> I am stumped at the moment, but I will keep digging.
>>>>>
>>>>> Tim
>>>>>
>

Re: defpath extension won't work with jdk/submit

[Thomas Stüfe]

Hi guys,

this is still not resolved. I am still unable to do hg defpath -du
 on jdk-submit:

thomas@mainframe /shared/projects/openjdk/jdk-submit/source $ hg
defpath -du stuefe
abort: http://hg.openjdk.java.net/jdk/submit: Repository not found
No hgrc files updated

Is defpath not supported anymore?

Thank you,

Best Regards, Thomas


On Wed, Aug 29, 2018 at 8:49 AM, Thomas Stüfe  wrote:
> Adding the push path manually to hgrc works fine, so I use that as a 
> workaround.
>
> Thanks, Thomas
>
> On Wed, Aug 29, 2018 at 8:01 AM, Thomas Stüfe  wrote:
>> Thank you Tim. Happy to know it is not my setup. Lets see what ops says.
>>
>> Best, Thomas
>>
>> On Wed, Aug 29, 2018 at 1:58 AM, Tim Bell  wrote:
>>> Thomas:
>>>
>>>> when trying to run hg defpath to set the paths to the jdk12 submit
>>>> repo (http://hg.openjdk.java.net/jdk/submit), it fails:
>>>>
>>>> thomas@t450:/shared/projects/openjdk/jdk-submit/source$ hg defpath -du
>>>> stuefe
>>>> abort: http://hg.openjdk.java.net/jdk/submit: Repository not found
>>>> No hgrc files updated
>>>
>>>
>>> Adding the ops list  in case others have seen this.
>>>
>>>> But it works for the submit repo for jdk11
>>>> (http://hg.openjdk.java.net/jdk/submit11/), and there it updates the
>>>> .hg/hgrc just fine.
>>>
>>>
>>> FWIW, I see the same thing.  I don't have an explanation yet.
>>>
>>> I created fresh clones and then tried 'hg defpath -du tbell':
>>>
>>> works: http://hg.openjdk.java.net/jdk/sandbox
>>> works: http://hg.openjdk.java.net/jdk/submit11
>>> fails: http://hg.openjdk.java.net/jdk/submit
>>>
>>>> Not sure what the problem is. Both URLs are reachable fine in a browser.
>>>>
>>>> Any ideas?
>>>
>>>
>>> I am stumped at the moment, but I will keep digging.
>>>
>>> Tim
>>>

Re: defpath extension won't work with jdk/submit

[Thomas Stüfe]

Adding the push path manually to hgrc works fine, so I use that as a workaround.

Thanks, Thomas

On Wed, Aug 29, 2018 at 8:01 AM, Thomas Stüfe  wrote:
> Thank you Tim. Happy to know it is not my setup. Lets see what ops says.
>
> Best, Thomas
>
> On Wed, Aug 29, 2018 at 1:58 AM, Tim Bell  wrote:
>> Thomas:
>>
>>> when trying to run hg defpath to set the paths to the jdk12 submit
>>> repo (http://hg.openjdk.java.net/jdk/submit), it fails:
>>>
>>> thomas@t450:/shared/projects/openjdk/jdk-submit/source$ hg defpath -du
>>> stuefe
>>> abort: http://hg.openjdk.java.net/jdk/submit: Repository not found
>>> No hgrc files updated
>>
>>
>> Adding the ops list  in case others have seen this.
>>
>>> But it works for the submit repo for jdk11
>>> (http://hg.openjdk.java.net/jdk/submit11/), and there it updates the
>>> .hg/hgrc just fine.
>>
>>
>> FWIW, I see the same thing.  I don't have an explanation yet.
>>
>> I created fresh clones and then tried 'hg defpath -du tbell':
>>
>> works: http://hg.openjdk.java.net/jdk/sandbox
>> works: http://hg.openjdk.java.net/jdk/submit11
>> fails: http://hg.openjdk.java.net/jdk/submit
>>
>>> Not sure what the problem is. Both URLs are reachable fine in a browser.
>>>
>>> Any ideas?
>>
>>
>> I am stumped at the moment, but I will keep digging.
>>
>> Tim
>>

Re: defpath extension won't work with jdk/submit

[Thomas Stüfe]

Thank you Tim. Happy to know it is not my setup. Lets see what ops says.

Best, Thomas

On Wed, Aug 29, 2018 at 1:58 AM, Tim Bell  wrote:
> Thomas:
>
>> when trying to run hg defpath to set the paths to the jdk12 submit
>> repo (http://hg.openjdk.java.net/jdk/submit), it fails:
>>
>> thomas@t450:/shared/projects/openjdk/jdk-submit/source$ hg defpath -du
>> stuefe
>> abort: http://hg.openjdk.java.net/jdk/submit: Repository not found
>> No hgrc files updated
>
>
> Adding the ops list  in case others have seen this.
>
>> But it works for the submit repo for jdk11
>> (http://hg.openjdk.java.net/jdk/submit11/), and there it updates the
>> .hg/hgrc just fine.
>
>
> FWIW, I see the same thing.  I don't have an explanation yet.
>
> I created fresh clones and then tried 'hg defpath -du tbell':
>
> works: http://hg.openjdk.java.net/jdk/sandbox
> works: http://hg.openjdk.java.net/jdk/submit11
> fails: http://hg.openjdk.java.net/jdk/submit
>
>> Not sure what the problem is. Both URLs are reachable fine in a browser.
>>
>> Any ideas?
>
>
> I am stumped at the moment, but I will keep digging.
>
> Tim
>

defpath extension won't work with jdk/submit

[Thomas Stüfe]

Hi all,

when trying to run hg defpath to set the paths to the jdk12 submit
repo (http://hg.openjdk.java.net/jdk/submit), it fails:

thomas@t450:/shared/projects/openjdk/jdk-submit/source$ hg defpath -du stuefe
abort: http://hg.openjdk.java.net/jdk/submit: Repository not found
No hgrc files updated

But it works for the submit repo for jdk11
(http://hg.openjdk.java.net/jdk/submit11/), and there it updates the
.hg/hgrc just fine.

Not sure what the problem is. Both URLs are reachable fine in a browser.

Any ideas?

Thanks!

Thomas

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Thomas Stüfe]

On Fri, Jun 22, 2018 at 1:57 PM, David Holmes  wrote:
> On 21/06/2018 10:37 PM, Thomas Stüfe wrote:
>>
>> On Thu, Jun 21, 2018 at 1:27 PM, David Holmes 
>> wrote:
>>>
>>> On 21/06/2018 10:05 AM, Martin Buchholz wrote:
>>>>
>>>>
>>>> On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz >>> <mailto:marti...@google.com>> wrote:
>>>>
>>>>  Hi David and build-dev folk,
>>>>
>>>>  After way too much build/hotspot hacking, I have a better fix:
>>>>
>>>>  clang inlined os::current_stack_pointer into its caller __in the
>>>>  same translation unit___ (that could be fixed in a separate change)
>>>>  so of course in this case it didn't have to follow the ABI.  Fix is
>>>>  obvious in hindsight:
>>>>
>>>>  -address os::current_stack_pointer() {
>>>>  +NOINLINE address os::current_stack_pointer() {
>>>>
>>>>
>>>> If y'all like the addition of NOINLINE, it should probably be added to
>>>> all
>>>> of the 14 variants of os::current_stack_pointer.
>>>> Gives me a chance to try out the submit repo.
>>>
>>>
>>>
>>> I can't help but think other platforms actually rely on it being inlined
>>> so
>>> that it really does return the stack pointer of the method calling
>>> os::current_stack_pointer!
>>>
>>
>> But we only inline today if caller is in the same translation unit and
>> builds with optimization, no?
>
>
> Don't know, but Martin's encountering a case where it is being inlined - is
> that likely to be unique for some reason?
>

Okay I may have confused myself.

My original reasoning was: A lot of calls to
os::current_stack_pointer() today happen not-inlined. That includes
calls from outside os__.cpp, and calls from inside
os__.cpp if slowdebug. Hence, since the VM - in particular
the slowdebug one - seems to work fine, it should be okay to mark
os::current_stack_pointer() unconditionally as NOINLINE.

However, then I saw that the only "real" function (real meaning not
just asserting something) using os::current_stack_pointer() and living
in the same translation unit is os::current_frame(), e.g. on linux:

frame os::current_frame() {
  intptr_t* fp = _get_previous_fp();
  frame myframe((intptr_t*)os::current_stack_pointer(),
(intptr_t*)fp,
CAST_FROM_FN_PTR(address, os::current_frame));
  if (os::is_first_C_frame(&myframe)) {
// stack is not walkable
return frame();
  } else {
return os::get_sender_for_C_frame(&myframe);
  }
}

how does this even work if os::current_stack_pointer() is not inlined?
In slowdebug? Would the frame object in this case not contain the SP
from the frame built up for os::current_stack_pointer()?

So, now I wonder if making os::current_stack_pointer() NOINLINE would
break os::current_frame() - make if produce frames with a slightly-off
SP. os::current_frame() is used e.g. in NMT. So, I wonder if NMT still
works if os::current_stack_pointer is made NOINLINE, or in slowdebug.

> I never assume the compiler does the obvious these days :) or that there
> can't be clever link-time tricks as well.

Oh. I did not think of that at all, you are right.

>
> Maybe the safest thing to do is to only make a change for the clang case and
> leave everything else alone.
>

It would be better to know for sure, though.

..Thomas

> David
> -
>
>>
>> ..Thomas
>>
>>> David

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Thomas Stüfe]

On Thu, Jun 21, 2018 at 1:27 PM, David Holmes  wrote:
> On 21/06/2018 10:05 AM, Martin Buchholz wrote:
>>
>> On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz > > wrote:
>>
>> Hi David and build-dev folk,
>>
>> After way too much build/hotspot hacking, I have a better fix:
>>
>> clang inlined os::current_stack_pointer into its caller __in the
>> same translation unit___ (that could be fixed in a separate change)
>> so of course in this case it didn't have to follow the ABI.  Fix is
>> obvious in hindsight:
>>
>> -address os::current_stack_pointer() {
>> +NOINLINE address os::current_stack_pointer() {
>>
>>
>> If y'all like the addition of NOINLINE, it should probably be added to all
>> of the 14 variants of os::current_stack_pointer.
>> Gives me a chance to try out the submit repo.
>
>
> I can't help but think other platforms actually rely on it being inlined so
> that it really does return the stack pointer of the method calling
> os::current_stack_pointer!
>

But we only inline today if caller is in the same translation unit and
builds with optimization, no?

..Thomas

> David

Re: RFR(xxs): 8205407: [windows, vs<2017] C4800 after 8203197

[Thomas Stüfe]

Yes, but you are a Committer, so that is fine in this case, since we
have two Reviewers already :)

Thank you! I'll push now.

..Thomas

On Wed, Jun 20, 2018 at 5:00 PM, Baesken, Matthias
 wrote:
> Hi  Thomas , looks good  (however I am not a Reviewer) !
>
>
>> From: Thomas Stüfe 
>> Date: Wed, Jun 20, 2018 at 10:00 AM
>> Subject: RFR(xxs): 8205407: [windows, vs<2017] C4800 after 8203197
>> To: build-dev 
>>
>>
>> Hi all,
>>
>> May I please have reviews for this small build fix?
>>
>> https://bugs.openjdk.java.net/browse/JDK-8205407
>> http://cr.openjdk.java.net/~stuefe/webrevs/8205497-windows-
>> c4800/webrev.00/webrev/
>>
>> In short, we need to disable C4800 for the JVM build as well since 8203197.
>>
>> Thank you,
>>
>> Thomas

Re: RFR(xxs): 8205407: [windows, vs<2017] C4800 after 8203197

[Thomas Stüfe]

Thanks Tim.


On Wed, Jun 20, 2018 at 3:48 PM, Tim Bell  wrote:
> Thomas:
>
>> May I please have reviews for this small build fix?
>>
>> https://bugs.openjdk.java.net/browse/JDK-8205407
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8205497-windows-c4800/webrev.00/webrev/
>>
>> In short, we need to disable C4800 for the JVM build as well since
>> 8203197.
>
>
> Looks good.
>
> Tim
>
>

Re: RFR(xxs): 8205407: [windows, vs<2017] C4800 after 8203197

[Thomas Stüfe]

jdk-submit ran through without errors.

Anyone?

Plus, do you agree this falls under the trivial rule?

Thanks, Thomas

On Wed, Jun 20, 2018 at 10:00 AM, Thomas Stüfe  wrote:
> Hi all,
>
> May I please have reviews for this small build fix?
>
> https://bugs.openjdk.java.net/browse/JDK-8205407
> http://cr.openjdk.java.net/~stuefe/webrevs/8205497-windows-c4800/webrev.00/webrev/
>
> In short, we need to disable C4800 for the JVM build as well since 8203197.
>
> Thank you,
>
> Thomas

RFR(xxs): 8205407: [windows, vs<2017] C4800 after 8203197

[Thomas Stüfe]

Hi all,

May I please have reviews for this small build fix?

https://bugs.openjdk.java.net/browse/JDK-8205407
http://cr.openjdk.java.net/~stuefe/webrevs/8205497-windows-c4800/webrev.00/webrev/

In short, we need to disable C4800 for the JVM build as well since 8203197.

Thank you,

Thomas

Re: RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default

[Thomas Stüfe]

Looks good to me, Volker. Thank you for fixing the tests.

..Thomas

On Mon, Jun 18, 2018 at 6:04 PM, Volker Simonis
 wrote:
> On Mon, Jun 18, 2018 at 8:17 AM, David Holmes  wrote:
>> Hi Volker,
>>
>> src/hotspot/share/runtime/globals.hpp
>>
>> This change should not be needed! We do minimal VM builds without CDS and we
>> don't have to touch the UseSharedSpaces defaults (unless recent change have
>> broken this - in which case that needs to be addressed in its own right!)
>>
>
> Yes, you're right, CDS_ONLY/NOT_CDS isn't really required here,
> because UseSharedSpaces is reseted later on at the end of
> Arguments::parse(). I just thought it would be cleaner to disable it
> statically, if the VM doesn't support it. But anyway I don't really
> mind and I've reverted that change in globals.hpp.
>
>> src/hotspot/share/classfile/javaClasses.cpp
>>
>> AFAICS you should be using INCLUDE_CDS in the ifdefs not
>> INCLUDE_CDS_JAVA_HEAP. But again I'm unclear (as was Thomas) why this should
>> be needed as we have not needed it before. As Thomas notes we have:
>>
>> ./hotspot/share/memory/metaspaceShared.hpp:  static bool
>> is_archive_object(oop p) NOT_CDS_JAVA_HEAP_RETURN_(false);
>> ./hotspot/share/classfile/stringTable.hpp:  static oop
>> create_archived_string(oop s, Thread* THREAD)
>> NOT_CDS_JAVA_HEAP_RETURN_(NULL);
>>
>> so these methods should be defined when CDS is not available.
>>
>
> Thomas and you are right. Must have been a mis-configuration on AIX
> where I saw undefined symbols at link time. I've removed the ifdefs
> from javaClasses.cpp now.
>
> Finally, I've also wrapped all the FileMapInfo fields in vmStructs.cpp
> into CDS_ONLY macros as suggested by Jiangli because the really only
> make sense for a CDS-enabled VM.
>
> Here's the new webrev:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965.v3/
>
> Please let me know if you think there's still something missing.
>
> Regards,
> Volker
>
>
>> ??
>>
>> Thanks,
>> David
>> -
>>
>>
>>
>>
>>
>> On 15/06/2018 12:26 AM, Volker Simonis wrote:
>>>
>>> Hi,
>>>
>>> can I please have a review for the following fix:
>>>
>>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/
>>> https://bugs.openjdk.java.net/browse/JDK-8204965
>>>
>>> CDS does currently not work on AIX because of the way how we
>>> reserve/commit memory on AIX. The problem is that we're using a
>>> combination of shmat/mmap depending on the page size and the size of
>>> the memory chunk to reserve. This makes it impossible to reliably
>>> reserve the memory for the CDS archive and later on map the various
>>> parts of the archive into these regions.
>>>
>>> In order to fix this we would have to completely rework the memory
>>> reserve/commit/uncommit logic on AIX which is currently out of our
>>> scope because of resource limitations.
>>>
>>> Unfortunately, I could not simply disable CDS in the configure step
>>> because some of the shared code apparently relies on parts of the CDS
>>> code which gets excluded from the build when CDS is disabled. So I
>>> also fixed the offending parts in hotspot and cleaned up the configure
>>> logic for CDS.
>>>
>>> Thank you and best regards,
>>> Volker
>>>
>>> PS: I did run the job through the submit forest
>>> (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results
>>> weren't really useful because they mention build failures on linux-x64
>>> which I can't reproduce locally.
>>>
>>

Re: RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default

[Thomas Stüfe]

Hi Volker,



On Fri, Jun 15, 2018 at 10:05 AM, Volker Simonis
 wrote:
> On Thu, Jun 14, 2018 at 9:04 PM, Thomas Stüfe  wrote:
>> Hi Volker,
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/make/autoconf/hotspot.m4.udiff.html
>>
>> Seems like a roundabout way to have a platform specific default value.
>>
>> Why not determine a default value beforehand:
>>
>> if test "x$OPENJDK_TARGET_OS" = "xaix"; then
>>   ENABLE_CDS_DEFAULT="false"
>> else
>>   ENABLE_CDS_DEFAULT=true"
>> fi
>>
>> AC_ARG_ENABLE([cds], [AS_HELP_STRING([--enable-cds@<:@=yes/no/auto@:>@],
>>  [enable class data sharing feature in non-minimal VM. Default is
>> ${ENABLE_CDS_DEFAULT}.])])
>>
>> and so on?
>>
>
> I've just followed the pattern used for '--enable-aot' right above the
> code I changed.
>
> Moreover, I don't think that we would save any code because we would
> still have to check for AIX in the '--enable-cds=yes' case. Also, the
> new reporting added later in the file (see "AC_MSG_CHECKING([if cds
> should be enabled])" seems easier to me without the extra default
> value. So if you don't mind I'd prefer to leave it as is.
>

I just think that having three options (on/off/auto) is confusing.
Okay, I still think a platform dependent default value would be
cleaner, but I can live with auto="yes, if possible".

>> See also what we did for "8202325: [aix] disable warnings-as-errors by 
>> default".
>>
>> --
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/src/hotspot/share/classfile/javaClasses.cpp.udiff.html
>>
>> Here, do we really need to exclude this from compiling,
>> DumpSharedSpaces = false is not enough?
>>
>
> Yes, we need it because the excluded code references methods (e.g.
> 'StringTable::create_archived_string()') which are not compiled into
> libjvm.so if we disable CDS.
>

Are you really sure?

Both MetaspaceShared::is_archive_object() and
StringTable::create_archived_string() are available outside CDS, the
latter explicitly returns NULL if CDS is not enabled at build time:
NOT_CDS_JAVA_HEAP_RETURN_(NULL);

I also just built a Linux vm without CDS, and it compiles without
problems without the #ifdef. But maybe AIX is different.

---

But all this is idle nitpicking, so I leave it up to you if you change
anything. The change is good in its current form to me and I do not
need another webrev.

Best Regards, Thomas

>>
>> Best Regards, Thomas
>>
>> On Thu, Jun 14, 2018 at 4:26 PM, Volker Simonis
>>  wrote:
>>> Hi,
>>>
>>> can I please have a review for the following fix:
>>>
>>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/
>>> https://bugs.openjdk.java.net/browse/JDK-8204965
>>>
>>> CDS does currently not work on AIX because of the way how we
>>> reserve/commit memory on AIX. The problem is that we're using a
>>> combination of shmat/mmap depending on the page size and the size of
>>> the memory chunk to reserve. This makes it impossible to reliably
>>> reserve the memory for the CDS archive and later on map the various
>>> parts of the archive into these regions.
>>>
>>> In order to fix this we would have to completely rework the memory
>>> reserve/commit/uncommit logic on AIX which is currently out of our
>>> scope because of resource limitations.
>>>
>>> Unfortunately, I could not simply disable CDS in the configure step
>>> because some of the shared code apparently relies on parts of the CDS
>>> code which gets excluded from the build when CDS is disabled. So I
>>> also fixed the offending parts in hotspot and cleaned up the configure
>>> logic for CDS.
>>>
>>> Thank you and best regards,
>>> Volker
>>>
>>> PS: I did run the job through the submit forest
>>> (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results
>>> weren't really useful because they mention build failures on linux-x64
>>> which I can't reproduce locally.

Re: RFR: 8205091: AIX: build errors in hotspot after 8203641: Refactor String Deduplication into shared

[Thomas Stüfe]

Hi Matthias,

Good catch. Patch for me is good if you guys agree on a good uncommon name.

Gruß Thomas

On Fri, Jun 15, 2018, 09:48 Baesken, Matthias 
wrote:

> Please review  this small  change  that fixes the AIX build after
> "8203641: Refactor String Deduplication into shared".
>
> We are getting this  compilation error  :
> /build_ci_jdk_jdk_rs6000_64/src/hotspot/share/gc/shared/stringdedup/stringDedup.hpp",
> line 107.38: 1540-0063 (S) The text "1" is unexpected.
>
>
> Looks like the name of the second template parameter (STAT)
>
> template 
> static void initialize_impl();
>
> is clashing with defines from the AIX system headers  (where I find
> #define STAT  1 ) .
> Renaming  STAT  to something else fixes the build on AIX .
>
> Webrev :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8205091/
>
> Bug :
>
> https://bugs.openjdk.java.net/browse/JDK-8205091
>
>
> Thanks, Matthias
>

Re: RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default

[Thomas Stüfe]

Hi Volker,

http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/make/autoconf/hotspot.m4.udiff.html

Seems like a roundabout way to have a platform specific default value.

Why not determine a default value beforehand:

if test "x$OPENJDK_TARGET_OS" = "xaix"; then
  ENABLE_CDS_DEFAULT="false"
else
  ENABLE_CDS_DEFAULT=true"
fi

AC_ARG_ENABLE([cds], [AS_HELP_STRING([--enable-cds@<:@=yes/no/auto@:>@],
 [enable class data sharing feature in non-minimal VM. Default is
${ENABLE_CDS_DEFAULT}.])])

and so on?

See also what we did for "8202325: [aix] disable warnings-as-errors by default".

--

http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/src/hotspot/share/classfile/javaClasses.cpp.udiff.html

Here, do we really need to exclude this from compiling,
DumpSharedSpaces = false is not enough?


Best Regards, Thomas

On Thu, Jun 14, 2018 at 4:26 PM, Volker Simonis
 wrote:
> Hi,
>
> can I please have a review for the following fix:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/
> https://bugs.openjdk.java.net/browse/JDK-8204965
>
> CDS does currently not work on AIX because of the way how we
> reserve/commit memory on AIX. The problem is that we're using a
> combination of shmat/mmap depending on the page size and the size of
> the memory chunk to reserve. This makes it impossible to reliably
> reserve the memory for the CDS archive and later on map the various
> parts of the archive into these regions.
>
> In order to fix this we would have to completely rework the memory
> reserve/commit/uncommit logic on AIX which is currently out of our
> scope because of resource limitations.
>
> Unfortunately, I could not simply disable CDS in the configure step
> because some of the shared code apparently relies on parts of the CDS
> code which gets excluded from the build when CDS is disabled. So I
> also fixed the offending parts in hotspot and cleaned up the configure
> logic for CDS.
>
> Thank you and best regards,
> Volker
>
> PS: I did run the job through the submit forest
> (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results
> weren't really useful because they mention build failures on linux-x64
> which I can't reproduce locally.

Re: RFR(xs): 8204935: [aix] TOC overflow in libjvm.so (release build)

[Thomas Stüfe]

Thank you Magnus.

On Wed, Jun 13, 2018 at 3:40 PM, Magnus Ihse Bursie
 wrote:
> Looks good to me.
>
> /Magnus
>
>> 13 juni 2018 kl. 10:28 skrev Thomas Stüfe :
>>
>> Thank you Matthias!
>>
>> I'll wait for someone from the Oracle Build Group to have a say.
>>
>> ..Thomas
>>
>> On Wed, Jun 13, 2018 at 9:43 AM, Baesken, Matthias
>>  wrote:
>>> Hi Thomas , thanks for fixing it, looks good  ( not a Reviewer however) .
>>>
>>> Best regards, Matthias
>>>
>>>
>>>
>>>> -Original Message-
>>>> From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-
>>>> boun...@openjdk.java.net] On Behalf Of Thomas Stüfe
>>>> Sent: Mittwoch, 13. Juni 2018 08:14
>>>> To: build-dev ; ppc-aix-port-
>>>> d...@openjdk.java.net
>>>> Subject: RFR(xs): 8204935: [aix] TOC overflow in libjvm.so (release build)
>>>>
>>>> Hi all,
>>>>
>>>> may I have reviews for this small build fix:
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8204935
>>>> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8204935-aix-toc-
>>>> overflow/webrev.00/webrev/
>>>>
>>>> We finally have reached the point where even the standard (not gtest)
>>>> release build libjvm.so got too large and blew its TOC. Hurray for
>>>> templates. On the bright side, no need to keep different flags for
>>>> different libjvm variants anymore.
>>>>
>>>> Kudos to whoever added auto-automake to the build, that makes these
>>>> kind of changes really easier :)
>>>>
>>>> Thank you,
>>>>
>>>> Best Regards, Thomas
>