RE: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-29 Thread Nick Gasson
Hi Alan, Martin,

Do you want me to make any more changes to this patch, or is it OK to go in 
with just the modified #ifdef?

Thanks,
Nick

From: Martin Buchholz 
Sent: 28 November 2018 11:33
To: Nick Gasson 
Cc: Alan Bateman ; Magnus Ihse Bursie 
; build-dev ; 
core-libs-...@openjdk.java.net; nd 
Subject: Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32



On Tue, Nov 27, 2018 at 7:25 PM, Nick Gasson 
mailto:nick.gas...@arm.com>> wrote:
> I missed one thing then looking at this. In TimeZone_md.c it should be
> #ifdef MACOSX rather than #ifndef. I can sponsor the change for you but
> I need to change this one line before pushing.

Hi Alan,

Yes feel free to modify it. I think looking at the at other files
with these #defines more closely, is it the case that the #ifndef
MACOSX check is only required for statvfs64? As in
e.g. UnixNativeDispatcher.c. So TimeZone_md.c should look like
this:

#if defined(_ALLBSD_SOURCE)
  #define stat64 stat
  #define lstat64 lstat
  #define fstat64 fstat
#endif

I don't have access to any OS X machines to test unfortunately.

But I wonder if a better way to handle this is to check for the
presence of the stat64 functions in the configure script, and
then we could just write something like this, which would be a
lot clearer:

#if !defined(HAVE_STAT64)
  #define stat64 stat
#endif


The best way is to implement

https://bugs.openjdk.java.net/browse/JDK-8165620
"Entire JDK should be built with -D_FILE_OFFSET_BITS=64"
but yes, another good way is to do as you suggest, have configure define 
HAVE_ for all known functions with a 64-bit variant and then put them into 
a header file with proper ifdefs for platforms that don't have them.

You could also "simply" add
#define _FILE_OFFSET_BITS 64
but you have to do it for cliques of files that share ambiguously sized data 
simultaneously.



Re: [12] Review Request: 8212680 (JDK12b14/Solaris-sparc) SplashScreen::getSplashScreen call fails with ULE: "libsplashscreen.so: ld.so.1: java: fatal: libz.so.1: open failed: No such file o

2018-11-29 Thread Sergey Bylokhov

Then it can be simplified further:
http://cr.openjdk.java.net/~serb/8212680/webrev.01


I added the new option since I am not sure how important the usage of
"PNG_MAXIMUM_INFLATE_WINDOW or PNG_SKIP_sRGB_CHECK_PROFILE"

As I said, I don't see anything important ...



--
Best regards, Sergey.


Re: [12] Review Request: 8212680 (JDK12b14/Solaris-sparc) SplashScreen::getSplashScreen call fails with ULE: "libsplashscreen.so: ld.so.1: java: fatal: libz.so.1: open failed: No such file o

2018-11-29 Thread Philip Race

+1 from me.

-phil.

On 11/29/18, 7:27 PM, Sergey Bylokhov wrote:

Then it can be simplified further:
http://cr.openjdk.java.net/~serb/8212680/webrev.01


I added the new option since I am not sure how important the usage of
"PNG_MAXIMUM_INFLATE_WINDOW or PNG_SKIP_sRGB_CHECK_PROFILE"

As I said, I don't see anything important ...





Re: [12] Review Request: 8212680 (JDK12b14/Solaris-sparc) SplashScreen::getSplashScreen call fails with ULE: "libsplashscreen.so: ld.so.1: java: fatal: libz.so.1: open failed: No such file o

2018-11-29 Thread Philip Race




On 11/29/18, 9:38 AM, Sergey Bylokhov wrote:

On 28/11/2018 20:24, Philip Race wrote:

Not something we want to do if there's any way out of it.

can we just disable PNG_SET_OPTION_SUPPORTED in
pnglibconfig.h which is something that is "generated" and not part of 
the png source ?


I don't see anything that is important enabled by that option.


I added the new option since I am not sure how important the usage of
"PNG_MAXIMUM_INFLATE_WINDOW or PNG_SKIP_sRGB_CHECK_PROFILE"
which is also controlled by PNG_SET_OPTION_SUPPORTED, it also may 
affect some other options

which will be added later.


As I said, I don't see anything important ...


So the current fix adds the new property, as if it was added by the 
pnglib maintainer,
and reuses this property for our purpose(comment default define in 
"pnglibconfig.h"

and enable it in some cases in makefile).

BTW the small clarification for other questions in the thread:
  the changes in upstream library is in "png.h" file only, the 
"pnglibconf.h" is our config on how

to build the lib.



Right, that is why I suggested confining the change to that file.

-phil.


RFR (round 3), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-29 Thread Roman Kennke
Hello all,

Thanks so far for all the reviews and support!

I forgot to update the 'round' yesterday. We are in round 3 now :-)
Also, I fixed the numbering of my webrevs to match the review-round. ;-)

Things we've changed today:
- We moved shenandoah-specific code out of .ad files into our own .ad
files under gc/shenandoah (in shenandoah-gc), how cool is that? This
requires an addition in build machinery though, see
make/hotspot/gensrc/GensrcAdlc.gmk (in shared-build).
- Improved zero-disabling and build-code-simplification as suggested by
Magnus and Per
- Cleaned up some leftovers in C2
- Improved C2 loop opts code by introducing another APIs in
BarrierSetC2. See the new APIs in shared-gc under BarrierSetC2.hpp.
- I don't see where it makes sense to put INCLUDE_SHENANDOAHGC guards now.
- We would all very much prefer to keep ShenandoahXYZNode names, as
noted earlier. This stuff is Shenandoah-specific, so let's just call it
that.
- Rehashed Shenandoah tests (formatting, naming, directory layout, etc)
- Rebased on jdk-12+22

- Question: let us know if you need separate RFE for the new BSC2 APIs?

http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/03/

Thanks,
Roman

