JDK 9 RFR of JDK-4851777 Add BigDecimal sqrt method

[joe darcy]

Hello,

Please review the addition of BigDecimal.sqrt:

 JDK-4851777 Add BigDecimal sqrt method

http://cr.openjdk.java.net/~darcy/4851777.5/

Thanks,

-Joe

Re: Review Request: 8157391: jdeps left JarFile open

[Alan Bateman]

On 20/05/2016 04:55, Mandy Chung wrote:

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157391/webrev.00/index.html

tools/jdeps/modules/GenModuleInfo.java
tools/jdeps/modules/TransitiveDeps.java
tools/jdeps/modules/InverseDeps.java

These tests still failed on windows after JDK-8152502.

Now I have the jtreg logs showing that JAR files can’t be deleted although I 
still can’t reproduce them locally and on JPRT (will find out what’s the 
difference tomorrow).

This looks okay to me. One thing you could do in the test is have 
JdepsUtil.Command be Closeable so that the tests can use 
try-with-resources and avoid failing with a file open.


-Alan.

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

[Alan Bateman]

On 20/05/2016 07:25, Peter Levart wrote:

Hi Martin,


On 05/20/2016 12:04 AM, Martin Buchholz wrote:

Hi Peter,

I would make e.g. the change to Resource an independent change.


I agree. It's just that if we measure the impact of ZipFile changes to 
class loading, for example, we should measure them with this change 
included so that we get the optimal caching strategy implemented. 
After we settle on the ZipFile changes, we can strip-off this 
independent change and propose it separately.
I agree too. Also it would be good to keep the follow on changes to code 
like BuiltinClassLoader to a minimum as we have quite a few changes come 
that will replace sections of this.


-Alan

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

[Peter Levart]

Hi Martin,


On 05/20/2016 12:04 AM, Martin Buchholz wrote:

Hi Peter,

I would make e.g. the change to Resource an independent change.


I agree. It's just that if we measure the impact of ZipFile changes to 
class loading, for example, we should measure them with this change 
included so that we get the optimal caching strategy implemented. After 
we settle on the ZipFile changes, we can strip-off this independent 
change and propose it separately.




Below you declare that you throw IOException, but you actually swallow
it, which is not good.

  /**
+ * Closes cached input stream used for getBytes / getByteBuffer
+ * @throws IOException
+ */
+@Override
+public synchronized void close() {


Yes, I forgot to remove the @throws. I think it is OK to swallow the 
IOException here, as what is currently done, when the input stream(s) 
are finalized, such IOException(s) are ignored (if they ever get 
thrown). It's also nothing we can do here since this exception would get 
thrown after the class has already been defined and we can't propagate 
the exception or unsuccessful class loading result to the caller after 
successfully defining the class.



Regards, Peter

Re: RFR [9] 8157154: jmod jlink properties file need copyright header

[Alan Bateman]

On 19/05/2016 22:56, Chris Hegarty wrote:

Yes of course. I'll add the year range before pushing.

-Chris


Looks fine. There is someone on jdk9-dev with translations of this file 
where the missing headers came up too.


-Alan.

Re: JDK 9 RFR of JDK-8157211: Mark tools/launcher/FXLauncherTest.java as intermittently failing

[Hamlin Li]

Would you please help to review the code change?

Thank you
-Hamlin

On 2016/5/18 12:52, Hamlin Li wrote:

tools/launcher/FXLauncherTest.java
This test is known to fail intermittently (JDK-8130392 
), it should be 
marked accordingly with@key intermittentjtreg tag in the test file.


bug: https://bugs.openjdk.java.net/browse/JDK-8157211
webrev: http://cr.openjdk.java.net/~mli/8157211/webrev.00/

Thank you
-Hamlin

--- a/test/tools/launcher/FXLauncherTest.javaTue May 17 19:20:05 
2016 -0700
+++ b/test/tools/launcher/FXLauncherTest.javaTue May 17 21:44:18 
2016 -0700

@@ -29,6 +29,7 @@
  * jfx app class, a main-class for the manifest, a bogus one and none.
  * All should execute except the incorrect fx app class entries.
  * @run main/othervm FXLauncherTest
+ * @key intermittent
  */
 import java.io.File;
 import java.io.IOException;

Re: [PING] Re: [9] RFR of 8023217: Additional floorDiv/floorMod/multiplyExact methods for java.lang.Math

[joe darcy]

Hi Brian,

Looks okay; thanks,

-Joe


On 5/18/2016 12:06 PM, Brian Burkhalter wrote:

Following up on the as yet unresolved thread initiated here:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035468.html

Thanks,

Brian

On Sep 29, 2015, at 5:49 PM, Brian Burkhalter  
wrote:


I revised floorMod(long x, int y) not to check explicitly check for integer 
overflow as it does not look as if this is even possible. I also updated the 
appropriate tests for these versions of the three methods at issue.

In testing I still found discrepancies between the existing implementations and 
the ones suggested earlier in this thread. Therefore those implementations have 
not been used. If it is desired to move to different implementations at a later 
date a separate enhancement issue may be filed.

The updated webrev is here:

http://cr.openjdk.java.net/~bpb/8023217/webrev.01/

Thanks,

Brian

On Sep 29, 2015, at 8:16 AM, Brian Burkhalter  
wrote:


On Sep 29, 2015, at 8:05 AM, Stephen Colebourne  wrote:


I tested the code which was in the original issue description and found some
discrepancies. I’ll need to revisit this to see what happened.

Yes, the code in the issue for floorDiv() fails when the divisor is
negative. The one in my email today works though.

Excellent! I’ll double check it as part of the “fit and finish."

JDK 9 RFR of JDK-8151904: test/java/lang/StackWalker/VerifyStackTrace.java needs to handle update releases

[Hamlin Li]

The test currently hardcodes the version number "9". We should build a 
JDK with a different release number e.g. 9.1 and checks if this test 
handles it properly.
Test run successfully on 9-ea+118, dummybundles(9-ea+255, 9.1-ea+255, 
9.0.1-ea+255, 9.0.0.1-ea+255).


bug: https://bugs.openjdk.java.net/browse/JDK-8151904
webrev: http://cr.openjdk.java.net/~mli/8151904/webrev.00/

Thank you
-Hamlin

Review request 8153042: jdeps should continue to report JDK internal APIs that are removed/renamed in JDK 9

[Mandy Chung]

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153042/webrev.00/index.html

Several JDK internal APIs have been removed JDK 9.  It’d be helpful to continue 
to flag those internal APIs if they are used by existing libraries.  This fix 
is an interim solution.  A longer-term solution may be for jdeps to support 
-release option (something for the future).

Mandy

Re: Review Request: 8157391: jdeps left JarFile open

[Martin Buchholz]

We should have a debugging version of the JDK that fails whenever an
AutoCloseable object is not properly closed in JDK code!

On Thu, May 19, 2016 at 8:55 PM, Mandy Chung  wrote:
> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157391/webrev.00/index.html
>
> tools/jdeps/modules/GenModuleInfo.java
> tools/jdeps/modules/TransitiveDeps.java
> tools/jdeps/modules/InverseDeps.java
>
> These tests still failed on windows after JDK-8152502.
>
> Now I have the jtreg logs showing that JAR files can’t be deleted although I 
> still can’t reproduce them locally and on JPRT (will find out what’s the 
> difference tomorrow).
>
> Mandy

Review Request: 8157391: jdeps left JarFile open

[Mandy Chung]

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157391/webrev.00/index.html

tools/jdeps/modules/GenModuleInfo.java 
tools/jdeps/modules/TransitiveDeps.java 
tools/jdeps/modules/InverseDeps.java 

These tests still failed on windows after JDK-8152502.

Now I have the jtreg logs showing that JAR files can’t be deleted although I 
still can’t reproduce them locally and on JPRT (will find out what’s the 
difference tomorrow).

Mandy

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

[Xueming Shen]

On 5/19/16 5:51 PM, Martin Buchholz wrote:

On Thu, May 19, 2016 at 7:29 AM, Peter Levart  wrote:

So Martin, what do you think?

I studied your code and we are fundamentally in agreement!
... Except for need for periodic cleanup of the cache...
Maybe we should do
CompletableFuture.runAsync(purgeCache,
CompletableFuture.delayedExecutor(1, SECONDS))
as a periodic task while cache is non-empty instead of my strategy of
relying on a weak reference.

Purging a cache on quiescence in the optimal way seems to be a hard problem.
Time to write ObjectPool (as many others have done before us)?

ObjectPool(Supplier factory,  Consumer releaser, maxCacheSize,
keepAliveTime)

Except this time we'll do it right!  With jdk9 machinery!



personally i agree a non-weak-reference based solution is attractive 
here :-)

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

[Martin Buchholz]

On Thu, May 19, 2016 at 7:29 AM, Peter Levart  wrote:
> So Martin, what do you think?

I studied your code and we are fundamentally in agreement!
... Except for need for periodic cleanup of the cache...
Maybe we should do
CompletableFuture.runAsync(purgeCache,
CompletableFuture.delayedExecutor(1, SECONDS))
as a periodic task while cache is non-empty instead of my strategy of
relying on a weak reference.