> Alright, we fixed:
> - The minor issues that Kim reported in shared-gc
> - A lot of fixes in shared-tests according to Leonid's review
> - Disabled SA heapdumping similar to ZGC as Per suggested
> 
> Some notes:
> Leonid:  test/hotspot/jtreg/gc/TestFullGCCount.java was actually
> correct. The @requires there means to exclude runs with both CMS and
> ExplicitGCInvokesConcurrent at the same time, because that would be
> (expectedly) failing. It can run CMS, default GC and any other GC just
> fine. Adding the same clause for Shenandoah means the same, and filters
> the combination (+UseShenandoahGC)+(+ExplicitGCInvokesConcurrent). I
> made the condition a bit clearer by avoiding triple-negation.
> 
> See:
> http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008457.html
> 
> Per: Disabling the SA part for heapdumping makes 2 tests fail:
> - test/hotspot/jtreg/serviceability/sa/ClhsdbJhisto.java
> - test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java
> 
> we filter them for Shenandoah now. I'm wondering: how do you get past
> those with ZGC?
> 
> See:
> http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008466.html
> 
> (Note to Leonid and tests reviewers: I'll add those related filters in
> next round).
> 
> Vladimir: Roland integrated a bunch of changes to make loop* code look
> better. I can tell that we're not done with C2 yet. Can you look over
> the code and see what is ok, and especially what is not ok, so that we
> can focus our efforts on the relevant parts?
> 
> Updated set of webrevs:
> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/01/
> 
> Thanks,
> Roman
> 
> 
>> Hi,
>>
>> This is the first round of changes for including Shenandoah GC into
>> mainline.
>> I divided the review into parts that roughly correspond to the mailing lists
>> that would normally review it, and I divided it into 'shared' code
>> changes and
>> 'shenandoah' code changes (actually, mostly additions). The intend is to
>> eventually
>> push them as single 'combined' changeset, once reviewed.
>>
>> JEP:
>>   https://openjdk.java.net/jeps/189
>> Bug entry:
>>
>>  https://bugs.openjdk.java.net/browse/JDK-8214259
>>
>> Webrevs:
>>   http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/
>>
>> For those who want to see the full change, have a look at the
>> shenandoah-complete
>> 
>> directory,
>> it contains the full combined webrev. Alternatively, there is the file
>> shenandoah-master.patch
>> ,
>> which is what I intend to commit (and which should be equivalent to the
>> 'shenandoah-complete' webrev).
>>
>> Sections to review (at this point) are the following:
>>  *) shenandoah-gc
>> 
>>     - Actual Shenandoah implementation, almost completely residing in
>> gc/shenandoah
>>
>>  *) shared-gc
>> 
>>     - This is mostly boilerplate that is common to any GC
>>     - referenceProcessor.cpp has a little change to make one assert not
>> fail (next to CMS and G1)
>>     - taskqueue.hpp has some small adjustments to enable subclassing
>>
>>  *) shared-serviceability
>> 
>>     - The usual code to support another GC
>>
>>  *) shared-runtime
>> 
>>     - A number of friends declarations to allow Shenandoah iterators to
>> hook up with,
>>   e.g. ClassLoaderData, CodeCache, etc
>>     - Warning and disabling JFR LeakProfiler
>>     - 

Re: RFR 8214309: Enhance makefiles to allow generating JCov instrumented build

2018-11-29 Thread Erik Joelsson

Looks good to me.

/Erik

On 2018-11-27 14:58, Alexandre (Shura) Iline wrote:

Hi.

Please take a look on a suggested change in OpenJDK make system which allows to 
generate an image instrumented for code coverage analysis with JCov. There will 
need to be follow-on changes making it possible to run test with code coverage.

When —with-jcov options supplied during configuration, it is possible to use 
jcov-image target which generates an instrumented image in images/jdk-jcov. 
There is a code coverage template within it. There is also jcov-bundle target. 
It is also possible to instrument an external JDK image by pointing to it by 
--with-jcov-input-jdk configuration option.

Webrev: http://cr.openjdk.java.net/~shurailine/8214309/webrev.02

Many thanks to Erik for helping out with this change.

Shura


Re: [12] Review Request: 8212680 (JDK12b14/Solaris-sparc) SplashScreen::getSplashScreen call fails with ULE: "libsplashscreen.so: ld.so.1: java: fatal: libz.so.1: open failed: No such file o

2018-11-29 Thread Sergey Bylokhov

On 28/11/2018 20:24, Philip Race wrote:

Not something we want to do if there's any way out of it.

can we just disable PNG_SET_OPTION_SUPPORTED in
pnglibconfig.h which is something that is "generated" and not part of the png 
source ?

I don't see anything that is important enabled by that option.


I added the new option since I am not sure how important the usage of
"PNG_MAXIMUM_INFLATE_WINDOW or PNG_SKIP_sRGB_CHECK_PROFILE"
which is also controlled by PNG_SET_OPTION_SUPPORTED, it also may affect some 
other options
which will be added later.

So the current fix adds the new property, as if it was added by the pnglib 
maintainer,
and reuses this property for our purpose(comment default define in 
"pnglibconfig.h"
and enable it in some cases in makefile).

BTW the small clarification for other questions in the thread:
  the changes in upstream library is in "png.h" file only, the "pnglibconf.h" 
is our config on how
to build the lib.

--
Best regards, Sergey.


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

2018-11-29 Thread Adam Farley8
Hi All,

The build passed on xlC 13.1 with the makefile patch I proposed (good 
catch on the comments Volkar!).

With Volkar, Erik, Matthias, and Magnus all approving the change, it 
sounds like we're good to merge!

Volkar, can you do the honours?

Best Regards

Adam Farley 
IBM Runtimes

P.S. I approve the change too. ;)


Volker Simonis  wrote on 29/11/2018 11:54:33:

> From: Volker Simonis 
> To: Magnus Ihse Bursie 
> Cc: build-dev , ppc-aix-port-
> d...@openjdk.java.net, adam.far...@uk.ibm.com
> Date: 29/11/2018 11:54
> Subject: Re: RFR(XS): 8214063: [AIX] Disable symbol visibility flags
> 
> On Thu, Nov 29, 2018 at 12:20 PM Magnus Ihse Bursie
>  wrote:
> >
> > On 2018-11-27 16:33, Volker Simonis wrote:
> >
> > > Hi,
> > >
> > > can I please have a review for the following trivial change which
> > > simply disables the symbol visibility flags on AIX:
> > >
> > > INVALID URI REMOVED
> 
u=http-3A__cr.openjdk.java.net_-7Esimonis_webrevs_2018_8214063_=DwIBaQ=jf_iaSHvJObTbx-
> siA1ZOg=P5m8KWUXJf-
> CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=6y4Npxy6aG4q8E9Xca--
> YxF4UGVrVEIqu_wVvivFVUA=DptrWUUtJCcpUCbCWkkBOeFJCVk5im3hm9T_DcD0Jd8=
> > > INVALID URI REMOVED
> 
u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8214063=DwIBaQ=jf_iaSHvJObTbx-
> siA1ZOg=P5m8KWUXJf-
> CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=6y4Npxy6aG4q8E9Xca--
> YxF4UGVrVEIqu_wVvivFVUA=jBFABkJb5E5W9K8pMX794-3gnpLfPyi3oASA1kizQ7A=
> > Looks good to me. I am sorry for the mess I caused by optimisically
> > trying to fix things on a platform I could not compile on... :(
> >
> 
> Thanks for the review and don't worry! We really appreciate your
> continued help. It's really us who should have tested and spotted the
> problems earlier :)
> 
> Regards,
> Volker
> 
> > This also reminds me that the visibility flags *really* should move 
into
> > configure/spec, not be sprinkled like this in the make files.
> >
> > /Magnus
> > >
> > > 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). 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] INVALID URI REMOVED
> 
u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8202322=DwIBaQ=jf_iaSHvJObTbx-
> siA1ZOg=P5m8KWUXJf-
> CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=6y4Npxy6aG4q8E9Xca--
> YxF4UGVrVEIqu_wVvivFVUA=pd7-rH7OPxeaq2g6S0dQPmb_3-8PLi8JZFKcP_Abp6Q=
> > > [2] INVALID URI REMOVED
> 
u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8204541=DwIBaQ=jf_iaSHvJObTbx-
> siA1ZOg=P5m8KWUXJf-
> CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=6y4Npxy6aG4q8E9Xca--
> YxF4UGVrVEIqu_wVvivFVUA=q7KHUASpF-opdcLXbTTUT1bPoKrkTeaHTtd7c2jN4rc=
> >
> 

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU


Re: RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-29 Thread Per Liden
Oh, I filed a bug [1] and sent out a fix for review, before noticing 
Jini's mail.


[1] https://bugs.openjdk.java.net/browse/JDK-8214484

/Per

On 11/29/18 5:39 AM, Jini George wrote:

Hi Roman,

The SA tests which iterate over the heap (ClhsdbJhisto.java, 
TestHeapDumpFor*.java) fail if ZGC is passed as an option to these also 
(Am planning on fixing these). It might be better to avoid invoking 
those tests for Shenandoah now with an @requires tag.


Thank you,
Jini

On 11/29/2018 3:59 AM, Roman Kennke wrote:

Alright, we fixed:
- The minor issues that Kim reported in shared-gc
- A lot of fixes in shared-tests according to Leonid's review
- Disabled SA heapdumping similar to ZGC as Per suggested

Some notes:
Leonid:  test/hotspot/jtreg/gc/TestFullGCCount.java was actually
correct. The @requires there means to exclude runs with both CMS and
ExplicitGCInvokesConcurrent at the same time, because that would be
(expectedly) failing. It can run CMS, default GC and any other GC just
fine. Adding the same clause for Shenandoah means the same, and filters
the combination (+UseShenandoahGC)+(+ExplicitGCInvokesConcurrent). I
made the condition a bit clearer by avoiding triple-negation.

See:
http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008457.html 



Per: Disabling the SA part for heapdumping makes 2 tests fail:
- test/hotspot/jtreg/serviceability/sa/ClhsdbJhisto.java
- test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java

we filter them for Shenandoah now. I'm wondering: how do you get past
those with ZGC?

See:
http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008466.html 



(Note to Leonid and tests reviewers: I'll add those related filters in
next round).

Vladimir: Roland integrated a bunch of changes to make loop* code look
better. I can tell that we're not done with C2 yet. Can you look over
the code and see what is ok, and especially what is not ok, so that we
can focus our efforts on the relevant parts?

Updated set of webrevs:
http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/01/

Thanks,
Roman



Hi,

This is the first round of changes for including Shenandoah GC into
mainline.
I divided the review into parts that roughly correspond to the 
mailing lists

that would normally review it, and I divided it into 'shared' code
changes and
'shenandoah' code changes (actually, mostly additions). The intend is to
eventually
push them as single 'combined' changeset, once reviewed.

JEP:
   https://openjdk.java.net/jeps/189
Bug entry:

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

Webrevs:
   http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/

For those who want to see the full change, have a look at the
shenandoah-complete
 


directory,
it contains the full combined webrev. Alternatively, there is the file
shenandoah-master.patch
, 


which is what I intend to commit (and which should be equivalent to the
'shenandoah-complete' webrev).

Sections to review (at this point) are the following:
  *) shenandoah-gc
 


 - Actual Shenandoah implementation, almost completely residing in
gc/shenandoah

  *) shared-gc

 - This is mostly boilerplate that is common to any GC
 - referenceProcessor.cpp has a little change to make one assert not
fail (next to CMS and G1)
 - taskqueue.hpp has some small adjustments to enable subclassing

  *) shared-serviceability
 


 - The usual code to support another GC

  *) shared-runtime
 


 - A number of friends declarations to allow Shenandoah iterators to
hook up with,
   e.g. ClassLoaderData, CodeCache, etc
 - Warning and disabling JFR LeakProfiler
 - fieldDescriptor.hpp added is_stable() accessor, for use in
Shenandoah C2 optimizations
 - Locks initialization in mutexLocker.cpp as usual
 - VM operations defines for Shenandoah's VM ops
 - globalDefinitions.hpp added UINT64_FORMAT_HEX_W for use in
Shenandoah's logging
 - The usual macros in macro.hpp

  *) shared-build
 


 - Add shenandoah feature, enabled by default, as agreed with
Vladimir K. beforehand
 - Some flags for shenandoah-enabled compilation to get
SUPPORT_BARRIER_ON_PRIMITIVES
   and SUPPORT_NOT_TO_SPACE_INVARIANT which is required for
Shenandoah's barriers
 - --param inline-unit-growth=1000 settings for 2 shenandoah source