Purging a cache on quiescence in the optimal way seems to be a hard problem.
Time to write ObjectPool (as many others have done before us)?

ObjectPool(Supplier factory,  Consumer releaser, maxCacheSize,
keepAliveTime)

Except this time we'll do it right!  With jdk9 machinery!

Re: Review request: 8152502: tools/jdeps/modules/GenModuleInfo.java and TransitiveDeps fails on windows

[Jonathan Gibbons]

Looks good to me.

-- Jon

On 05/19/2016 04:50 PM, Mandy Chung wrote:

These jdeps tests fails running on windows “Error. failed clean up files after 
test”.  A bug in the tests and implementation that calls Files.walk without 
try-with-resources.

Webrev at:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8152502/webrev.00/index.html

This fix calls Files::walk with try-with-resources.  I wasn’t quite able to 
reproduce the error on my windows machine.  I’d like to push this fix and 
address any issue, if any.

Mandy

Review request: 8152502: tools/jdeps/modules/GenModuleInfo.java and TransitiveDeps fails on windows

[Mandy Chung]

These jdeps tests fails running on windows “Error. failed clean up files after 
test”.  A bug in the tests and implementation that calls Files.walk without 
try-with-resources.

Webrev at:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8152502/webrev.00/index.html

This fix calls Files::walk with try-with-resources.  I wasn’t quite able to 
reproduce the error on my windows machine.  I’d like to push this fix and 
address any issue, if any.

Mandy

Re: RFR (JAXP) 8139585: Typo: "APIi" instead of "API"

[huizhe wang]

Thanks Mandy!

Joe

On 5/19/2016 3:44 PM, Mandy Chung wrote:

On May 19, 2016, at 3:35 PM, huizhe wang  wrote:

Hi,

Replaced the texts with the titles of the documents they are referring to.

JBS: https://bugs.openjdk.java.net/browse/JDK-8139585
Webrev: http://cr.openjdk.java.net/~joehw/jdk9/8139585/webrev/

+1

Mandy

Re: RFR (JAXP) 8139585: Typo: "APIi" instead of "API"

[Mandy Chung]

> On May 19, 2016, at 3:35 PM, huizhe wang  wrote:
> 
> Hi,
> 
> Replaced the texts with the titles of the documents they are referring to.
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8139585
> Webrev: http://cr.openjdk.java.net/~joehw/jdk9/8139585/webrev/

+1

Mandy

RFR (JAXP) 8139585: Typo: "APIi" instead of "API"

[huizhe wang]

Hi,

Replaced the texts with the titles of the documents they are referring to.

JBS: https://bugs.openjdk.java.net/browse/JDK-8139585
Webrev: http://cr.openjdk.java.net/~joehw/jdk9/8139585/webrev/

Thanks,
Joe

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

[Martin Buchholz]

Hi Peter,

I would make e.g. the change to Resource an independent change.

Below you declare that you throw IOException, but you actually swallow
it, which is not good.

 /**
+ * Closes cached input stream used for getBytes / getByteBuffer
+ * @throws IOException
+ */
+@Override
+public synchronized void close() {

Re: RFR [9] 8157154: jmod jlink properties file need copyright header

[Chris Hegarty]

Yes of course. I'll add the year range before pushing.

-Chris

> On 19 May 2016, at 22:26, Mandy Chung  wrote:
> 
> Should the start year be 2015?  You can fix up before you push.
> 
> Mandy
> 
>> On May 19, 2016, at 2:20 PM, Chris Hegarty  wrote:
>> 
>> Quite trivially, the copyright headers were omitted from these new
>> property files. This review request simply adds the standard header
>> to:
>> 
>> jdk/src/jdk.jlink/share/classes/jdk/tools/jimage/resources/jimage.properties
>> jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties
>> jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties
>> jdk/src/jdk.jlink/share/classes/jdk/tools/jmod/resources/jmod.properties
>> 
>> -Chris.
>> 
> 

Re: RFR [9] 8157154: jmod jlink properties file need copyright header

[Mandy Chung]

Should the start year be 2015?  You can fix up before you push.

Mandy

> On May 19, 2016, at 2:20 PM, Chris Hegarty  wrote:
> 
> Quite trivially, the copyright headers were omitted from these new
> property files. This review request simply adds the standard header
> to:
> 
> jdk/src/jdk.jlink/share/classes/jdk/tools/jimage/resources/jimage.properties
> jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties
> jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties
> jdk/src/jdk.jlink/share/classes/jdk/tools/jmod/resources/jmod.properties
> 
> -Chris.
> 

RFR [9] 8157154: jmod jlink properties file need copyright header

[Chris Hegarty]

Quite trivially, the copyright headers were omitted from these new
property files. This review request simply adds the standard header
to:

jdk/src/jdk.jlink/share/classes/jdk/tools/jimage/resources/jimage.properties
jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties
jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties
jdk/src/jdk.jlink/share/classes/jdk/tools/jmod/resources/jmod.properties

-Chris.


$ hg diff
diff --git 
a/src/jdk.jlink/share/classes/jdk/tools/jimage/resources/jimage.properties 
b/src/jdk.jlink/share/classes/jdk/tools/jimage/resources/jimage.properties
--- a/src/jdk.jlink/share/classes/jdk/tools/jimage/resources/jimage.properties
+++ b/src/jdk.jlink/share/classes/jdk/tools/jimage/resources/jimage.properties
@@ -1,3 +1,28 @@
+#
+# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+#
+# This code is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 2 only, as
+# published by the Free Software Foundation.  Oracle designates this
+# particular file as subject to the "Classpath" exception as provided
+# by Oracle in the LICENSE file that accompanied this code.
+#
+# This code is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+# version 2 for more details (a copy is included in the LICENSE file that
+# accompanied this code).
+#
+# You should have received a copy of the GNU General Public License version
+# 2 along with this work; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+# or visit www.oracle.com if you need additional information or have any
+# questions.
+#
+
 main.usage.summary=\
 Usage: {0}   jimage...\n\
 use --help for a list of possible options
diff --git 
a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties 
b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties
--- a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties
+++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties
@@ -1,3 +1,28 @@
+#
+# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+#
+# This code is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 2 only, as
+# published by the Free Software Foundation.  Oracle designates this
+# particular file as subject to the "Classpath" exception as provided
+# by Oracle in the LICENSE file that accompanied this code.
+#
+# This code is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+# version 2 for more details (a copy is included in the LICENSE file that
+# accompanied this code).
+#
+# You should have received a copy of the GNU General Public License version
+# 2 along with this work; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+# or visit www.oracle.com if you need additional information or have any
+# questions.
+#
+
 main.usage.summary=\
 Usage: {0}  --modulepath  --addmods  --output 
\n\
 use --help for a list of possible options
diff --git 
a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties 
b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties
--- a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties
+++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties
@@ -1,3 +1,28 @@
+#
+# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+#
+# This code is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 2 only, as
+# published by the Free Software Foundation.  Oracle designates this
+# particular file as subject to the "Classpath" exception as provided
+# by Oracle in the LICENSE file that accompanied this code.
+#
+# This code is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+# version 2 for more details (a copy is included in the LICENSE file that
+# accompanied this code).
+#
+# You should have received a copy of the GNU General Public License version
+# 2 al

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

[Martin Buchholz]

Peter and Sherman,

I'm still quixotic fighting java memory bloat.  Caching Inflaters at
all is only barely profitable; a one-element Inflater cache is
probably fine for those apps that occasionally iterate through the
classpath.  I don't want ThreadLocal bloat; I want _all_ the Inflaters
to go away by themselves after a while e.g. in a long-running server.
I'm open to having the cache contain more than one element, as long as
they go away eventually.  If so, I would choose a design based on
linked nodes, still with a WeakReference to make them go away.

On Thu, May 19, 2016 at 9:49 AM, Peter Levart  wrote:
>
>
> On 05/19/2016 06:31 PM, Xueming Shen wrote:
>
>
> Martin,
>
> Given we now only cache one deflater per Zip/JarFile object, is
> WeakReference here really
> necessary? Basically wr is based on the vm heap memory and deflater is a
> native memory,
> arguably we are using the wrong measurement to decide whether or not to give
> up the
> deflater's native memory. Given someone (classloader) is keeping hundreds
> and thousands
> of jar/zip files alive, is it reasonable to assume it'd better to keep the
> 32k cache for each
> of them?
>
>
> It would perhaps make more sense to keep a 32k native object for each thread
> - not each jar file, don't you think?
>
> Regards, Peter
>
>
>
> -Sherman
>
> On 5/19/16 8:00 AM, Martin Buchholz wrote:
>
> On Thu, May 19, 2016 at 7:29 AM, Peter Levart 
> wrote:
>
> But I have reservation for the implementation of one-element global cache of
> Inflater. This cache can't be efficient. In order for cache to be efficient,
> majority of calls to ZipFile.getInputStream(zipEntry) would have to be
> accompanied by a corresponding explicit close() for the input stream before
> the WeakReference to the cached Inflater is cleared.
>
> That's my assumption.  In most cases, failure to close something that
> can be closed is a bug.
> If there's code in the JDK that fails to do that, it should be fixed
> independently.
>
> The "assert !inf.ended()" in
> releaseInflater() can therefore fail as final() methods on individual
> objects that are eligible for finalization may be invoked in arbitrary
> order.
>
> Yeah, that's a bug. We can only assert after we verify that the
> Inflater is still weakly reachable.
> Updating my webrev with:
>
>* Releases the Inflater for possible later reuse.
>*/
>   private static void releaseInflater(Inflater inf) {
> -assert !inf.ended();
>   CachedInflater cachedInflater = inflaterCache.get();
>   if (inf != cachedInflater.get()) {
>   inf.end();
>   } else {
>   // "return" the Inflater to the "pool".
> +assert !inf.ended();
>   inf.reset();
>   assert cachedInflater.inUse.get();
>   cachedInflater.inUse.set(false);
>
>
>
>

RE: RFR: 8144062: Move jdk.Version to java.lang.Runtime.Version

[Iris Clark]

Hi, Mandy.

Thank you so much for pushing the changesets [0,1] for this bug.

Regards,
Iris

[0]: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/3976fadb091d
[1]: http://hg.openjdk.java.net/jdk9/dev/langtools/rev/2a49d47a37d8

Re: Helpers for MethodHandles.byteArrayViewVarHandle VarHandle

[Jaroslav Kameník]

>
> I meant change the shape of the VarHandle produced by:
>
>   MethodHandles.byteArrayViewVarHandle
>   MethodHandles.byteBufferViewVarHandle
>

Aaaha:) I must say, one reason, why I'd like to have it wrapped is
that VarHandle.get/set do not have fixed typed signature.

Helper.longAt(arr, index) looks much better than
SOME_STRANGE_CONSTANT.get(i can write here what I want and compiler says
nothing:);


> The problem is feature complete is very close and it will be hard, but not
> impossible, to make changes afterwards.
>

I should have started to write here earlier;). So, what will we do with it?


Jaroslav

Re: Review request for JDK-8156575: Add jdeps -addmods, -system, -inverse options

[Mandy Chung]

> On May 19, 2016, at 8:20 AM, Daniel Fuchs  wrote:
> 
> Hi Mandy,
> 
> I played with that a bit (-I/-addmods) and it looks good.
> 
>  92 main.opt.I=\
>  93 \  -I   -inverse Analyzes the dependences per other 
> given options\n\
>  94 \and then find all artifacts that 
> directly\n\
>  95 \and indirectly depend on the 
> matching nodes.\n\
>  96 \This is equivalent to the inverse 
> of\n\
>  97 \compile-time view analysis and 
> print\n\
>  98 \dependency summary.  This option 
> must use\n\
>  99 \with -requires, -package or -regex 
> option.
> 
> Maybe it would be useful to clarify that the 'artifacts'
> found by the -I option are jars and modules (not classes and packages).
> That was not clear for me when I started playing with it, until
> you explained it.
> 


  -ct  -compile-timeCompile-time view of transitive dependencies
i.e. compile-time view of -R option.
Analyzes the dependences per other given 
options
If a dependence is found from a directory,
a JAR file or a module, all classes in that 
containing archive are analyzed.

This defines what compile-time view analysis means.

> Also it's not clear if there's any difference between
>   -m  and
>   -addmods 
> (if you give a single module to addmods).
> Maybe there isn't and maybe it's fine.
> 

-m specifies the root module to be analyzed.

In some cases, you will need to specify -addmods to include more modules in the 
graph for analysis or needed for the resolution (e.g. java.se.ee is not in the 
default policy as specified in JEP 261).

> No need for a new webrev - I'll let you decide whether you want
> to bring in the above clarifications...

Thanks.  I’ll push as is and we could tweak the help message as a follow-up 
issue.

Mandy

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

[Peter Levart]

On 05/19/2016 06:31 PM, Xueming Shen wrote:


Martin,

Given we now only cache one deflater per Zip/JarFile object, is 
WeakReference here really
necessary? Basically wr is based on the vm heap memory and deflater is 
a native memory,
arguably we are using the wrong measurement to decide whether or not 
to give up the
deflater's native memory. Given someone (classloader) is keeping 
hundreds and thousands
of jar/zip files alive, is it reasonable to assume it'd better to keep 
the 32k cache for each

of them?


It would perhaps make more sense to keep a 32k native object for each 
thread - not each jar file, don't you think?


Regards, Peter



-Sherman

On 5/19/16 8:00 AM, Martin Buchholz wrote:
On Thu, May 19, 2016 at 7:29 AM, Peter Levart 
 wrote:
But I have reservation for the implementation of one-element global 
cache of
Inflater. This cache can't be efficient. In order for cache to be 
efficient,

majority of calls to ZipFile.getInputStream(zipEntry) would have to be
accompanied by a corresponding explicit close() for the input stream 
before

the WeakReference to the cached Inflater is cleared.

That's my assumption.  In most cases, failure to close something that
can be closed is a bug.
If there's code in the JDK that fails to do that, it should be fixed
independently.


The "assert !inf.ended()" in
releaseInflater() can therefore fail as final() methods on individual
objects that are eligible for finalization may be invoked in arbitrary
order.

Yeah, that's a bug. We can only assert after we verify that the
Inflater is still weakly reachable.
Updating my webrev with:

   * Releases the Inflater for possible later reuse.
   */
  private static void releaseInflater(Inflater inf) {
-assert !inf.ended();
  CachedInflater cachedInflater = inflaterCache.get();
  if (inf != cachedInflater.get()) {
  inf.end();
  } else {
  // "return" the Inflater to the "pool".
+assert !inf.ended();
  inf.reset();
  assert cachedInflater.inUse.get();
  cachedInflater.inUse.set(false);

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

[Xueming Shen]

Martin,

Given we now only cache one deflater per Zip/JarFile object, is 
WeakReference here really
necessary? Basically wr is based on the vm heap memory and deflater is a 
native memory,
arguably we are using the wrong measurement to decide whether or not to 
give up the
deflater's native memory. Given someone (classloader) is keeping 
hundreds and thousands
of jar/zip files alive, is it reasonable to assume it'd better to keep 
the 32k cache for each

of them?

-Sherman

On 5/19/16 8:00 AM, Martin Buchholz wrote:

On Thu, May 19, 2016 at 7:29 AM, Peter Levart  wrote:

But I have reservation for the implementation of one-element global cache of
Inflater. This cache can't be efficient. In order for cache to be efficient,
majority of calls to ZipFile.getInputStream(zipEntry) would have to be
accompanied by a corresponding explicit close() for the input stream before
the WeakReference to the cached Inflater is cleared.

That's my assumption.  In most cases, failure to close something that
can be closed is a bug.
If there's code in the JDK that fails to do that, it should be fixed
independently.


The "assert !inf.ended()" in
releaseInflater() can therefore fail as final() methods on individual
objects that are eligible for finalization may be invoked in arbitrary
order.

Yeah, that's a bug. We can only assert after we verify that the
Inflater is still weakly reachable.
Updating my webrev with:

   * Releases the Inflater for possible later reuse.
   */
  private static void releaseInflater(Inflater inf) {
-assert !inf.ended();
  CachedInflater cachedInflater = inflaterCache.get();
  if (inf != cachedInflater.get()) {
  inf.end();
  } else {
  // "return" the Inflater to the "pool".
+assert !inf.ended();
  inf.reset();
  assert cachedInflater.inUse.get();
  cachedInflater.inUse.set(false);

Re: Cleaner cleanup

[Peter Levart]

Hi Roger,

Here's the same webrev with an example of use of WeakCleanable that I 
have been thinking to propose later (I have done this in 5 minutes):


http://cr.openjdk.java.net/~plevart/jdk9-dev/Cleaner.cleanup/webrev_with_ClassLoader_exmaple/

This example uses WeakCleanable as a lightweight cleanup mechanism that 
removes entries from ClassLoader's lock map as soon as they are not 
needed any more - usually this is immediately after the class has been 
loaded. This keeps the map size from growing and has a nice impact on 
final VM footprint.


So the question is whether it is worth keeping those superclass(es) for 
such and similar cases. It's not difficult to add them back if/when 
needed. The most work is in the test.



Regards, Peter

On 05/19/2016 03:23 PM, Roger Riggs wrote:


Hi,

I haven't had time to look into this thoroughly, but since the 
at-most-once semantics
have been removed from the Phantom|Weak|SoftCleanable classes and they 
are not
used anywhere, they should be completely removed also, completing the 
cleanup.


The only function being used (except by the tests) is the primary 
support for Cleaner.register().


Roger

On 5/19/2016 6:35 AM, Christoph Engelbert wrote:

Hey Peter,

I just realized, there are two mistakes in the Javadoc code example 
inside the Cleaner Javadoc:


private final State; -> private final Statestate;
private final Cleaner.Cleanable cleanable -> private final Cleaner.Cleanable 
cleanable;

Chris


On 16 May 2016, at 00:08, Peter Levart  wrote:

Hi Roger and others,

When the new Cleaner API was created the implementation of 
Cleanable(s) was split into the low-level abstract 
[Soft|Weak|Phantom]Cleanable classes to be used internally for 
purposes where the footprint matters and their corresponding 
CleanerImpl.[Soft|Weak|Phantom]CleanableRef subclasses used as 
implementations that take a Runnable cleanup action and are exposed 
via the public Cleaner API.


When thinking of possible JDK internal use cases for the low-level 
API, I came to the conclusion that [Soft|Weak|Phantom]Cleanable 
classes are not suitable as is, because in cases where footprint 
matters, it is usually also the case that the number of 
[Soft|Weak|Phantom]Cleanable instances created is larger and that 
construction performance also matters. Especially multi-threaded 
construction. I'm thinking of the use cases of auto-cleanable 
concurrent data structures. In such use cases, the present features 
of [Soft|Weak|Phantom]Cleanable classes, namely the guaranteed 
just-once cleanup action invocation and keeping the Cleanable 
instance reachable until the cleanup action is performed, are 
actually not needed and just present footprint and performance 
(contention) overhead. They also present an overhead as they don't 
allow GC to automatically collect the Cleanable instances if the 
data structure containing them becomes unreachable and corresponding 
registered cleanup actions obsolete.


The mentioned features are important for public Cleaner.Cleanable 
instances as they are usually used for cleanup of native resources 
where the performance of their creation is not so drastically 
important and where there is no intrinsic data structure to hold 
them reachable.


I propose to move those features from the 
[Soft|Weak|Phantom]Cleanable classes down the hierarchy to the 
CleanerImpl.[Soft|Weak|Phantom]CleanableRef subclasses:


http://cr.openjdk.java.net/~plevart/jdk9-dev/Cleaner.cleanup/webrev.01/ 




In this change I also removed the 
CleanerImpl.[Soft|Weak]CleanableRef subclasses as they are not 
needed and I believe will never be. I also renamed the 
CleanerImpl.PhantomCleanableRef subclass to 
CleanerImpl.PhantomCleanableImpl.


Changes to the implementation are straightforward. The most work was 
put into the corresponding test. I did some clean-up to it and also 
changed it to accommodate for the new behavior of 
[Soft|Weak|Phantom]Cleanable classes. The changes speak for itself. 
One of the not-so obvious changes was to replace the 
CleanableCase.clearRef() action with the 
CleanableCase.releaseReferent() action. The old clearRef() action 
did not serve any purpose. Whether this method was called or not, 
the behavior of the corresponding Cleanable was unchanged as the 
Reference instance (referenced from the 'ref' field) was always of 
the same strength as the Cleanable itself. So clearing it could not 
affect the behavior of the Cleanable.


I changed 'ref' to hold a direct reference to the referent and 
renamed the field to 'referent'. I changed the EV_XXX int constants 
to Event enum constants with helper methods used in 
CleanableCase.expectedCleanups() method that now returns the number 
of expected cleanup invocations - in the PhantomCleanableImpl case 
this is the number of expected cleanup action invocations while in 
the plain XxxCleanable subclass cases it is the number of 
Cleanable.clean() meth

Re: Helpers for MethodHandles.byteArrayViewVarHandle VarHandle

[Paul Sandoz]

> On 19 May 2016, at 16:26, Jaroslav Kameník  wrote:
> 
> 
> There certainly is in nio buffer sources (and may be for encoding/decoding in 
> other areas, i am speculating as i have not looked), but for buffer sources 
> we can actually remove much if not all of Bits.java via use of the internal 
> Unsafe unaligned accessors.
> 
>  I made some notes when I was looking at it, I am attaching it if you want 
> look at it.
> 

Thanks, that is helpful.


> > getLongFrom[LE/BE](byte[] data, int firstByteIndex) 
> >  - for direct access
> > getLongFrom[LE/BE](byte[] data, int firstByteOffset, int longArrayIndex)
> >  - for loops/shifted array access
> >
> > etc.
> 
> I was proposing just the latter, and just for VarHandles :-) it’s not just 
> for loops but also to bring alignment issues to the foreground. (It also it 
> fits in with something we have been pondering about for the unsafe unaligned 
> accessors on systems that don’t support unaligned access, such as SPARC).
> 
> What do you mean by 'just for VarHandles'? Add it between VarHandle methods?
> 

I meant change the shape of the VarHandle produced by:

  MethodHandles.byteArrayViewVarHandle
  MethodHandles.byteBufferViewVarHandle

The problem is feature complete is very close and it will be hard, but not 
impossible, to make changes afterwards.

Paul.

> 
> Jaroslav
> 
> 
> 
> 

Re: Review request for JDK-8156575: Add jdeps -addmods, -system, -inverse options

[Daniel Fuchs]

Hi Mandy,

I played with that a bit (-I/-addmods) and it looks good.

  92 main.opt.I=\
  93 \  -I   -inverse Analyzes the dependences per 
other given options\n\
  94 \and then find all artifacts 
that directly\n\
  95 \and indirectly depend on the 
matching nodes.\n\
  96 \This is equivalent to the 
inverse of\n\
  97 \compile-time view analysis 
and print\n\
  98 \dependency summary.  This 
option must use\n\
  99 \with -requires, -package or 
-regex option.


Maybe it would be useful to clarify that the 'artifacts'
found by the -I option are jars and modules (not classes and packages).
That was not clear for me when I started playing with it, until
you explained it.

Also it's not clear if there's any difference between
   -m  and
   -addmods 
(if you give a single module to addmods).
Maybe there isn't and maybe it's fine.

No need for a new webrev - I'll let you decide whether you want
to bring in the above clarifications...

best regards,

-- daniel


On 14/05/16 06:48, Mandy Chung wrote:

Daniel,

I have added more tests and fixed a few issues
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8156575/webrev.02/

I also created a test case similar to yours.

This patch applies on top of:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8156680/webrev.02/

for JDK-8156680: jdeps implementation refresh

Thanks
Mandy

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

[Peter Levart]

Hi Martin,

To make it simpler to compare your and mine changes, here's the diff 
between your changed ZipFile.java and mine (mostly just removal of code):



diff -r 45dcd8ef14a7 src/java.base/share/classes/java/util/zip/ZipFile.java
--- a/src/java.base/share/classes/java/util/zip/ZipFile.javaThu May 
19 17:10:12 2016 +0200
+++ b/src/java.base/share/classes/java/util/zip/ZipFile.javaThu May 
19 17:12:54 2016 +0200

@@ -31,11 +31,9 @@
 import java.io.EOFException;
 import java.io.File;
 import java.io.RandomAccessFile;
-import java.lang.ref.WeakReference;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.attribute.BasicFileAttributes;
-import java.nio.file.Path;
 import java.nio.file.Files;

 import java.util.ArrayList;
@@ -43,21 +41,18 @@
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.Iterator;
-import java.util.Map;
 import java.util.Objects;
 import java.util.NoSuchElementException;
 import java.util.Spliterator;
 import java.util.Spliterators;
 import java.util.WeakHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 import jdk.internal.misc.JavaUtilZipFileAccess;
 import jdk.internal.misc.SharedSecrets;
 import jdk.internal.perf.PerfCounter;

-import static java.util.zip.ZipConstants.*;
 import static java.util.zip.ZipConstants64.*;
 import static java.util.zip.ZipUtils.*;

@@ -370,7 +365,7 @@
 private boolean eof = false;

 ZipFileInflaterInputStream(ZipFileInputStream zfin, int size) {
-super(zfin, getInflater(), size);
+super(zfin, new Inflater(true), size, true);
 }

 public void close() throws IOException {
@@ -379,7 +374,6 @@
 synchronized (streams) {
 streams.remove(this);
 }
-releaseInflater(inf);
 }
 }

@@ -408,72 +402,6 @@
 ? Integer.MAX_VALUE
 : (int) avail;
 }
-
-protected void finalize() throws Throwable {
-close();
-}
-}
-
-/**
- * A "pooled" inflater.  The weak reference does not prevent 
finalization

- * of the Inflater, but if get() returns non-null, then the referent is
- * surely not yet eligible for finalization (which would otherwise be a
- * rich source of bugs).
- */
-static class CachedInflater extends WeakReference {
-final AtomicBoolean inUse;
-CachedInflater(Inflater inf, boolean inUse) {
-super(inf);
-this.inUse = new AtomicBoolean(inUse);
-}
-}
-
-/**
- * Cache of Inflaters.  We use a simple one-element "pool".
- * Performance measurements show that it's barely profitable to
- * cache an Inflater (which consumes around 32kb of native memory)
- * while iterating through a zip file and decompressing many small
- * entries.
- */
-private static final AtomicReference inflaterCache
-= new AtomicReference<>(new CachedInflater(new Inflater(true), 
false));

-
-/**
- * Returns an Inflater, either a new one, or cached and reset.
- */
-private static Inflater getInflater() {
-CachedInflater cachedInflater = inflaterCache.get();
-Inflater inf = cachedInflater.get();
-if (inf == null) {
-inf = new Inflater(true);
-// attempt to install as the new cache, but failure is OK.
-inflaterCache.compareAndSet(cachedInflater,
-new CachedInflater(inf, true));
-return inf;
-} else if (!cachedInflater.inUse.get()
-   && cachedInflater.inUse.compareAndSet(false, true)) {
-assert !inf.ended();
-// we now "own" this Inflater, but must keep it weakly 
referenced.

-return inf;
-} else {
-return new Inflater(true);
-}
-}
-
-/**
- * Releases the Inflater for possible later reuse.
- */
-private static void releaseInflater(Inflater inf) {
-assert !inf.ended();
-CachedInflater cachedInflater = inflaterCache.get();
-if (inf != cachedInflater.get()) {
-inf.end();
-} else {
-// "return" the Inflater to the "pool".
-inf.reset();
-assert cachedInflater.inUse.get();
-cachedInflater.inUse.set(false);
-}
 }

 /**
@@ -804,10 +732,6 @@
 }
 }
 }
-
-protected void finalize() {
-close();
-}
 }

 static {




Regards, Peter

On 05/19/2016 05:00 PM, Martin Buchholz wrote:

On Thu, May 19, 2016 at 7:29 AM, Peter Levart  wrote:

But I have reservation for the implementation of one-element global cache of
Inflater. This cache can't be efficient. In order for cache to b

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

[Martin Buchholz]

On Thu, May 19, 2016 at 7:29 AM, Peter Levart  wrote:
> But I have reservation for the implementation of one-element global cache of
> Inflater. This cache can't be efficient. In order for cache to be efficient,
> majority of calls to ZipFile.getInputStream(zipEntry) would have to be
> accompanied by a corresponding explicit close() for the input stream before
> the WeakReference to the cached Inflater is cleared.

That's my assumption.  In most cases, failure to close something that
can be closed is a bug.
If there's code in the JDK that fails to do that, it should be fixed
independently.

> The "assert !inf.ended()" in
> releaseInflater() can therefore fail as final() methods on individual
> objects that are eligible for finalization may be invoked in arbitrary
> order.

Yeah, that's a bug. We can only assert after we verify that the
Inflater is still weakly reachable.
Updating my webrev with:

  * Releases the Inflater for possible later reuse.
  */
 private static void releaseInflater(Inflater inf) {
-assert !inf.ended();
 CachedInflater cachedInflater = inflaterCache.get();
 if (inf != cachedInflater.get()) {
 inf.end();
 } else {
 // "return" the Inflater to the "pool".
+assert !inf.ended();
 inf.reset();
 assert cachedInflater.inUse.get();
 cachedInflater.inUse.set(false);

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

[Peter Levart]

Hi Martin,


On 05/19/2016 04:27 AM, Martin Buchholz wrote:

Another batch of ZipFile hackery.  I think I finally understand how
weak references and finalizers interact.
We could eliminate the Inflater cache entirely, but this proposal
keeps a one-element Inflater cache.

http://cr.openjdk.java.net/~martin/webrevs/openjdk9/ZipFile-Inflater-cache/
https://bugs.openjdk.java.net/browse/JDK-8156484

- fixes the memory bloat from cached Inflaters
- removes small races in close
- various other minor improvements


The fixes to races are (almost) good. Also other minor improvements and 
cleanups.


But I have reservation for the implementation of one-element global 
cache of Inflater. This cache can't be efficient. In order for cache to 
be efficient, majority of calls to ZipFile.getInputStream(zipEntry) 
would have to be accompanied by a corresponding explicit close() for the 
input stream before the WeakReference to the cached Inflater is cleared. 
We know that all WeakReference(s) to a referent are 1st cleared and then 
the finalize() method is called on it. GC clears all WeakReference(s) to 
interconnected graph of weakly-reachable referents atomically, so if the 
ZipFileInflaterInputStream is closed implicitly by the finalize() 
invocation which then tries to releaseInflater(), the inflater will not 
be released as the 'cachedInflater' WeakReference will already be 
cleared. It is too late to release inflater into cache anyway because if 
the input stream is already being finalized, then the associated 
inflater has either already been finalized or will be shortly as GC 
discovers and enqueues the FinalReference(s) pointing to inflater and 
corresponding input stream in the same GC cycle - Inflater and 
corresponding input stream become eligible for finalization at the same 
time. The "assert !inf.ended()" in releaseInflater() can therefore fail 
as final() methods on individual objects that are eligible for 
finalization may be invoked in arbitrary order.


To correct that and improve the efficiency of cache, we have to do two 
things:


- we can't fix all the usages of ZipFile.getInputStream(zipEntry) out 
there, but we can at least fix the usages in URLClassLoader and 
BuiltinClassLoader which amount to most getInputStream() invocations 
without corresponding close()s when classes are being loaded (one stream 
for each class).

- we can make the Inflater cache a bit smarter.

http://cr.openjdk.java.net/~plevart/jdk9-dev/ZipFile_Inflater.cleanup/webrev.01/

We don't actually need to cache the Inflater objects themselves, but 
rather Inflater's internal ZStreamRef(s) which are just objects holding 
address to native zlib's z_stream structure. I also suggest using new 
Cleaner API for tracking reachability of Inflater(s) so that 
Inflater.end() that results from explicit stream close() clears the 
PhantomCleanable and allows GC to refrain from pushing already end()-ed 
Inflater(s)/FinalReference(s) through the finalization pipeline. I also 
took the liberty to create a trusted package-private InflaterInputStream 
constructor so that trusted subclasses can use it and so that 
usesDefaultInflater flag can be private and final. Deflater is using 
ZStreamRef(s) too, so I also had to modify Deflater, but there's no 
caching of ZStreamRef(s) in deflater as there are too many different 
kinds of them and this is not critical.


By making ZStreamRef(s) take care of themselves, corresponding logic in 
ZipFile's streams is much simpler - no need for finalize() method any 
more on ZipFileInflaterInputStream. finalize() is also not needed on 
ZipFileInputStream any more as 'streams' WeakHashMap is not holding 
Inflater instances as values any more and so finalize() would serve only 
to remove finalized ZipFile[Inflater]InputStream(s) from the WeakHashMap 
which the map is doing by itself already (expunging).


java.util.zip tests pass with this patch except 2 tests that are ignored 
and ZipFile/TestZipFile.java that prints:


java.lang.OutOfMemoryError: Java heap space: failed reallocation of 
scalar replaced objects


...which seems unrelated and the test passes when executed alone.

This touches quite a number of other classes, but it seems necessary.

So Martin, what do you think?

Regards, Peter

Re: Helpers for MethodHandles.byteArrayViewVarHandle VarHandle

[Jaroslav Kameník]

>
>
> There certainly is in nio buffer sources (and may be for encoding/decoding
> in other areas, i am speculating as i have not looked), but for buffer
> sources we can actually remove much if not all of Bits.java via use of the
> internal Unsafe unaligned accessors.
>

 I made some notes when I was looking at it, I am attaching it if you want
look at it.

> getLongFrom[LE/BE](byte[] data, int firstByteIndex)
> - for direct access
> > getLongFrom[LE/BE](byte[] data, int firstByteOffset, int
> longArrayIndex) - for loops/shifted array access
> >
> > etc.
>
> I was proposing just the latter, and just for VarHandles :-) it’s not just
> for loops but also to bring alignment issues to the foreground. (It also it
> fits in with something we have been pondering about for the unsafe
> unaligned accessors on systems that don’t support unaligned access, such as
> SPARC).
>

What do you mean by 'just for VarHandles'? Add it between VarHandle
methods?


Jaroslav

Re: RFR (M) 8148604: JEP 280, Switch to more optimal concatenation strategy

[Aleksey Shipilev]

Thanks Paul and Claes!

Hearing no objections for 24 hours now, I have pushed this change.

Thanks,
-Aleksey

On 05/18/2016 03:57 PM, Claes Redestad wrote:
> +1
> 
> I think it's a good time to enable this. I do expect some impact to
> various startup tests based on some earlier experiments, but that will
> help us pinpoint areas to focus startup optimization efforts on going
> forward.
> 
> Thanks!
> 
> /Claes
> 
> On 2016-05-18 14:09, Aleksey Shipilev wrote:
>> Hi,
>>
>> We have been waiting for this for at least 8 weeks, waiting for Jigsaw
>> to settle in, and hs-main -> jdk9/dev integration to happen that brings
>> a few testbug fixes into the jdk9/dev.
>>
>> Bug:
>>https://bugs.openjdk.java.net/browse/JDK-8148604
>>
>> Webrev:
>>http://cr.openjdk.java.net/~shade/8148604/webrev.01/
>>   (the change is XS, but impact is M)
>>
>> Targeted benchmarks show significant improvements with the
>> MH_INLINE_SIZED_EXACT strategy, see the bug for the test results.
>> Performance improvements are due to these reasons:
>>
>>   *) it does not rely on OptimizeStringConcat in C2, but instead does
>> lift up the concatenation into the Java code, spelling it out with
>> java.lang.invoke building blocks. This also helps to improve C1
>> performance significantly;
>>
>>   *) it uniformly covers the cases that OptimizeStringConcat does not
>> cover, e.g. non-int/char arguments where OptimizeStringConcat bails, etc;
>>
>>   *) it uniformly covers the Compact Strings (JEP 254) changes, e.g.
>> selecting the final coder for the concatenated String, and calling
>> appropriate coder-specific routines from JDK. Doing this in Java has the
>> apparent benefit of piggy-backing on normal Java code profiling to
>> figure out the hot branches -- a luxury OptimizeStringConcat-constructed
>> code lacks.
>>
>> Unfortunately, we cannot test this with our regular performance testing,
>> because it requires tests compiled with -target 9. The only easy target
>> we have now is javac, and switching the concat strategy does not regress
>> it (see JDK-8148605).
>>
>> MH_INLINE_SIZED_EXACT is functionally tested routinely with
>> java/lang/String/concat/ jtregs on all platforms.
>>
>> Testing: java/lang/String/ jtregs; JPRT -testset hotspot
>>
>> Thanks,
>> -Aleksey
>>
> 