files, which is
   useful to get the whole marking loop inlined (observed 
significant

regression if we
   

Re: RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-29 Thread Per Liden

Hi Roman,

On 11/28/18 11:29 PM, Roman Kennke wrote:
[...]

Per: Disabling the SA part for heapdumping makes 2 tests fail:
- test/hotspot/jtreg/serviceability/sa/ClhsdbJhisto.java
- test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java

we filter them for Shenandoah now. I'm wondering: how do you get past
those with ZGC?


Good catch! These don't work with ZGC either. As far as I can tell, 
these tests are not part of any test tier, so they are never executed in 
our CI pipeline and have gone under the radar.


I'll file a bug and add the appropriate @require tag to exclude ZGC. I'm 
not sure if there's a reason they are excluded from our CI, or if they 
should be added. Leonid, do you know?


Thanks!
/Per


Re: RFR: 8214476: ZGC: Build ZGC by default

2018-11-29 Thread Per Liden

Thanks Magnus!

/Per

On 11/29/18 3:15 PM, Magnus Ihse Bursie wrote:

Looks good to me.

/Magnus


29 nov. 2018 kl. 13:35 skrev Per Liden :

At one point, it was decided that experimental VM features (such as ZGC) should 
be not be built by default. This policy was later changed/abandoned, so we 
should enable building of ZGC by default.

Bug: https://bugs.openjdk.java.net/browse/JDK-8214476
Webrev: http://cr.openjdk.java.net/~pliden/8214476/webrev.0

/Per




Re: RFR: 8214476: ZGC: Build ZGC by default

2018-11-29 Thread Magnus Ihse Bursie
Looks good to me. 

/Magnus

> 29 nov. 2018 kl. 13:35 skrev Per Liden :
> 
> At one point, it was decided that experimental VM features (such as ZGC) 
> should be not be built by default. This policy was later changed/abandoned, 
> so we should enable building of ZGC by default.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8214476
> Webrev: http://cr.openjdk.java.net/~pliden/8214476/webrev.0
> 
> /Per



Re: RFR: 8212794 IBM-964 and IBM-29626C are required for AIX default charset

2018-11-29 Thread Ichiroh Takiguchi

Hello Alan & Magnus.

Sorry for you confusion.
I did many copy actions and rename actions.
So you may see, I added unexpected code into non AIX platform.

I think I should not put 2 kind of modification.

For this bug id, I'll handle IBM964 and IBM33722.
(also SimpleEUCEncoder.java is required)

I'll submit code review again.

Additionally, I'll touch
make/data/charsetmapping/charsets
make/data/charsetmapping/stdcs-aix

I'll not touch
make/jdk/src/classes/build/tools/charsetmapping/Main.java
make/jdk/src/classes/build/tools/charsetmapping/SRC.java

My build machine is not so fast, after test is done.
I'll post new code.

Thanks,
Ichiroh Takiguchi

On 2018-11-28 19:10, Magnus Ihse Bursie wrote:

On 2018-11-28 10:36, Alan Bateman wrote:

On 28/11/2018 09:28, Magnus Ihse Bursie wrote:
I'm quite unsatisfied with the current handling of character sets in 
the build in general. :-( I'd really like to modernize it. I have a, 
slightly fuzzy, laundry list of things I want to fix from a build 
perspective, but I'm not sure of what "external" requirements are 
coming from AIX and the general core-libs agenda regarding character 
sets in general.


I think there is a good opportunity to solve many problems at the 
same time here, as long as everyone agrees on what is the preferred 
outcome.
The support in the build to configure the charsets to include in 
java.base on each platform has been working well. Charsets that aren't 
in java.base go into the jdk.charsets service provider module and that 
has been working well too. From the result point of view, perhaps, but 
definitely not from the build perspective. ;-) But yes, I understand 
this is functionality that should be kept.
One thing that we lack is some way to add charsets for specific 
platforms and this comes up with the IBM patches where they are 
looking to adding several additional IBM charsets. One starting point 
that we've touched on in several threads here is dropping the EBCDIC 
charsets from the main stream builds. Going there will need build 
support.

So build support for trivially adding specific charsets to specific
platforms? Both to java.base (for AIX) and jdk.charsets, I presume,
then?

Can you expand on the issue of dropping ebcdic? What's the problem
that needs build support?

/Magnus



-Alan




Re: RFR: 8214476: ZGC: Build ZGC by default

2018-11-29 Thread Per Liden

Thanks for reviewing, Roman!

/Per

On 11/29/18 2:16 PM, Roman Kennke wrote:

Yes! Great! Patch looks good!

Roman



At one point, it was decided that experimental VM features (such as ZGC)
should be not be built by default. This policy was later
changed/abandoned, so we should enable building of ZGC by default.

Bug: https://bugs.openjdk.java.net/browse/JDK-8214476
Webrev: http://cr.openjdk.java.net/~pliden/8214476/webrev.0

/Per




Re: RFR: 8214476: ZGC: Build ZGC by default

2018-11-29 Thread Roman Kennke
Yes! Great! Patch looks good!

Roman


> At one point, it was decided that experimental VM features (such as ZGC)
> should be not be built by default. This policy was later
> changed/abandoned, so we should enable building of ZGC by default.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8214476
> Webrev: http://cr.openjdk.java.net/~pliden/8214476/webrev.0
> 
> /Per



Re: RFR: 8214476: ZGC: Build ZGC by default

2018-11-29 Thread Aleksey Shipilev
Also, if that's not already the plan, please consider backporting it to jdk11u.
This would help downstream builders (like us) not to miss ZGC there (like we 
did).

-Aleksey

On 11/29/18 1:40 PM, Per Liden wrote:
> Thanks for reviewing, Aleksey.
> 
> cheers,
> Per
> 
> On 11/29/18 1:38 PM, Aleksey Shipilev wrote:
>> On 11/29/18 1:35 PM, Per Liden wrote:
>>> At one point, it was decided that experimental VM features (such as ZGC) 
>>> should be not be built by
>>> default. This policy was later changed/abandoned, so we should enable 
>>> building of ZGC by default.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8214476
>>> Webrev: http://cr.openjdk.java.net/~pliden/8214476/webrev.0
>>
>> Finally! Looks good!
>>
>> -Aleksey
>>
>>




Re: RFR: 8214476: ZGC: Build ZGC by default

2018-11-29 Thread Per Liden

Thanks for reviewing, Aleksey.

cheers,
Per

On 11/29/18 1:38 PM, Aleksey Shipilev wrote:

On 11/29/18 1:35 PM, Per Liden wrote:

At one point, it was decided that experimental VM features (such as ZGC) should 
be not be built by
default. This policy was later changed/abandoned, so we should enable building 
of ZGC by default.

Bug: https://bugs.openjdk.java.net/browse/JDK-8214476
Webrev: http://cr.openjdk.java.net/~pliden/8214476/webrev.0


Finally! Looks good!

-Aleksey




Re: RFR: 8214476: ZGC: Build ZGC by default

2018-11-29 Thread Aleksey Shipilev
On 11/29/18 1:35 PM, Per Liden wrote:
> At one point, it was decided that experimental VM features (such as ZGC) 
> should be not be built by
> default. This policy was later changed/abandoned, so we should enable 
> building of ZGC by default.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8214476
> Webrev: http://cr.openjdk.java.net/~pliden/8214476/webrev.0

Finally! Looks good!

-Aleksey




Re: [PATCH v3] 8214332: Add a flag for overriding default JNI library search path

2018-11-29 Thread Jakub Vaněk
Hi David,

Thanks!

Jakub


On 2018-11-29 at 13:20 +1000, David Holmes wrote:
> Sponsoring. :)
> 
> Thanks,
> David



RFR: 8214476: ZGC: Build ZGC by default

2018-11-29 Thread Per Liden
At one point, it was decided that experimental VM features (such as ZGC) 
should be not be built by default. This policy was later 
changed/abandoned, so we should enable building of ZGC by default.


Bug: https://bugs.openjdk.java.net/browse/JDK-8214476
Webrev: http://cr.openjdk.java.net/~pliden/8214476/webrev.0

/Per


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

2018-11-29 Thread Volker Simonis
On Thu, Nov 29, 2018 at 12:20 PM Magnus Ihse Bursie
 wrote:
>
> On 2018-11-27 16:33, 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
> Looks good to me. I am sorry for the mess I caused by optimisically
> trying to fix things on a platform I could not compile on... :(
>

Thanks for the review and don't worry! We really appreciate your
continued help. It's really us who should have tested and spotted the
problems earlier :)

Regards,
Volker

> This also reminds me that the visibility flags *really* should move into
> configure/spec, not be sprinkled like this in the make files.
>
> /Magnus
> >
> > 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). 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
>


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

2018-11-29 Thread Magnus Ihse Bursie

On 2018-11-27 16:33, 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
Looks good to me. I am sorry for the mess I caused by optimisically 
trying to fix things on a platform I could not compile on... :(


This also reminds me that the visibility flags *really* should move into 
configure/spec, not be sprinkled like this in the make files.


/Magnus


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). 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




Re: RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-29 Thread Roman Kennke
Thanks, Per!

I will do this. Check out round 3 when it arrives. ;-)

Roman

Am 29.11.18 um 12:07 schrieb Per Liden:
> Small comment on the build changes. Instead of checking for zero variant
> here:
> 
> +  # Only enable Shenandoah on supported arches
> +  AC_MSG_CHECKING([if shenandoah can be built])
> +  if HOTSPOT_CHECK_JVM_VARIANT(zero); then
> +    DISABLED_JVM_FEATURES="$DISABLED_JVM_FEATURES shenandoahgc"
> +    AC_MSG_RESULT([no, this JVM variant not supported])
> +  else
> 
> I think you should just add shanandoahgc to the existing check, here:
> 
>   # Disable unsupported GCs for Zero
>   if HOTSPOT_CHECK_JVM_VARIANT(zero); then
>     DISABLED_JVM_FEATURES="$DISABLED_JVM_FEATURES epsilongc g1gc zgc"
>   fi
> 
> cheers,
> Per
> 
> On 11/26/18 11:47 PM, Erik Joelsson wrote:
>> Build changes look ok to me.
>>
>> /Erik
>>
>> On 2018-11-26 13:39, Roman Kennke wrote:
>>> Hi,
>>>
>>> This is the first round of changes for including Shenandoah GC into
>>> mainline.
>>> I divided the review into parts that roughly correspond to the
>>> mailing lists
>>> that would normally review it, and I divided it into 'shared' code
>>> changes and
>>> 'shenandoah' code changes (actually, mostly additions). The intend is to
>>> eventually
>>> push them as single 'combined' changeset, once reviewed.
>>>
>>> JEP:
>>>    https://openjdk.java.net/jeps/189
>>> Bug entry:
>>>
>>>   https://bugs.openjdk.java.net/browse/JDK-8214259
>>>
>>> Webrevs:
>>>    http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/
>>>
>>> For those who want to see the full change, have a look at the
>>> shenandoah-complete
>>> 
>>>
>>> directory,
>>> it contains the full combined webrev. Alternatively, there is the file
>>> shenandoah-master.patch
>>> ,
>>>
>>> which is what I intend to commit (and which should be equivalent to the
>>> 'shenandoah-complete' webrev).
>>>
>>> Sections to review (at this point) are the following:
>>>   *) shenandoah-gc
>>> 
>>>
>>>  - Actual Shenandoah implementation, almost completely residing in
>>> gc/shenandoah
>>>
>>>   *) shared-gc
>>> 
>>>  - This is mostly boilerplate that is common to any GC
>>>  - referenceProcessor.cpp has a little change to make one assert not
>>> fail (next to CMS and G1)
>>>  - taskqueue.hpp has some small adjustments to enable subclassing
>>>
>>>   *) shared-serviceability
>>> 
>>>
>>>  - The usual code to support another GC
>>>
>>>   *) shared-runtime
>>> 
>>>
>>>  - A number of friends declarations to allow Shenandoah iterators to
>>> hook up with,
>>>    e.g. ClassLoaderData, CodeCache, etc
>>>  - Warning and disabling JFR LeakProfiler
>>>  - fieldDescriptor.hpp added is_stable() accessor, for use in
>>> Shenandoah C2 optimizations
>>>  - Locks initialization in mutexLocker.cpp as usual
>>>  - VM operations defines for Shenandoah's VM ops
>>>  - globalDefinitions.hpp added UINT64_FORMAT_HEX_W for use in
>>> Shenandoah's logging
>>>  - The usual macros in macro.hpp
>>>
>>>   *) shared-build
>>> 
>>>
>>>  - Add shenandoah feature, enabled by default, as agreed with
>>> Vladimir K. beforehand
>>>  - Some flags for shenandoah-enabled compilation to get
>>> SUPPORT_BARRIER_ON_PRIMITIVES
>>>    and SUPPORT_NOT_TO_SPACE_INVARIANT which is required for
>>> Shenandoah's barriers
>>>  - --param inline-unit-growth=1000 settings for 2 shenandoah source
>>> files, which is
>>>    useful to get the whole marking loop inlined (observed
>>> significant
>>> regression if we
>>>    don't)
>>>
>>>   *) shared-tests
>>> 
>>>
>>>  - Test infrastructure to support Shenandoah
>>>  - Shenandoah test groups
>>>  - Exclude Shenandoah in various tests that can be run with
>>> selected GC
>>>  - Enable/add configure for Shenandoah for tests that make sense to
>>> run with it
>>>
>>>   *) shenandoah-tests
>>> 
>>>
>>>  - Shenandoah specific tests, most reside in gc/shenandoah
>>> subdirectory
>>>  - A couple of tests configurations have been added, e.g.
>>> TestGCBasherWithShenandoah.java
>>>
>>> I intentionally left out shared-compiler for now, because we have some
>>> work left to do
>>> there, but if you click around you'll find the patch anyway, in case you
>>> want to take
>>> a peek at it.
>>>
>>> We have regular 

Re: RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-29 Thread Roman Kennke
Thanks, Magnus,

this makes sense. I'll incorporate that simplification.

Thanks,
Roman

Am 29.11.18 um 11:53 schrieb Magnus Ihse Bursie:
> On 2018-11-26 23:47, Erik Joelsson wrote:
> 
>> Build changes look ok to me.
> I agree, but I'd like to point out a (possible) style improvement. In
> hotspot.m4,
> 
> if test "x$OPENJDK_TARGET_CPU" = "xx86_64" || test
> "x$OPENJDK_TARGET_CPU" = "xaarch64" || test "x$OPENJDK_TARGET_CPU" ==
> "xx86"; then
> 
> can be more succinctly expressed as
> 
> if test "x$OPENJDK_TARGET_CPU_ARCH" = "xx86" || test
> "x$OPENJDK_TARGET_CPU" = "xaarch64" ; then
> 
> 
> But it's a matter of preference, if you do not want to change. Otherwise
> it looks good!
> 
> /Magnus
>>
>> /Erik
>>
>> On 2018-11-26 13:39, Roman Kennke wrote:
>>> Hi,
>>>
>>> This is the first round of changes for including Shenandoah GC into
>>> mainline.
>>> I divided the review into parts that roughly correspond to the
>>> mailing lists
>>> that would normally review it, and I divided it into 'shared' code
>>> changes and
>>> 'shenandoah' code changes (actually, mostly additions). The intend is to
>>> eventually
>>> push them as single 'combined' changeset, once reviewed.
>>>
>>> JEP:
>>>    https://openjdk.java.net/jeps/189
>>> Bug entry:
>>>
>>>   https://bugs.openjdk.java.net/browse/JDK-8214259
>>>
>>> Webrevs:
>>>    http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/
>>>
>>> For those who want to see the full change, have a look at the
>>> shenandoah-complete
>>> 
>>>
>>> directory,
>>> it contains the full combined webrev. Alternatively, there is the file
>>> shenandoah-master.patch
>>> ,
>>>
>>> which is what I intend to commit (and which should be equivalent to the
>>> 'shenandoah-complete' webrev).
>>>
>>> Sections to review (at this point) are the following:
>>>   *) shenandoah-gc
>>> 
>>>
>>>  - Actual Shenandoah implementation, almost completely residing in
>>> gc/shenandoah
>>>
>>>   *) shared-gc
>>> 
>>>  - This is mostly boilerplate that is common to any GC
>>>  - referenceProcessor.cpp has a little change to make one assert not
>>> fail (next to CMS and G1)
>>>  - taskqueue.hpp has some small adjustments to enable subclassing
>>>
>>>   *) shared-serviceability
>>> 
>>>
>>>  - The usual code to support another GC
>>>
>>>   *) shared-runtime
>>> 
>>>
>>>  - A number of friends declarations to allow Shenandoah iterators to
>>> hook up with,
>>>    e.g. ClassLoaderData, CodeCache, etc
>>>  - Warning and disabling JFR LeakProfiler
>>>  - fieldDescriptor.hpp added is_stable() accessor, for use in
>>> Shenandoah C2 optimizations
>>>  - Locks initialization in mutexLocker.cpp as usual
>>>  - VM operations defines for Shenandoah's VM ops
>>>  - globalDefinitions.hpp added UINT64_FORMAT_HEX_W for use in
>>> Shenandoah's logging
>>>  - The usual macros in macro.hpp
>>>
>>>   *) shared-build
>>> 
>>>
>>>  - Add shenandoah feature, enabled by default, as agreed with
>>> Vladimir K. beforehand
>>>  - Some flags for shenandoah-enabled compilation to get
>>> SUPPORT_BARRIER_ON_PRIMITIVES
>>>    and SUPPORT_NOT_TO_SPACE_INVARIANT which is required for
>>> Shenandoah's barriers
>>>  - --param inline-unit-growth=1000 settings for 2 shenandoah source
>>> files, which is
>>>    useful to get the whole marking loop inlined (observed
>>> significant
>>> regression if we
>>>    don't)
>>>
>>>   *) shared-tests
>>> 
>>>
>>>  - Test infrastructure to support Shenandoah
>>>  - Shenandoah test groups
>>>  - Exclude Shenandoah in various tests that can be run with
>>> selected GC
>>>  - Enable/add configure for Shenandoah for tests that make sense to
>>> run with it
>>>
>>>   *) shenandoah-tests
>>> 
>>>
>>>  - Shenandoah specific tests, most reside in gc/shenandoah
>>> subdirectory
>>>  - A couple of tests configurations have been added, e.g.
>>> TestGCBasherWithShenandoah.java
>>>
>>> I intentionally left out shared-compiler for now, because we have some
>>> work left to do
>>> there, but if you click around you'll find the patch anyway, in case you
>>> want to take
>>> a peek at it.
>>>
>>> We have regular builds on:
>>>    - {Linux} x {x86_64, x86_32, armhf, aarch64, ppc64el, s390x}
>>>    - {Windows} x {x86_64},
>>>    - {MacOS X} x 