Re: Cleaner cleanup

[Roger Riggs]

Hi,

I haven't had time to look into this thoroughly, but since the 
at-most-once semantics
have been removed from the Phantom|Weak|SoftCleanable classes and they 
are not
used anywhere, they should be completely removed also, completing the 
cleanup.


The only function being used (except by the tests) is the primary 
support for Cleaner.register().


Roger

On 5/19/2016 6:35 AM, Christoph Engelbert wrote:

Hey Peter,

I just realized, there are two mistakes in the Javadoc code example 
inside the Cleaner Javadoc:


private final State; -> private final Statestate;
private final Cleaner.Cleanable cleanable -> private final Cleaner.Cleanable 
cleanable;

Chris

On 16 May 2016, at 00:08, Peter Levart > wrote:


Hi Roger and others,

When the new Cleaner API was created the implementation of 
Cleanable(s) was split into the low-level abstract 
[Soft|Weak|Phantom]Cleanable classes to be used internally for 
purposes where the footprint matters and their corresponding 
CleanerImpl.[Soft|Weak|Phantom]CleanableRef subclasses used as 
implementations that take a Runnable cleanup action and are exposed 
via the public Cleaner API.


When thinking of possible JDK internal use cases for the low-level 
API, I came to the conclusion that [Soft|Weak|Phantom]Cleanable 
classes are not suitable as is, because in cases where footprint 
matters, it is usually also the case that the number of 
[Soft|Weak|Phantom]Cleanable instances created is larger and that 
construction performance also matters. Especially multi-threaded 
construction. I'm thinking of the use cases of auto-cleanable 
concurrent data structures. In such use cases, the present features 
of [Soft|Weak|Phantom]Cleanable classes, namely the guaranteed 
just-once cleanup action invocation and keeping the Cleanable 
instance reachable until the cleanup action is performed, are 
actually not needed and just present footprint and performance 
(contention) overhead. They also present an overhead as they don't 
allow GC to automatically collect the Cleanable instances if the data 
structure containing them becomes unreachable and corresponding 
registered cleanup actions obsolete.


The mentioned features are important for public Cleaner.Cleanable 
instances as they are usually used for cleanup of native resources 
where the performance of their creation is not so drastically 
important and where there is no intrinsic data structure to hold them 
reachable.


I propose to move those features from the 
[Soft|Weak|Phantom]Cleanable classes down the hierarchy to the 
CleanerImpl.[Soft|Weak|Phantom]CleanableRef subclasses:


http://cr.openjdk.java.net/~plevart/jdk9-dev/Cleaner.cleanup/webrev.01/ 




In this change I also removed the CleanerImpl.[Soft|Weak]CleanableRef 
subclasses as they are not needed and I believe will never be. I also 
renamed the CleanerImpl.PhantomCleanableRef subclass to 
CleanerImpl.PhantomCleanableImpl.


Changes to the implementation are straightforward. The most work was 
put into the corresponding test. I did some clean-up to it and also 
changed it to accommodate for the new behavior of 
[Soft|Weak|Phantom]Cleanable classes. The changes speak for itself. 
One of the not-so obvious changes was to replace the 
CleanableCase.clearRef() action with the 
CleanableCase.releaseReferent() action. The old clearRef() action did 
not serve any purpose. Whether this method was called or not, the 
behavior of the corresponding Cleanable was unchanged as the 
Reference instance (referenced from the 'ref' field) was always of 
the same strength as the Cleanable itself. So clearing it could not 
affect the behavior of the Cleanable.