Re: RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-29 Thread Per Liden
Small comment on the build changes. Instead of checking for zero variant 
here:


+  # Only enable Shenandoah on supported arches
+  AC_MSG_CHECKING([if shenandoah can be built])
+  if HOTSPOT_CHECK_JVM_VARIANT(zero); then
+DISABLED_JVM_FEATURES="$DISABLED_JVM_FEATURES shenandoahgc"
+AC_MSG_RESULT([no, this JVM variant not supported])
+  else

I think you should just add shanandoahgc to the existing check, here:

  # Disable unsupported GCs for Zero
  if HOTSPOT_CHECK_JVM_VARIANT(zero); then
DISABLED_JVM_FEATURES="$DISABLED_JVM_FEATURES epsilongc g1gc zgc"
  fi

cheers,
Per

On 11/26/18 11:47 PM, Erik Joelsson wrote:

Build changes look ok to me.

/Erik

On 2018-11-26 13:39, Roman Kennke wrote:

Hi,

This is the first round of changes for including Shenandoah GC into
mainline.
I divided the review into parts that roughly correspond to the mailing 
lists

that would normally review it, and I divided it into 'shared' code
changes and
'shenandoah' code changes (actually, mostly additions). The intend is to
eventually
push them as single 'combined' changeset, once reviewed.

JEP:
   https://openjdk.java.net/jeps/189
Bug entry:

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

Webrevs:
   http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/

For those who want to see the full change, have a look at the
shenandoah-complete
 


directory,
it contains the full combined webrev. Alternatively, there is the file
shenandoah-master.patch
, 


which is what I intend to commit (and which should be equivalent to the
'shenandoah-complete' webrev).

Sections to review (at this point) are the following:
  *) shenandoah-gc
 


 - Actual Shenandoah implementation, almost completely residing in
gc/shenandoah

  *) shared-gc

 - This is mostly boilerplate that is common to any GC
 - referenceProcessor.cpp has a little change to make one assert not
fail (next to CMS and G1)
 - taskqueue.hpp has some small adjustments to enable subclassing

  *) shared-serviceability
 


 - The usual code to support another GC

  *) shared-runtime
 


 - A number of friends declarations to allow Shenandoah iterators to
hook up with,
   e.g. ClassLoaderData, CodeCache, etc
 - Warning and disabling JFR LeakProfiler
 - fieldDescriptor.hpp added is_stable() accessor, for use in
Shenandoah C2 optimizations
 - Locks initialization in mutexLocker.cpp as usual
 - VM operations defines for Shenandoah's VM ops
 - globalDefinitions.hpp added UINT64_FORMAT_HEX_W for use in
Shenandoah's logging
 - The usual macros in macro.hpp

  *) shared-build
 


 - Add shenandoah feature, enabled by default, as agreed with
Vladimir K. beforehand
 - Some flags for shenandoah-enabled compilation to get
SUPPORT_BARRIER_ON_PRIMITIVES
   and SUPPORT_NOT_TO_SPACE_INVARIANT which is required for
Shenandoah's barriers
 - --param inline-unit-growth=1000 settings for 2 shenandoah source
files, which is
   useful to get the whole marking loop inlined (observed significant
regression if we
   don't)

  *) shared-tests
 


 - Test infrastructure to support Shenandoah
 - Shenandoah test groups
 - Exclude Shenandoah in various tests that can be run with 
selected GC

 - Enable/add configure for Shenandoah for tests that make sense to
run with it

  *) shenandoah-tests
 

 - Shenandoah specific tests, most reside in gc/shenandoah 
subdirectory

 - A couple of tests configurations have been added, e.g.
TestGCBasherWithShenandoah.java

I intentionally left out shared-compiler for now, because we have some
work left to do
there, but if you click around you'll find the patch anyway, in case you
want to take
a peek at it.

We have regular builds on:
   - {Linux} x {x86_64, x86_32, armhf, aarch64, ppc64el, s390x}
   - {Windows} x {x86_64},
   - {MacOS X} x {x86_64}

This also routinely passes:
   - the new Shenandoah tests
   - jcstress with/without aggressive Shenandoah verification
   - specjvm2008 with/without aggressive Shenandoah verification


I'd like to thank my collegues at Red Hat: Christine Flood, she deserves
the credit for being the original inventor of Shenandoah, Aleksey
Shiplëv, Roland Westrelin & Zhengyu Gu for their countless
contributions, everybody else in Red Hat's 

Re: [12] Review Request: 8212680 (JDK12b14/Solaris-sparc) SplashScreen::getSplashScreen call fails with ULE: "libsplashscreen.so: ld.so.1: java: fatal: libz.so.1: open failed: No such file o

2018-11-29 Thread Magnus Ihse Bursie




On 2018-11-29 00:11, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk 12.

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

Hi Sergey,

I understand the change in png.h and why it fixes the issue. But I, like 
Erik, am a bit weary about changing upstream code. I think you should 
add this patch of pnh.h, or at least a note of it, in UPDATING.txt.


However, I do not understand the reasons for commenting out the define 
for PNG_ADLER32_SUPPORTED in pnglibconf.h, and adding a 
-DPNG_ADLER32_SUPPORTED in Awt2dLibraries.gmk. What's the purpose of 
this? Will not the same effect be achieved by just keeping these two 
files as-is?


/Magnus


On Solaris we faced the bug which was fixed in macOS already:
  https://bugs.openjdk.java.net/browse/JDK-8196803

The problem is that there is a call to "inflateValidate" function in 
pngrutil.c[1], guarded by a preprocessor check of ZLIB_VERNUM being 
high enough and by the "PNG_IGNORE_ADLER32". If we compile this call 
in and link to the newer version of zlib, we will get link errors if 
the code is executed on an older Mac/Solaris/ with an older version of 
zlib.


The bug can be reproduced on "old" Solaris 11.3, which was not updated 
for a while(since 2015).


We can fix it by requiring some "OS Patches and Package Updates", but 
since it was
reproduced on macOS, and potentially can occur on other platforms, I 
have decided
to fix it in the code. The new property is introduced to the libpng 
"PNG_ADLER32_SUPPORTED",
which control the usage of "PNG_IGNORE_ADLER32" and as a result 
control the call to "inflateValidate"[1].
This new property is set in the makefile when we build "bundled" 
versions of libpng+zlib only.


This was reported upstream, and the future version of libpng may have 
some similar solution.


[1] 
http://hg.openjdk.java.net/jdk/jdk/file/396dfb0e8ba5/src/java.desktop/share/native/libsplashscreen/libpng/pngrutil.c#l457







Re: RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-29 Thread Magnus Ihse Bursie

On 2018-11-26 23:47, Erik Joelsson wrote:


Build changes look ok to me.
I agree, but I'd like to point out a (possible) style improvement. In 
hotspot.m4,


if test "x$OPENJDK_TARGET_CPU" = "xx86_64" || test 
"x$OPENJDK_TARGET_CPU" = "xaarch64" || test "x$OPENJDK_TARGET_CPU" == 
"xx86"; then


can be more succinctly expressed as

if test "x$OPENJDK_TARGET_CPU_ARCH" = "xx86" || test 
"x$OPENJDK_TARGET_CPU" = "xaarch64" ; then



But it's a matter of preference, if you do not want to change. Otherwise 
it looks good!


/Magnus


/Erik

On 2018-11-26 13:39, Roman Kennke wrote:

Hi,

This is the first round of changes for including Shenandoah GC into
mainline.
I divided the review into parts that roughly correspond to the 
mailing lists

that would normally review it, and I divided it into 'shared' code
changes and
'shenandoah' code changes (actually, mostly additions). The intend is to
eventually
push them as single 'combined' changeset, once reviewed.

JEP:
   https://openjdk.java.net/jeps/189
Bug entry:

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

Webrevs:
   http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/

For those who want to see the full change, have a look at the
shenandoah-complete
 


directory,
it contains the full combined webrev. Alternatively, there is the file
shenandoah-master.patch
, 


which is what I intend to commit (and which should be equivalent to the
'shenandoah-complete' webrev).

Sections to review (at this point) are the following:
  *) shenandoah-gc
 


 - Actual Shenandoah implementation, almost completely residing in
gc/shenandoah

  *) shared-gc

 - This is mostly boilerplate that is common to any GC
 - referenceProcessor.cpp has a little change to make one assert not
fail (next to CMS and G1)
 - taskqueue.hpp has some small adjustments to enable subclassing

  *) shared-serviceability
 


 - The usual code to support another GC

  *) shared-runtime
 


 - A number of friends declarations to allow Shenandoah iterators to
hook up with,
   e.g. ClassLoaderData, CodeCache, etc
 - Warning and disabling JFR LeakProfiler
 - fieldDescriptor.hpp added is_stable() accessor, for use in
Shenandoah C2 optimizations
 - Locks initialization in mutexLocker.cpp as usual
 - VM operations defines for Shenandoah's VM ops
 - globalDefinitions.hpp added UINT64_FORMAT_HEX_W for use in
Shenandoah's logging
 - The usual macros in macro.hpp

  *) shared-build
 


 - Add shenandoah feature, enabled by default, as agreed with
Vladimir K. beforehand
 - Some flags for shenandoah-enabled compilation to get
SUPPORT_BARRIER_ON_PRIMITIVES
   and SUPPORT_NOT_TO_SPACE_INVARIANT which is required for
Shenandoah's barriers
 - --param inline-unit-growth=1000 settings for 2 shenandoah source
files, which is
   useful to get the whole marking loop inlined (observed 
significant

regression if we
   don't)

  *) shared-tests
 


 - Test infrastructure to support Shenandoah
 - Shenandoah test groups
 - Exclude Shenandoah in various tests that can be run with 
selected GC

 - Enable/add configure for Shenandoah for tests that make sense to
run with it

  *) shenandoah-tests
 

 - Shenandoah specific tests, most reside in gc/shenandoah 
subdirectory

 - A couple of tests configurations have been added, e.g.
TestGCBasherWithShenandoah.java

I intentionally left out shared-compiler for now, because we have some
work left to do
there, but if you click around you'll find the patch anyway, in case you
want to take
a peek at it.

We have regular builds on:
   - {Linux} x {x86_64, x86_32, armhf, aarch64, ppc64el, s390x}
   - {Windows} x {x86_64},
   - {MacOS X} x {x86_64}

This also routinely passes:
   - the new Shenandoah tests
   - jcstress with/without aggressive Shenandoah verification
   - specjvm2008 with/without aggressive Shenandoah verification


I'd like to thank my collegues at Red Hat: Christine Flood, she deserves
the credit for being the original inventor of Shenandoah, Aleksey
Shiplëv, Roland Westrelin & Zhengyu Gu for their countless
contributions, everybody else in Red Hat's OpenJDK team for testing,
advice and support, my collegues in Oracle's GC, runtime and compiler
teams for tirelessly helping with and 

Re: RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-29 Thread Roman Kennke
Hi Jini,

> The SA tests which iterate over the heap (ClhsdbJhisto.java,
> TestHeapDumpFor*.java) fail if ZGC is passed as an option to these also
> (Am planning on fixing these). It might be better to avoid invoking
> those tests for Shenandoah now with an @requires tag.

Thanks for clarifying. Yeah, we already have a fix in shenandoah/jdk and
it will appear in next round of reviews.

Thanks,
Roman