I changed 'ref' to hold a direct reference to the referent and 
renamed the field to 'referent'. I changed the EV_XXX int constants 
to Event enum constants with helper methods used in 
CleanableCase.expectedCleanups() method that now returns the number 
of expected cleanup invocations - in the PhantomCleanableImpl case 
this is the number of expected cleanup action invocations while in 
the plain XxxCleanable subclass cases it is the number of 
Cleanable.clean() method invocations. I added the no-actions case to 
both PhantomCleanableImpl and XxxCleanable cases and extended the 
number and combinations of XxxCleanable cases.


The checkCleaned() method was extended to verify that the number of 
cleanup invocations is *no more* and no less then the expected.


See how WeakKey test is now simplified. This is the typical use-case 
for WeakCleanable I was talking about.



So, what do you think of this cleanup?


Regards, Peter

Re: RFR 8156718: Need tests for IsoFields getFrom for unsupported non-Iso Temporal fields

[Roger Riggs]

Hi Bhanu,

Can you remind me why the value from line: 459

field.rangeRefinedBy(LocalDate.now());

is not checked also?  Is looks odd to see a value passed to a test and 
not have it verified.


It would need to use a fixed date (the same), but that is fine for that 
test.


Thanks, Roger

On 5/19/2016 7:16 AM, Stephen Colebourne wrote:

Fine by me
Stephen

On 19 May 2016 at 11:34, Bhanu Gopularam
 wrote:

Thank you Nadeesh and Stephen.

Here is the updated webrev link:
http://cr.openjdk.java.net/~bgopularam/JDK-8156718/webrev.01

Please review.

Bhanu

-Original Message-
From: Stephen Colebourne [mailto:scolebou...@joda.org]
Sent: Tuesday, May 17, 2016 5:11 PM
To: core-libs-dev
Subject: Re: RFR 8156718: Need tests for IsoFields getFrom for unsupported 
non-Iso Temporal fields

I would also like to see the test case methods be named "getFrom" not "getfrom".

Stephen

On 17 May 2016 at 05:18, nadeesh tv  wrote:

Hi Bhanu,

I think you should add a test case comparing the return value of
getFrom()

( Not an official reviewer)

Regards,
Nadeesh

On 5/16/2016 11:46 AM, Bhanu Gopularam wrote:

Hi all,

Could you please review fix for following issue.

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

Solution: Added tck tests for validating getFrom method for
unsupported non-Iso temporal fields

Webrev: http://cr.openjdk.java.net/~bgopularam/JDK-8156718/webrev.00/

Thanks,
Bhanu


--
Thanks and Regards,
Nadeesh TV

Re: Can an object be finalized while still weakly reachable?

[Martin Buchholz]

Martin's law of object pools in the presence of finalizers:

Resurrecting a pooled object with a finalizer can be disastrous as it
can be finalized later while in active use.  Returning an object to
the pool is a common thing to do in close() methods, and close()
methods are reasonable things to call in foreign code finalizers.
So you keep it behind a weak reference at all times, even when in use.
As long as the object remains reachable through the weak reference, it
is safe from finalization.  But the weak reference also permits
finalization for resource reclamation.

On Wed, May 18, 2016 at 7:52 AM, Martin Buchholz  wrote:
> Sherman,
>
> Thank you very much for pointing me at that old thread.  I was indeed
> going down the same path and stumbling over the same obstacles!  I
> still think we can do better, but it's definitely not easy.
>
> On Tue, May 17, 2016 at 8:15 AM, Xueming Shen  wrote:
>> On 5/17/16 12:55 AM, Martin Buchholz wrote:
>>>
>>> Thanks, Peter.
>>>
>>> My current theory is indeed that I made a mistake, and have
>>> encountered my first real finalization resurrection bug.
>>> ZipFile + Inflater have 4 finalize methods and a WeakHashMap in play!
>>
>> there was a long discussion when we touched that part of the code last time.
>> just
>> for your reference:-)
>>
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-April/thread.html#6545
>>
>> -sherman
>>
>>> My static reference was finalized because it used to be unreachable,
>>> not because it is now weakly reachable!
>>>
>>> On Mon, May 16, 2016 at 11:49 PM, Peter Levart 
>>> wrote:

 Hi Martin,



 On 05/17/2016 05:19 AM, Martin Buchholz wrote:
>
> I have some evidence that an object's finalize method can run while a
> weak reference pointing to it is not yet cleared, which surprised me.
>
> E.g.
> class F { protected void finalize() { assert wref.get() != this; } }
> static WeakReference wref = new WeakReference(new F());
>
> If this is a bug, I can try to give y'all a repro recipe.
> If not, we should fix the docs
> """When the weak references to a weakly-reachable object are cleared,
> the object becomes eligible for finalization."""
>
> (It's also quite possible I made a mistake diagnosing this)


 What can happen with above code is that you get a NPE from dereferencing
 wref in finlailze(). In case NPE is not thrown and the program constructs
 only a single instance of F then assert should succeed.

 It is possible that you made a mistake. Can you post the real code?

 Regards, Peter

>>

Re: RFR 8156718: Need tests for IsoFields getFrom for unsupported non-Iso Temporal fields

[Stephen Colebourne]

Fine by me
Stephen

On 19 May 2016 at 11:34, Bhanu Gopularam
 wrote:
> Thank you Nadeesh and Stephen.
>
> Here is the updated webrev link:
> http://cr.openjdk.java.net/~bgopularam/JDK-8156718/webrev.01
>
> Please review.
>
> Bhanu
>
> -Original Message-
> From: Stephen Colebourne [mailto:scolebou...@joda.org]
> Sent: Tuesday, May 17, 2016 5:11 PM
> To: core-libs-dev
> Subject: Re: RFR 8156718: Need tests for IsoFields getFrom for unsupported 
> non-Iso Temporal fields
>
> I would also like to see the test case methods be named "getFrom" not 
> "getfrom".
>
> Stephen
>
> On 17 May 2016 at 05:18, nadeesh tv  wrote:
>> Hi Bhanu,
>>
>> I think you should add a test case comparing the return value of
>> getFrom()
>>
>>( Not an official reviewer)
>>
>> Regards,
>> Nadeesh
>>
>> On 5/16/2016 11:46 AM, Bhanu Gopularam wrote:
>>>
>>> Hi all,
>>>
>>> Could you please review fix for following issue.
>>>
>>> Bug id: https://bugs.openjdk.java.net/browse/JDK-8156718
>>>
>>> Solution: Added tck tests for validating getFrom method for
>>> unsupported non-Iso temporal fields
>>>
>>> Webrev: http://cr.openjdk.java.net/~bgopularam/JDK-8156718/webrev.00/
>>>
>>> Thanks,
>>> Bhanu
>>
>>
>> --
>> Thanks and Regards,
>> Nadeesh TV
>>

Re: RFR 8156718: Need tests for IsoFields getFrom for unsupported non-Iso Temporal fields

[nadeesh tv]

Hi Bhanu,

Looking fine to me.

Thanks and Regards,
Nadeesh

On 5/19/2016 4:04 PM, Bhanu Gopularam wrote:

Thank you Nadeesh and Stephen.

Here is the updated webrev link:
http://cr.openjdk.java.net/~bgopularam/JDK-8156718/webrev.01

Please review.
  
Bhanu


-Original Message-
From: Stephen Colebourne [mailto:scolebou...@joda.org]
Sent: Tuesday, May 17, 2016 5:11 PM
To: core-libs-dev
Subject: Re: RFR 8156718: Need tests for IsoFields getFrom for unsupported 
non-Iso Temporal fields

I would also like to see the test case methods be named "getFrom" not "getfrom".

Stephen

On 17 May 2016 at 05:18, nadeesh tv  wrote:

Hi Bhanu,

I think you should add a test case comparing the return value of
getFrom()

( Not an official reviewer)

Regards,
Nadeesh

On 5/16/2016 11:46 AM, Bhanu Gopularam wrote:

Hi all,

Could you please review fix for following issue.

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

Solution: Added tck tests for validating getFrom method for
unsupported non-Iso temporal fields

Webrev: http://cr.openjdk.java.net/~bgopularam/JDK-8156718/webrev.00/

Thanks,
Bhanu


--
Thanks and Regards,
Nadeesh TV



--
Thanks and Regards,
Nadeesh TV

Re: Cleaner cleanup

[Christoph Engelbert]

Hey Peter,

I just realized, there are two mistakes in the Javadoc code example inside the 
Cleaner Javadoc:

private final State; -> private final State state;
private final Cleaner.Cleanable cleanable -> private final Cleaner.Cleanable 
cleanable;

Chris

> On 16 May 2016, at 00:08, Peter Levart  wrote:
> 
> Hi Roger and others,
> 
> When the new Cleaner API was created the implementation of Cleanable(s) was 
> split into the low-level abstract [Soft|Weak|Phantom]Cleanable classes to be 
> used internally for purposes where the footprint matters and their 
> corresponding CleanerImpl.[Soft|Weak|Phantom]CleanableRef subclasses used as 
> implementations that take a Runnable cleanup action and are exposed via the 
> public Cleaner API.
> 
> When thinking of possible JDK internal use cases for the low-level API, I 
> came to the conclusion that [Soft|Weak|Phantom]Cleanable classes are not 
> suitable as is, because in cases where footprint matters, it is usually also 
> the case that the number of [Soft|Weak|Phantom]Cleanable instances created is 
> larger and that construction performance also matters. Especially 
> multi-threaded construction. I'm thinking of the use cases of auto-cleanable 
> concurrent data structures. In such use cases, the present features of 
> [Soft|Weak|Phantom]Cleanable classes, namely the guaranteed just-once cleanup 
> action invocation and keeping the Cleanable instance reachable until the 
> cleanup action is performed, are actually not needed and just present 
> footprint and performance (contention) overhead. They also present an 
> overhead as they don't allow GC to automatically collect the Cleanable 
> instances if the data structure containing them becomes unreachable and 
> corresponding registered cleanup actions obsolete.
> 
> The mentioned features are important for public Cleaner.Cleanable instances 
> as they are usually used for cleanup of native resources where the 
> performance of their creation is not so drastically important and where there 
> is no intrinsic data structure to hold them reachable.
> 
> I propose to move those features from the [Soft|Weak|Phantom]Cleanable 
> classes down the hierarchy to the CleanerImpl.[Soft|Weak|Phantom]CleanableRef 
> subclasses:
> 
> http://cr.openjdk.java.net/~plevart/jdk9-dev/Cleaner.cleanup/webrev.01/
> 
> 
> In this change I also removed the CleanerImpl.[Soft|Weak]CleanableRef 
> subclasses as they are not needed and I believe will never be. I also renamed 
> the CleanerImpl.PhantomCleanableRef subclass to 
> CleanerImpl.PhantomCleanableImpl.
> 
> Changes to the implementation are straightforward. The most work was put into 
> the corresponding test. I did some clean-up to it and also changed it to 
> accommodate for the new behavior of [Soft|Weak|Phantom]Cleanable classes. The 
> changes speak for itself. One of the not-so obvious changes was to replace 
> the CleanableCase.clearRef() action with the CleanableCase.releaseReferent() 
> action. The old clearRef() action did not serve any purpose. Whether this 
> method was called or not, the behavior of the corresponding Cleanable was 
> unchanged as the Reference instance (referenced from the 'ref' field) was 
> always of the same strength as the Cleanable itself. So clearing it could not 
> affect the behavior of the Cleanable.
> 
> I changed 'ref' to hold a direct reference to the referent and renamed the 
> field to 'referent'. I changed the EV_XXX int constants to Event enum 
> constants with helper methods used in CleanableCase.expectedCleanups() method 
> that now returns the number of expected cleanup invocations - in the 
> PhantomCleanableImpl case this is the number of expected cleanup action 
> invocations while in the plain XxxCleanable subclass cases it is the number 
> of Cleanable.clean() method invocations. I added the no-actions case to both 
> PhantomCleanableImpl and XxxCleanable cases and extended the number and 
> combinations of XxxCleanable cases.
> 
> The checkCleaned() method was extended to verify that the number of cleanup 
> invocations is *no more* and no less then the expected.
> 
> See how WeakKey test is now simplified. This is the typical use-case for 
> WeakCleanable I was talking about.
> 
> 
> So, what do you think of this cleanup?
> 
> 
> Regards, Peter
> 

RE: RFR 8156718: Need tests for IsoFields getFrom for unsupported non-Iso Temporal fields

[Bhanu Gopularam]

Thank you Nadeesh and Stephen.

Here is the updated webrev link:
http://cr.openjdk.java.net/~bgopularam/JDK-8156718/webrev.01

Please review.
 
Bhanu

-Original Message-
From: Stephen Colebourne [mailto:scolebou...@joda.org] 
Sent: Tuesday, May 17, 2016 5:11 PM
To: core-libs-dev
Subject: Re: RFR 8156718: Need tests for IsoFields getFrom for unsupported 
non-Iso Temporal fields

I would also like to see the test case methods be named "getFrom" not "getfrom".

Stephen

On 17 May 2016 at 05:18, nadeesh tv  wrote:
> Hi Bhanu,
>
> I think you should add a test case comparing the return value of 
> getFrom()
>
>( Not an official reviewer)
>
> Regards,
> Nadeesh
>
> On 5/16/2016 11:46 AM, Bhanu Gopularam wrote:
>>
>> Hi all,
>>
>> Could you please review fix for following issue.
>>
>> Bug id: https://bugs.openjdk.java.net/browse/JDK-8156718
>>
>> Solution: Added tck tests for validating getFrom method for 
>> unsupported non-Iso temporal fields
>>
>> Webrev: http://cr.openjdk.java.net/~bgopularam/JDK-8156718/webrev.00/
>>
>> Thanks,
>> Bhanu
>
>
> --
> Thanks and Regards,
> Nadeesh TV
>

Re: RFR 8157239 java/lang/invoke/VarHandles/ tests fail by timeout with -Xcomp with lambda form linkage

[Vladimir Ivanov]

Reviewed.

Best regards,
Vladimir Ivanov

On 5/19/16 12:47 PM, Paul Sandoz wrote:

Hi,

Please review:

  http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8157239-vh-tests-xcomp-timeout/webrev/ 


A recent change modified the VH tests to test the lambda form linkage route:

   -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false

However this causes test timeouts when the tests are run with -Xcomp. I think 
this is due to -Xcomp triggering recompilation storms due to lack of caching of 
linking lambda forms (which will be sorted out later on).

The solution for the moment is to remove that test execution mode on 
iteration-based tests, and the mode will be added back once -Xcomp execution 
issues have been resolved.

Paul.

Re: RFR 8130023 API java.util.stream: explicitly specify guaranteed execution of the pipeline

[Paul Sandoz]

> On 17 May 2016, at 23:15, Claes Redestad  wrote:
> 
> Hi,
> 
> the first block in Stream.java bothers me:
> 
> + * A stream implementation is permitted significant latitude in optimizing
> + * the computation of the result. For example, a stream implementation is 
> free
> + * to elide operations (or entire stages) from a stream pipeline -- and
> + * therefore elide invocation of behavioral parameters -- if it can prove 
> that
> + * it would not affect the result of the computation. This means, that unless
> + * otherwise specified (such as by the terminal operations {@code forEach} 
> and
> + * {@code forEachOrdered}), that side-effects of behavioral parameters may 
> not
> + * always be executed and should not be relied upon. (For a specific example 
> of
> + * such an optimization, see the API note documented on the {@link #count}
> + * operation. For more detail, see the
> + * side-effects section of the
> + * strean package documentation.)
> + *
> 
> 
> The first sentence in particular is hard to read and interpret, and after 
> reading it twice I'm not sure if it's entirely redundant or if you need to 
> better specify what other freedoms are given to a stream implementation?
> 

The first sentence is setting the stage that it is the result and not the 
computation that is key and we want to get across that this is not a limitation 
but a feature, since streams are about specifying a computation declaratively 
(and if side-effects are present then watch out!).

We cannot specify the degrees of freedom, we can only specify the result be the 
same. Perhaps the "For example, “ in the following sentence can be removed or 
replaced with “As such ….” ?


> How about this:
> 
> + * Astream implementation is free  to elide operations (or entire stages) 
> + * from a stream pipeline -- andtherefore elide invocation of behavioral + * 
> parameters -- if it can prove that  it would not affect the result of the + * 
> computation. This means that side-effects of behavioral parameters may not + 
> * always be executed and should not be relied upon, unless  otherwise 
> specified + * (such as by the terminal operations {@code forEach} and
> + * {@code forEachOrdered}).(For a specific example ofsuch an optimization, + 
> * see the API note documented on the {@link #count}  operation. For more + * 
> details, see the  side-effects 
> + * section of the  stream package documentation.)
> + *
> 

I like the move of the “unless …” to the end of the sentence, that reads better.

Thanks,
Paul.

RFR 8157239 java/lang/invoke/VarHandles/ tests fail by timeout with -Xcomp with lambda form linkage

[Paul Sandoz]

Hi,

Please review:

  
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8157239-vh-tests-xcomp-timeout/webrev/
 


A recent change modified the VH tests to test the lambda form linkage route:

   -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false

However this causes test timeouts when the tests are run with -Xcomp. I think 
this is due to -Xcomp triggering recompilation storms due to lack of caching of 
linking lambda forms (which will be sorted out later on).

The solution for the moment is to remove that test execution mode on 
iteration-based tests, and the mode will be added back once -Xcomp execution 
issues have been resolved.

Paul.

Re: [PING] [9] RFR of 5100935: No way to access the 64-bit integer multiplication of 64-bit CPUs efficiently

[Andrew Haley]

This is described as being to help RSA, etc., but it will be very hard
to use for that purpose without an add with carry.  There are many ways
to do the product, but a simple version of the core is like this:

   for i=0 to s-1
   C := 0
   for j=0 to s-1
   (C,S) := t[i+j] + a[j] * b[i] + C
   t[i+j] := S
   t[i+s] := C

   for i=0 to s-1
   C := 0
   m := t i *n' 0 mod W
   for j=0 to s-1
   (C,S) := t[i+j] + m*n[j] + C
   t[i+j] := S
   ADD(t[i+s],C)

... the result is in the carry flag and t .  The logic in the x86
version of SharedRuntime::montgomery_multiply uses a primitive which
multiplies two longs and accumulates the result into a triple-length
sum.  x86 can do this in four instructions.  I guess a primitive like
this will fit nicely with value types, but I'm not sure how it's
possible to do this with Java today.

(My apologies: I'm sure you know this already, but I didn't think it
was wise not to say anything.)

Andrew.

[Algorithm from http://koclab.cs.ucsb.edu/docs/koc/j37.pdf]

Re: RFR (XS): 8157188: 2 test failures in demo/jvmti due to unexpected class file version 53

[serguei.spit...@oracle.com]

David,

The change looks good but you already have enough reviewers. :)
Just wanted to thank you for taking steps on this issue.

Thanks,
Serguei


On 5/18/16 21:52, David Holmes wrote:
Not sure who really owns this file so cc'ing core-libs and 
serviceability.


bug: https://bugs.openjdk.java.net/browse/JDK-8157188

The file src/java.base/share/native/include/classfile_constants.h 
describes information about classfiles and is used by libverify and 
./demo/share/jvmti/java_crw_demo/java_crw_demo.c


This file has not been updated for classfile version 53 and so asserts 
will fail in java_crw_demo.c when it encounters classes compiled for 
version 53 - as they now are. This version change caused test failures 
in the hotspot forest when it was pulled down earlier this week.


This fix trivially bumps the current version number to 53 to fix the 
failing tests. It is a separate issue as to whether other changes are 
needed in this file to reflect what is new with classfile version 53.


Diff below.

Thanks,
David
--

diff -r 3eea6819cc1f 
src/java.base/share/native/include/classfile_constants.h

--- a/src/java.base/share/native/include/classfile_constants.h
+++ b/src/java.base/share/native/include/classfile_constants.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004, 2012, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2004, 2016, Oracle and/or its affiliates. All rights 
reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -31,7 +31,7 @@
 #endif

 /* Classfile version number for this information */
-#define JVM_CLASSFILE_MAJOR_VERSION 52
+#define JVM_CLASSFILE_MAJOR_VERSION 53
 #define JVM_CLASSFILE_MINOR_VERSION 0

 /* Flags */

Re: RFR (XS): 8157188: 2 test failures in demo/jvmti due to unexpected class file version 53

[David Holmes]

Thanks Alan!

David

On 19/05/2016 3:37 PM, Alan Bateman wrote:

On 19/05/2016 05:52, David Holmes wrote:

Not sure who really owns this file so cc'ing core-libs and
serviceability.

bug: https://bugs.openjdk.java.net/browse/JDK-8157188

The file src/java.base/share/native/include/classfile_constants.h
describes information about classfiles and is used by libverify and
./demo/share/jvmti/java_crw_demo/java_crw_demo.c

This file has not been updated for classfile version 53 and so asserts
will fail in java_crw_demo.c when it encounters classes compiled for
version 53 - as they now are. This version change caused test failures
in the hotspot forest when it was pulled down earlier this week.

This fix trivially bumps the current version number to 53 to fix the
failing tests. It is a separate issue as to whether other changes are
needed in this file to reflect what is new with classfile version 53.

It's the serviceability area that has historically maintained this. The
change looks fine.

-Alan

Re: RFR (XS): 8157188: 2 test failures in demo/jvmti due to unexpected class file version 53

[David Holmes]

Thanks Dmitry!

David

On 19/05/2016 3:01 PM, Dmitry Samersoff wrote:

David,

Looks good for me.

Probably I was the last person who do something with java_crw_demo.c.

-Dmitry

On 2016-05-19 07:52, David Holmes wrote:

Not sure who really owns this file so cc'ing core-libs and serviceability.

bug: https://bugs.openjdk.java.net/browse/JDK-8157188

The file src/java.base/share/native/include/classfile_constants.h
describes information about classfiles and is used by libverify and
./demo/share/jvmti/java_crw_demo/java_crw_demo.c

This file has not been updated for classfile version 53 and so asserts
will fail in java_crw_demo.c when it encounters classes compiled for
version 53 - as they now are. This version change caused test failures
in the hotspot forest when it was pulled down earlier this week.

This fix trivially bumps the current version number to 53 to fix the
failing tests. It is a separate issue as to whether other changes are
needed in this file to reflect what is new with classfile version 53.

Diff below.

Thanks,
David
--

diff -r 3eea6819cc1f
src/java.base/share/native/include/classfile_constants.h
--- a/src/java.base/share/native/include/classfile_constants.h
+++ b/src/java.base/share/native/include/classfile_constants.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004, 2012, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2004, 2016, Oracle and/or its affiliates. All rights
reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -31,7 +31,7 @@
 #endif

 /* Classfile version number for this information */
-#define JVM_CLASSFILE_MAJOR_VERSION 52
+#define JVM_CLASSFILE_MAJOR_VERSION 53
 #define JVM_CLASSFILE_MINOR_VERSION 0

 /* Flags */