> Thank you,
> Jini
> 
> On 11/29/2018 3:59 AM, Roman Kennke wrote:
>> Alright, we fixed:
>> - The minor issues that Kim reported in shared-gc
>> - A lot of fixes in shared-tests according to Leonid's review
>> - Disabled SA heapdumping similar to ZGC as Per suggested
>>
>> Some notes:
>> Leonid:  test/hotspot/jtreg/gc/TestFullGCCount.java was actually
>> correct. The @requires there means to exclude runs with both CMS and
>> ExplicitGCInvokesConcurrent at the same time, because that would be
>> (expectedly) failing. It can run CMS, default GC and any other GC just
>> fine. Adding the same clause for Shenandoah means the same, and filters
>> the combination (+UseShenandoahGC)+(+ExplicitGCInvokesConcurrent). I
>> made the condition a bit clearer by avoiding triple-negation.
>>
>> See:
>> http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008457.html
>>
>>
>> Per: Disabling the SA part for heapdumping makes 2 tests fail:
>> - test/hotspot/jtreg/serviceability/sa/ClhsdbJhisto.java
>> - test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java
>>
>> we filter them for Shenandoah now. I'm wondering: how do you get past
>> those with ZGC?
>>
>> See:
>> http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008466.html
>>
>>
>> (Note to Leonid and tests reviewers: I'll add those related filters in
>> next round).
>>
>> Vladimir: Roland integrated a bunch of changes to make loop* code look
>> better. I can tell that we're not done with C2 yet. Can you look over
>> the code and see what is ok, and especially what is not ok, so that we
>> can focus our efforts on the relevant parts?
>>
>> Updated set of webrevs:
>> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/01/
>>
>> Thanks,
>> Roman
>>
>>
>>> Hi,
>>>
>>> This is the first round of changes for including Shenandoah GC into
>>> mainline.
>>> I divided the review into parts that roughly correspond to the
>>> mailing lists
>>> that would normally review it, and I divided it into 'shared' code
>>> changes and
>>> 'shenandoah' code changes (actually, mostly additions). The intend is to
>>> eventually
>>> push them as single 'combined' changeset, once reviewed.
>>>
>>> JEP:
>>>    https://openjdk.java.net/jeps/189
>>> Bug entry:
>>>
>>>   https://bugs.openjdk.java.net/browse/JDK-8214259
>>>
>>> Webrevs:
>>>    http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/
>>>
>>> For those who want to see the full change, have a look at the
>>> shenandoah-complete
>>> 
>>>
>>> directory,
>>> it contains the full combined webrev. Alternatively, there is the file
>>> shenandoah-master.patch
>>> ,
>>>
>>> which is what I intend to commit (and which should be equivalent to the
>>> 'shenandoah-complete' webrev).
>>>
>>> Sections to review (at this point) are the following:
>>>   *) shenandoah-gc
>>> 
>>>
>>>  - Actual Shenandoah implementation, almost completely residing in
>>> gc/shenandoah
>>>
>>>   *) shared-gc
>>> 
>>>  - This is mostly boilerplate that is common to any GC
>>>  - referenceProcessor.cpp has a little change to make one assert not
>>> fail (next to CMS and G1)
>>>  - taskqueue.hpp has some small adjustments to enable subclassing
>>>
>>>   *) shared-serviceability
>>> 
>>>
>>>  - The usual code to support another GC
>>>
>>>   *) shared-runtime
>>> 
>>>
>>>  - A number of friends declarations to allow Shenandoah iterators to
>>> hook up with,
>>>    e.g. ClassLoaderData, CodeCache, etc
>>>  - Warning and disabling JFR LeakProfiler
>>>  - fieldDescriptor.hpp added is_stable() accessor, for use in
>>> Shenandoah C2 optimizations
>>>  - Locks initialization in mutexLocker.cpp as usual
>>>  - VM operations defines for Shenandoah's VM ops
>>>  - globalDefinitions.hpp added UINT64_FORMAT_HEX_W for use in
>>> Shenandoah's logging
>>>  - The usual macros in macro.hpp
>>>
>>>   *) shared-build
>>> 
>>>
>>>  - Add shenandoah feature, enabled by default, as agreed with
>>> Vladimir K. beforehand
>>>  - Some flags for 

Re: RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-29 Thread Roman Kennke
Hi Vladimir,

thanks for reviewing!

> .ad files. I am thinking may be we should put new code in separate
> _shenandoah.ad files and merge them only if Shenandoah is included
> in built [1]. I don't like #ifdefs - you still generate this mach nodes
> even without Shenandoah.

We're looking into improving the situation.

> debug_final_field_at() and debug_stable_field_at() are not used - could
> be excluded from changes.

Those are used from shenandoahBarrierSetC2.cpp and related code in in
the shenandoah-gc or shenandoah-complete webrevs.

> loopnode.cpp Sometimes you add #if INCLUDE_SHENANDOAHGC and sometimes
> don't. Be consistent. I don't know why you need #ifdef if such nodes
> should be generated without Shenandoah

We'll look into improving that.

> compile.cpp - no need to include shenandoahBarrierSetC2.hpp

Right. Will fix it.

> type.hpp - cast_to_nonconst() are not used.

As above, this is used from shenandoahBarrierSetC2.cpp

> phasetype.hpp, subnode.cpp - don't add files with only cleanup changes.

right. Those are leftovers and I'll remove them.

> I wish loopnode.cpp and loopopts.cpp changes were more clean.

We'll see what we can do. Some of those could possibly be turned into
some sort of GC interface, but 1. it would be very Shenandoah-specific
and 2. require significant duplication of code.

> I like the idea where you expose only one ShenandoahBarrier node to C2
> and hide all subnodes behind it. I really don't want to see a lot of
> Shenandoah*Node new classes in C2 shared code.

I understand that. We have ShenandoahBarrierNode superclass and a bunch
of ShenandoahReadBarrier, WriteBarrier and some other nodes 'behind'
this. The crux is probably all the various ShenandoahCompareAndSwap etc
variants, which need to be subclasses of their
non-Shenandoah-counterparts, and I don't see how to change that.

Thanks for reviewing and helping!
Roman

> 
> [1]
> http://hg.openjdk.java.net/jdk/jdk/file/157c1130b46e/make/hotspot/gensrc/GensrcAdlc.gmk#l122
> 
> 
> On 11/28/18 2:29 PM, Roman Kennke wrote:
>> Alright, we fixed:
>> - The minor issues that Kim reported in shared-gc
>> - A lot of fixes in shared-tests according to Leonid's review
>> - Disabled SA heapdumping similar to ZGC as Per suggested
>>
>> Some notes:
>> Leonid:  test/hotspot/jtreg/gc/TestFullGCCount.java was actually
>> correct. The @requires there means to exclude runs with both CMS and
>> ExplicitGCInvokesConcurrent at the same time, because that would be
>> (expectedly) failing. It can run CMS, default GC and any other GC just
>> fine. Adding the same clause for Shenandoah means the same, and filters
>> the combination (+UseShenandoahGC)+(+ExplicitGCInvokesConcurrent). I
>> made the condition a bit clearer by avoiding triple-negation.
>>
>> See:
>> http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008457.html
>>
>>
>> Per: Disabling the SA part for heapdumping makes 2 tests fail:
>> - test/hotspot/jtreg/serviceability/sa/ClhsdbJhisto.java
>> - test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java
>>
>> we filter them for Shenandoah now. I'm wondering: how do you get past
>> those with ZGC?
>>
>> See:
>> http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008466.html
>>
>>
>> (Note to Leonid and tests reviewers: I'll add those related filters in
>> next round).
>>
>> Vladimir: Roland integrated a bunch of changes to make loop* code look
>> better. I can tell that we're not done with C2 yet. Can you look over
>> the code and see what is ok, and especially what is not ok, so that we
>> can focus our efforts on the relevant parts?
>>
>> Updated set of webrevs:
>> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/01/
>>
>> Thanks,
>> Roman
>>
>>
>>> Hi,
>>>
>>> This is the first round of changes for including Shenandoah GC into
>>> mainline.
>>> I divided the review into parts that roughly correspond to the
>>> mailing lists
>>> that would normally review it, and I divided it into 'shared' code
>>> changes and
>>> 'shenandoah' code changes (actually, mostly additions). The intend is to
>>> eventually
>>> push them as single 'combined' changeset, once reviewed.
>>>
>>> JEP:
>>>    https://openjdk.java.net/jeps/189
>>> Bug entry:
>>>
>>>   https://bugs.openjdk.java.net/browse/JDK-8214259
>>>
>>> Webrevs:
>>>    http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/
>>>
>>> For those who want to see the full change, have a look at the
>>> shenandoah-complete
>>> 
>>>
>>> directory,
>>> it contains the full combined webrev. Alternatively, there is the file
>>> shenandoah-master.patch
>>> ,
>>>
>>> which is what I intend to commit (and which should be equivalent to the
>>> 'shenandoah-complete' webrev).
>>>
>>> Sections to review (at this point) are the following:
>>>   *) shenandoah-gc
>>>