Re: RFR: 8136556 - Add the ability to perform static builds of MacOSX x64 binaries

2015-10-15 Thread Alan Bateman

On 15/10/2015 19:07, Bob Vandette wrote:

Please review this JDK 9 enhancement which allows a completely static build of 
the JDK for MacOSX x64 platforms.

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


The change involves:

1. Producing “.a” archives for each native libraries.
2. Ensuring that all symbols across the JDK native libraries are unique.
3. Changing the JNI_OnLoad and JNI_OnUnload (and the Agent equivalents) to have 
the each library name appended per
the JNI specification.
4. Modifications to the launcher and launcher Makefiles to allow them to be 
linked with the java.base and jdk.jdwp.agent libraries
and function.

http://cr.openjdk.java.net/~bobv/8136556/webrev.00/ 

http://cr.openjdk.java.net/~bobv/8136556/hotspot/webrev.00/ 

http://cr.openjdk.java.net/~bobv/8136556/jdk/webrev.00/ 


Note: This change does not link every possible static library with the 
launchers.  It is currently limited to
the java.base and jdk.jdwp.agent libraries in order to allow for the TCK 
validation of the base module only.

I've skimmed through the patches and the DEF_* macros look okay. The 
only one that doesn't look right is jawt.h/jawt.c. As jawt.h is shipped 
by the JDK then I think the include of jni_util.h needs to move from 
jawt.h to jawt.c.


If I read the changes correctly then not loading the 
JavaRuntimeSupport.framework on Mac means the locale might not be right, 
is that correct? Brent might remember this issue as we've often pondered 
the implications of this disappearing in an Mac update.


Will there be continuous or at least regular builds setup so that build 
breakages will be reported in a timely manner? It would be easy to 
change something that breaks the static library build.


-Alan


Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-15 Thread Xueming Shen

On 10/15/15 1:53 AM, Paul Sandoz wrote:

On 15 Oct 2015, at 05:00, Xueming Shen  wrote:

I'm not sure if it is a good idea, from performance perspective, to add a 
"versionEntry" field into the JarEntry
to support this feature, given most of the jar files might not be multi-release-jar 
aware, and the Jar input&
output streams dont work with a multi-release jar directly. Why should they all 
pay a runtime price for it. If
we really have to add an extra field, the JarFileEntry might be a better place, 
and it might be desired to
define a new subclass JarFileEntryMR to use when the MR is enabled, instead of 
adding directly into the existing
JarFileEntry.


According to jol there is currently space available due to alignment. If there was 
not it would add about 4% in direct instance size. But the actual footprint is 
likely to be chunkier because of the string character storage for the name so the 
% increase in size would be smaller e.g. perhaps on average < 2% which might be 
ok given that i presume such entries are unlikely to be cached.

So i am not concerned about the size. If there was a way to design it to avoid 
modification of existing classes all the better, but i dunno if it is possible. 
Steve will surely know more.

Paul.



Let's try it from another angle:-) Based on the webrev, no one need to 
and actually does create a
JarEntry with a versionedEntry, except JarFile, and JarFile only creates 
its own version of JarEntry,

the JarFileEntry.

The only non-JarFile consumer of "versioned" JarEntry is the package 
private JarVerifier.getCoderSigners,
and obviously it takes a JarFile together with the source JarEntry 
(again, if this jarEntry is not from A

JarFile, it should never have a "versionedEntry")

So why do you want to put this field into the super class JarEntry, not 
the JarFileEntry, don't see any

benefit of doing that.

While I'm writing this email, it appears to me that we might have a more 
"severe" issue with the
general/base JarEntry class to hold the link to the "versionedEntry". 
The "general" JarEntry object is
not associated with a specific JarFile (the JarFileEntry does). So there 
is no way for
JarFile.getInputStream(ZipFile ze) to verify that the JarEntry passed in 
and its "versionedEntry" is
actually belong to "this" JarFile in the following implementation, if 
the "ze" is just a JarEntry but

NOT instanceof of a JarFileEntry

 759 public synchronized InputStream getInputStream(ZipEntry ze)
 760 throws IOException
 761 {
 762 maybeInstantiateVerifier();
763
764 if (ze instanceof JarEntry) {
765 ZipEntry vze = ((JarEntry)ze).versionedEntry;
766 if (vze != null) {
767 return getInputStream(vze);
768 }
769 }
770

I think it's a bug of the implementation if we don't check, as the 
"versioned entry" is really
associated to the jar file that creates it. Sure, as I said above, there 
is actually no way you can
create a general JarEntry or a JarFileEntry with a "versionedEntry" from 
"outside", but it appears
to be possible (have not tried, just in theory) to mess up the current 
mechanism by passing a
"jar entry" from one JarFile instance to another one, if two JarFile 
instances are open on the same

multi-release-jar, but with different "version setting" policy...

But again, I still believe it might be a wrong approach to add such a 
"versionedEntry" into any of the
JarEntry, including the JarFileEntry. As specified by the specification, 
the returned entry should be the
jar entry pointing to the versioned entity in the Jar File, not the root 
one. The question I would like to
ask is why do you even need the "root entry" at all, if there is a 
matched versioned one. It might be
desired to have a mechanism to return the "base/root name" for such an 
entry, but it probably does

not deserve a "dedicate" entry object.

-Sherman








Re: RFR [9] 8139706: JarFile.getBytes could use InputStream.readNBytes

2015-10-15 Thread Xueming Shen

On 10/15/15 3:08 PM, Claes Redestad wrote:


On 2015-10-15 23:21, Chris Hegarty wrote:

On 15 Oct 2015, at 21:59, e...@zusammenkunft.net wrote:

Hello,

This does change a bit the semantic of the length check. If the 
stream would return more bytes than the zipentry says the new 
version would ignore them, the  old version was consuming then and 
then fail the check. However I am not sure if this is relevant.

Right, there are certainly some subtle differences resulting from
the proposed change. When working on JDK-8138978 I thought
about using readNBytes, but played it safe as IOUtils was growing
the bye[] lazily too, so no real perf difference.  In fact, I think I 
seen

a test failure with using readNBytes here. I’ll have to check.


Seeing no jtreg test failures in java/util/zip nor java/util/jar 
(apart from 2 ignored tests), but I can see a reason for the current 
implementation being conservative: Corrupt/malicious jar files might 
lie about the entry length and report very large values, which could 
bring a down with OOME.


I believe we could be both safe and faster than baseline by adding a 
reasonable limit for when to use readNBytes, e.g., 32k would deal with 
the majority of .class files.


getBytes should be used to read the meta-inf files only, not the .class 
files.


-sherman


Re: RFR: 8136556 - Add the ability to perform static builds of MacOSX x64 binaries

2015-10-15 Thread Christian Thalinger
Copy-pasting the comment I made earlier (internally):

>> make/bsd/makefiles/gcc.make:
>> 
>> + ifeq ($(BUILD_STATIC),true)
>> + CXXFLAGS += -DSTATIC_BUILD
>> + CFLAGS += -DSTATIC_BUILD
>> 
>> Can we use the same name everywhere?
> 
> We were trying to differentiate Makefile options from compile time 
> conditionals.
> In one case it’s set to true and the other it’s defined.
> 
> Are there no other cases where this is done?

I’m not sure but looking at make/excludeSrc.make all of them use the same name.

> On Oct 15, 2015, at 8:10 AM, Bob Vandette  wrote:
> 
> Please review this JDK 9 enhancement which allows a completely static build 
> of the JDK for MacOSX x64 platforms.
> 
> https://bugs.openjdk.java.net/browse/JDK-8136556 
> 
> 
> The change involves:
> 
> 1. Producing “.a” archives for each native libraries.
> 2. Ensuring that all symbols across the JDK native libraries are unique.
> 3. Changing the JNI_OnLoad and JNI_OnUnload (and the Agent equivalents) to 
> have the each library name appended per
>   the JNI specification.
> 4. Modifications to the launcher and launcher Makefiles to allow them to be 
> linked with the java.base and jdk.jdwp.agent libraries
>   and function.
> 
> http://cr.openjdk.java.net/~bobv/8136556/webrev.00/ 
> 
> http://cr.openjdk.java.net/~bobv/8136556/hotspot/webrev.00/ 
> 
> http://cr.openjdk.java.net/~bobv/8136556/jdk/webrev.00/ 
> 
> 
> Note: This change does not link every possible static library with the 
> launchers.  It is currently limited to
> the java.base and jdk.jdwp.agent libraries in order to allow for the TCK 
> validation of the base module only.
> 
> 
> Bob.



Re: RFR [9] 8139706: JarFile.getBytes could use InputStream.readNBytes

2015-10-15 Thread Chris Hegarty

> On 15 Oct 2015, at 11:08 p.m., Claes Redestad  
> wrote:
> 
> 
> On 2015-10-15 23:21, Chris Hegarty wrote:
>>> On 15 Oct 2015, at 21:59, e...@zusammenkunft.net wrote:
>>> 
>>> Hello,
>>> 
>>> This does change a bit the semantic of the length check. If the stream 
>>> would return more bytes than the zipentry says the new version would ignore 
>>> them, the  old version was consuming then and then fail the check. However 
>>> I am not sure if this is relevant.
>> Right, there are certainly some subtle differences resulting from
>> the proposed change. When working on JDK-8138978 I thought
>> about using readNBytes, but played it safe as IOUtils was growing
>> the bye[] lazily too, so no real perf difference.  In fact, I think I seen
>> a test failure with using readNBytes here. I’ll have to check.
> 
> Seeing no jtreg test failures in java/util/zip nor java/util/jar (apart from 
> 2 ignored tests), but I can see a reason for the current implementation being 
> conservative: Corrupt/malicious jar files might lie about the entry length 
> and report very large values, which could bring a down with OOME.
> 
> I believe we could be both safe and faster than baseline by adding a 
> reasonable limit for when to use readNBytes, e.g., 32k would deal with the 
> majority of .class files.

Sounds good to me.

-Chris.

> /Claes


Re: RFR [9] 8139706: JarFile.getBytes could use InputStream.readNBytes

2015-10-15 Thread Claes Redestad


On 2015-10-15 23:21, Chris Hegarty wrote:

On 15 Oct 2015, at 21:59, e...@zusammenkunft.net wrote:

Hello,

This does change a bit the semantic of the length check. If the stream would 
return more bytes than the zipentry says the new version would ignore them, the 
 old version was consuming then and then fail the check. However I am not sure 
if this is relevant.

Right, there are certainly some subtle differences resulting from
the proposed change. When working on JDK-8138978 I thought
about using readNBytes, but played it safe as IOUtils was growing
the bye[] lazily too, so no real perf difference.  In fact, I think I seen
a test failure with using readNBytes here. I’ll have to check.


Seeing no jtreg test failures in java/util/zip nor java/util/jar (apart 
from 2 ignored tests), but I can see a reason for the current 
implementation being conservative: Corrupt/malicious jar files might lie 
about the entry length and report very large values, which could bring a 
down with OOME.


I believe we could be both safe and faster than baseline by adding a 
reasonable limit for when to use readNBytes, e.g., 32k would deal with 
the majority of .class files.


/Claes


Re: RFR [9] 8139706: JarFile.getBytes could use InputStream.readNBytes

2015-10-15 Thread Chris Hegarty

> On 15 Oct 2015, at 21:59, e...@zusammenkunft.net wrote:
> 
> Hello,
> 
> This does change a bit the semantic of the length check. If the stream would 
> return more bytes than the zipentry says the new version would ignore them, 
> the  old version was consuming then and then fail the check. However I am not 
> sure if this is relevant.

Right, there are certainly some subtle differences resulting from
the proposed change. When working on JDK-8138978 I thought
about using readNBytes, but played it safe as IOUtils was growing
the bye[] lazily too, so no real perf difference.  In fact, I think I seen
a test failure with using readNBytes here. I’ll have to check.

-Chris.


> Gruss
> Bernd
> 
> -- 
> http://bernd.eckenfels.net
> 
> -Original Message-
> From: Claes Redestad 
> To: "core-libs-dev@openjdk.java.net Libs" 
> Sent: Do., 15 Okt. 2015 22:43
> Subject: RFR [9] 8139706: JarFile.getBytes could use InputStream.readNBytes
> 
> Hi all,
> 
> java.util.jar.JarFile could be improved further by using 
> InputStream.readNBytes when there's information in the ZipEntry about 
> the entry size.
> 
> Webrev: http://cr.openjdk.java.net/~redestad/8139706/webrev.01/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8139706
> 
> Testing: verified improvements in internal startup/footprint tests
> 
> /Claes



Re: RFR [9] 8139706: JarFile.getBytes could use InputStream.readNBytes

2015-10-15 Thread Claes Redestad

Hi Bernd,

thanks for looking at this!

You're right about the semantic change for this patch in isolation, but 
compared to the previous implementation that used sun.misc.IOUtils the 
semantics are unchanged (compare with theprevious implementation of 
JarFiles::getBytes and sun.misc.IOUtils::readFully in 
http://hg.openjdk.java.net/jdk9/dev/jdk/rev/5537b74dea17 )


/Claes

On 2015-10-15 22:59, e...@zusammenkunft.net wrote:

Hello,

This does change a bit the semantic of the length check. If the stream would 
return more bytes than the zipentry says the new version would ignore them, the 
 old version was consuming then and then fail the check. However I am not sure 
if this is relevant.

Gruss
Bernd





Re: [9] RFR(XS): 8137173: @HotSpotIntrinsicCandidate is not Oracle-specific

2015-10-15 Thread mark . reinhold
2015/10/1 10:57 -0700, zoltan.m...@oracle.com:
> On 10/01/2015 05:54 PM, mark.reinh...@oracle.com wrote:
>> I suggest putting this into jdk.internal.vm.annotation, which is
>> also a good place for the ReservedStackAccess annotation envisioned
>> in JEP 270 (http://openjdk.java.net/jeps/270).
> 
> I filed an RFE, JDK-8138732: "move @HotSpotIntrinsicCandidate to the 
> jdk.internal.vm.annotation package" [1], to track the issue of moving 
> the annotation to a different package. I hope I can take care of it soon.
> 
> Thank you and best regards,

Thanks.  While you're at it, could you please rename the annotation to
@IntrinsicCandidate?  There's no need for the "HotSpot" prefix any more
since this annotation will now be in a package that is reserved for
VM-specific annotations.

(Sorry for this late suggestion; this just came up in a discussion of
 the @ReservedStackAccess annotation for JEP 270, which is also destined
 for the jdk.internal.vm.annotation package.)

(I've pasted this addendum into JDK-8138732.)

- Mark


Re: RFR [9] 8139706: JarFile.getBytes could use InputStream.readNBytes

2015-10-15 Thread ecki
Hello,

This does change a bit the semantic of the length check. If the stream would 
return more bytes than the zipentry says the new version would ignore them, the 
 old version was consuming then and then fail the check. However I am not sure 
if this is relevant.

Gruss
Bernd

-- 
http://bernd.eckenfels.net

-Original Message-
From: Claes Redestad 
To: "core-libs-dev@openjdk.java.net Libs" 
Sent: Do., 15 Okt. 2015 22:43
Subject: RFR [9] 8139706: JarFile.getBytes could use InputStream.readNBytes

Hi all,

java.util.jar.JarFile could be improved further by using 
InputStream.readNBytes when there's information in the ZipEntry about 
the entry size.

Webrev: http://cr.openjdk.java.net/~redestad/8139706/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8139706

Testing: verified improvements in internal startup/footprint tests

/Claes


RFR [9] 8139706: JarFile.getBytes could use InputStream.readNBytes

2015-10-15 Thread Claes Redestad

Hi all,

java.util.jar.JarFile could be improved further by using 
InputStream.readNBytes when there's information in the ZipEntry about 
the entry size.


Webrev: http://cr.openjdk.java.net/~redestad/8139706/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8139706

Testing: verified improvements in internal startup/footprint tests

/Claes


RFR: 8136556 - Add the ability to perform static builds of MacOSX x64 binaries

2015-10-15 Thread Bob Vandette
Please review this JDK 9 enhancement which allows a completely static build of 
the JDK for MacOSX x64 platforms.

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


The change involves:

1. Producing “.a” archives for each native libraries.
2. Ensuring that all symbols across the JDK native libraries are unique.
3. Changing the JNI_OnLoad and JNI_OnUnload (and the Agent equivalents) to have 
the each library name appended per
   the JNI specification.
4. Modifications to the launcher and launcher Makefiles to allow them to be 
linked with the java.base and jdk.jdwp.agent libraries
   and function.

http://cr.openjdk.java.net/~bobv/8136556/webrev.00/ 

http://cr.openjdk.java.net/~bobv/8136556/hotspot/webrev.00/ 

http://cr.openjdk.java.net/~bobv/8136556/jdk/webrev.00/ 


Note: This change does not link every possible static library with the 
launchers.  It is currently limited to
the java.base and jdk.jdwp.agent libraries in order to allow for the TCK 
validation of the base module only.


Bob.

Re: [9] RFR 8138838: docs cleanup for java.desktop

2015-10-15 Thread Semyon Sadetsky

Hi Alexander,

Since you are doing cosmetic changes, could please wrap the amended 
lines to 80 characters per line?

Also some notes:
MultiResolutionImage.java interface has a mix of verbose/implicit method 
modifiers. It would be nice to reduce it to the uniform style.
MenuComponent.java : @param d - the Dimension...   - Should 
it also be replaced with brackets?

PrinterInfo.java - also  is used.

--Semyon



On 10/14/2015 7:49 PM, Alexander Stepanov wrote:
Sorry, just a reminder. If the activity is untimely, then could you 
please review the following minimum part of fix?


http://cr.openjdk.java.net/~avstepan/8138838/webrev.min.00/index.html
(some misprints + midget JDK-8138893 fixed).

Thanks,
Alexander

On 10/5/2015 2:12 PM, Alexander Stepanov wrote:

Hello,

Could you please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8138838

Patch + webrev zipped + specdiff report:
http://cr.openjdk.java.net/~avstepan/8138838

Just some cosmetic changes for docs (... -> {@code ...} 
replacement) + some misprints fixed.


Not sure if these changes are desired at all for now.

Thanks,
Alexander

(Just in case, adding the prehistory and sending a copy to 
core-libs-dev).




On 10/1/2015 2:31 PM, Alexander Stepanov wrote:

Hello Martin, Stuart,

Thank you for the notes,

Yes, the initial utility is quite ugly, I just tried to prepare it 
as quickly as possible hoping that it covers the majority of 
"trivial" replace cases. Yes, it does not process multi-line  
inclusions.


>  s = s.replace( "", tag1);
>  s = s.replace( "", tag1);
>  s = s.replace("", tag2);
>  s = s.replace("", tag2);

- replaced with "s = ln.replaceAll("(?i)", 
"").replaceAll("(?i)", "");"


Unfortunately my Perl/lisp knowledge are zero :)

> Should you publish your specdiff?  I guess not - it would be empty!
For now it contains a single fixed misprint diff, but there are a 
few another misprints at the moment which I'd like to include in the 
patch as well.


So if you don't have objections, I'll delay for a several days and 
then publish a final RFR (probably containing changes in some other 
repos like jaxws, corba or jaxp) which would be more formal 
(containing bug # and the final specdiff report).


Thanks again,
Alexander


On 10/1/2015 9:54 AM, Martin Buchholz wrote:

Hi s'marks,
You probably don't need to absolutify paths.
And you can easily handle multiple args.

(just for fun!)
Checks for javadoc comment; handles popular html entities; handles 
multiple lines; handles both tt and code:


#!/bin/bash
find "$@" -name '*.java' | \
  xargs -r perl -p0777i -e \
  'do {} while s~^ 
*\*.*\K<(tt|code)>((?:[^<>{}\&\@]|&(?:lt|gt|amp);)*)~$_=$2; 
s/> wrote:


Hi Alexander, Martin,

The challenge of Perl file slurping and Emacs one-liners was too
much to bear.

This is Java, so one-liners are hardly possible. Still, there are
a bunch of improvements that can be made to the Java version. (OK,
and I'm showing off a bit.)

Take a look at this:

http://cr.openjdk.java.net/~smarks/misc/SimpleTagEditorSmarks1.java 
 



I haven't studied the output exhaustively, but it seems to do a
reasonably good job for the common cases. I ran it over java.lang
and I noticed a few cases where there is markup embedded within
 text, which should be looked at more closely.

I don't particularly care if you use my version, but there are
some techniques that I'd strongly recommend that you consider
using in any such tool. In particular:

 - Pattern.DOTALL to do multi-line matches
 - Pattern.CASE_INSENSITIVE
 - try-with-resources to ensure that files are closed properly
 - NIO instead of old java.io  APIs, particularly
Files.walk() and streams
 - use Scanner to deal with input file buffering
 - Scanner's stream support (I recently added this to JDK 9)

Enjoy,

s'marks



On 9/29/15 2:23 PM, Martin Buchholz wrote:

Hi Alexander,

your change looks good.  It's OK to have manual corrections
for automated
mega-changes like this, as long as they all revert changes.

Random comments:

Should you publish your specdiff?  I guess not - it would be
empty!

 while((s = br.readLine()) != null) {

by matching only one line at a time, you lose the ability 
to make

replacements that span lines.  Perlers like to "slurp" in the
entire file
as a single string.

 s = s.replace( "", tag1);
 s = s.replace( "", tag1);
 s = s.replace("", tag2);
 s = s.replace("", tag2);

Why not use case-insensitive regex?

Here's an emacs-lisp one-liner I've been known to use:

(defun tt-code ()
   (i

Re: [concurrency-interest] Spin Loop Hint support: Draft JEP proposal

2015-10-15 Thread Gil Tene

> On Oct 15, 2015, at 11:32 PM, Doug Lea  wrote:
> 
> On 10/14/2015 11:53 PM, Gil Tene wrote:
>> I agree on the separation between spin-hinting and monitor-like constructs.
>> But not so much on the analogy to or use of the term "yield" to describe what
>> is intended y spin hints.
>> 
> 
> I've been focussing on the spec, which still seems to best support
> this naming. Let's try fleshing out some more (for no-arg version).
> 
>  /**
>   * A hint to the platform that the current thread is momentarily
>   * unable to progress until the occurrence of one or more actions
>   * of one or more other threads. The method is mainly applicable
>   * in spin-then-block constructions entailing a bounded number
>   * of re-checks of a condition, separated by spinYield(), followed
>   * if necessary with use of a blocking synchronization mechanism.
>   */
>  public static void spinYield();

I don't think that this is a good description of the use cases. Yes, the hint 
is helpful for spin-then-block constructions, but that's just a part of where 
it can help. In fact, I expect the hint to be very applicable for 
indefinitely-spinning loops, and I expect the measurable impact there to be 
much more reliably noticed because such loops are invariably concerned with 
fast reaction times above all else.

I also don't think that the "…momentarily unable to progress until the 
occurrence of one or more actions of one or more other threads. " is true: 
while (!(done || (count++ > threshold))) { spinLoopHint(); } can progress 
without any action by any other thread.

As noted in my proposed JavaDoc, I see the primary indication of the hint to be 
that the reaction time to events that would cause the loop to exit (e.g. in 
nanosecond units) is more important to the caller than the speed at which the 
loop is executing (e.g. in "number of loop iterations per second" units). So if 
we are focusing on the spec, here is my suggested (edited to be more specific
) spec:

/**
* Provide the JVM with a hint that this call is made from within a spinning
* loop. The JVM may assume that the reaction time to events that would
* cause the loop to terminate is more important than the speed of executing
* the loop (e.g. in terms of number of loop iterations per second).
* Power savings may also occur, but those are considered incidental to the 
* primary purpose of improving reaction time. The JVM will not slow down
* the loop execution to a point where execution will be delayed indefinitely,
* but other choices of loop execution speed are system-specific. Note that a
* nop is a valid implementation of this hint.
*/

> What should be the response to this hint? When applicable
> and available, the JVM should just issue PAUSE. But on a uniprocessor,
> or when load average is easily detected to be high, or
> on a tightly packed cloud node, a plain yield or something
> along those lines might be a better use of this hint, that
> the spec should not rule out.

Anyone running indefinite spin loops on a uniprocessor deserves whatever they 
get. Yielding in order to help them out is not mercy. Let Darwin take care of 
them instead.

But indefinite user-mode spinning on many-core systems is a valid and common 
use case (see the disruptor link in my previous e-mail). And because a spin 
loop hint is extremely useful for indefinitely spinning loop situations, and a 
spin hint is primarily intended to improve the reaction time of spin loops, I 
would describe any explicit yielding by the JVM at the hint point as 
mis-behavior. [Not quite an invalid behavior, because we don't want to specify 
allowed behavior too strongly, but certainly surprising, unexpected, and highly 
disappointing given the intent expressed by the hint]. Yes, the OS or 
hypervisor may choose to preempt a thread at any random point in code, 
including at these hint points, but that's their job and their problem, and not 
the JVM's. The JVM should not be in the business of voluntarily and implicitly 
yielding at specific points in code, and especially not at points in code that 
spins and hints that it wants to improve the performance of that spin.

If what you want is a spin loop that yields, write one: while (!done) {  
yield(); }. I don't see how while (!done) { spinYield(); } has any different 
meaning to the reader. It just reads as something like "yield faster, knowing 
that you are in a spin".

> Also, I believe that some x86
> hypervisors intercept PAUSE and do something roughly similar
> after repeated invocations.

As to hypervisor choices: preempting a guest OS at a PAUSE instruction is 
actually higher risk, since the PAUSE instruction could be taken while holding 
a ciritical kernel resource (e.g. mremap always grabs one spinlock while 
holding another spinlock). The trick most hypervisors seem to use is to prefer 
to preempt code in user mode rather than in kernel mode, since user mode code 
doesn't see preemption as an invalid operation, but kernel code paths (e.g 
those 

Re: RFR: JDK-8046565: Platform Logger API and Service

2015-10-15 Thread Daniel Fuchs

Hi Stephen,

On 15/10/15 12:48, Stephen Colebourne wrote:

On 14 October 2015 at 12:20, Daniel Fuchs  wrote:

On 14/10/15 07:21, Mandy Chung wrote:

Several log methods throws NPE if the level is legible and the parameter
is null.  E.g.
* @throws NullPointerException if {@code level} is {@code null}, or if the
level
* is loggable and {@code msgSupplier} is {@code null}.

Should it throw NPE if the input parameter is null regardless of whether
the level is loggable?



Yes probably. The reason it is like that is that it mimics
what java.util.logging.Logger does (at least for message suppliers), but
it is probably not a good idea to propagate this bad practice.

I will change System.Logger spec to mandate to always throw NPE
if Supplier is null - and ensure that the implementations
we have follow the spec.


I believe the desire in logging is to a sensible degree write
defensively to avoid getting an exception due to logging which
obscures the actual error.

Thus, log(level, msg) should accept a null message (but not a null level)
But, log(level, supplier) should not accept a null supplier or level


Yes exactly. I agree.


The Javadoc for 'System.Logger' starts with "The minimum set of
methods that a logger returned by..." which isn't a good description
of the interface.


Right. Mandy already told me that :-) Working on finding a better
wording.



I have a major concern that the class names 'Logger' and 'Level'
duplicate those of java.util.logging. While they are inner classes as
opposed to top level classes, both IntelliJ and Eclipse will find the
inner class and top level class when typing "Logger". This will no
doubt cause many users to import the wrong one. I propose that these
classes are renamed to avoid this problem. The simplest would be to
change them from inner classes to top level classes "System.Logger" ->
"SystemLogger". Alternatively, they could stay as inner classes and be
prefixed "System.Logger" -> "System.SysLogger" or "System.Logger" ->
"System.BasicLogger".


I usually like to avoid to have two classes with the same
name in different packages. As you say it can be confusing
when you mistakenly import the wrong one.

In NetBeans - when doing name completion, System.Logger
appears with a different font than j.u.l.Logger because it
is an interface. And if you search the file - then obviously
it is not proposed because it's an inner interface.

After having worked with it for some time I find that using
inner interfaces/classes for Logger and Level is not that
bad. Certainly better than if it was a top-level class of
the same name. It is fortunately rare that you need to use
both (the System. and the j.u.l one) in the same class.

It's hard to find a compelling new name though :-)

best regards,

-- daniel


Stephen





Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-10-15 Thread Roger Riggs

Hi Peter,

It was not the (my/our) intention for the Cleaner to be able to 
completely replace finalization.
The emphasis is on keeping it simple and efficient and covering the 99% 
of the cases.



On 10/15/2015 9:49 AM, Peter Levart wrote:



On 10/15/2015 03:21 PM, Roger Riggs wrote:

Peter, Chris,

Plausible but getting complicated as Chris observed.

We can be on the lookout for specific cases in the JDK like that. I 
expect most

can be resolved by specific cooperation between the super/subclasses.


How? Such "cooperation" can not use the tracked object's instance 
methods as the instance is gone when cleanup happens, so no overriding 
is possible. Can you rewrite this example:


class SuperClass {
protected final Cleanup cleanup = 
XXX.getCleaner().phantomCleanup(this);


SuperClass() {
cleanup.append(() -> {... super class cleanup ...});
}
}

class SubClass extends SuperClass {
SubClass() {
super();
cleanup.prepend(() -> {... pre-actions ...})
.append(() -> {... post-actions ...});
}
}


...using just current simple Cleaner API?

I think that if we design an API to replace finalize(), then it should 
support use cases that are possible with finalize(). Not just use 
cases in the JDK itself, but also elsewhere on the planet.
I'd be interested in the other cases, but it is not a goal to solve 
every finalization problem.


The cleanup of each class should *only* be doing cleanup for their 
own class.
Because it is invoked after finalization, there can be *no* access to 
the instance
being cleaned.  Any shared state will be duplicated between the two 
cleanable behaviors.
If the subclass shares state with the superclass (protected fields or 
references)

then the cleanup needs to be co-designed.


Even if state needed for clean-up is duplicated (it can just be doubly 
referenced, for example: superclass establishes state and exposes it 
to subclass that just references it), registering separate Cleanup 
instances per super/subclass means that you can not control the order 
of them fire-ing. If for example, the subclass cleanup logic needs the 
superclass still be fully functional, it can do that now by:


@Override protected void finalize() {
... subclass cleanup ... // flushing something - needs 
superclass functionality

super.finalize();
}

With separate Cleanable(s) for super/subclass, the order is not 
guaranteed.
The superclass can expose its Cleanable and additional functionality to 
prepend or append.
That function do not exclusively need to be supported by the 
Cleaner/Cleanable types.


The order can be ensured by registering the cleanups with each other and 
delegating

to the other to achieve a desired execution order.

This would put the necessary mechanisms in the types that need 
specialized behavior

without adding complexity to the Cleaner.

If it turns out that it is a frequent pattern then it would make sense 
to look at it again.


Regards, Roger






Canceling cleanup is easier, because the object has not been finalized.
If such a function is needed, it should be part of the API of the 
superclass

and part of the contract.


Ideally such methods would be in superclass only and be final and just 
delegate to Cleanable.clear() or Cleanable.clean(). The Cleanable 
should be the only place where the cleanup logic is encapsulated.




Also, I've seen a few calls to super.finalize() where there were no 
finalizers
in any of the superclasses.  It would be considered good design to 
always include it.

I don't know if the optimization for empty finalize methods includes the
case where it only calls super.finalize().


Ah, I forgot that empty finalize() is optimized away (the Object is 
not even registered for finalization).


Regards, Peter



Roger

On 10/15/15 7:43 AM, Chris Hegarty wrote:

Peter,

On 15 Oct 2015, at 09:12, Peter Levart  wrote:


On 10/14/2015 07:43 PM, Roger Riggs wrote:

Hi Alan, Mandy,

I looked at a few of the many uses of finalize and the likely 
changes.

The zip Inflater and Deflater are relatively simple cases.
Some finalizers are not used and can be removed.
The sun.net.www.MeteredStream example subclasses PhantomCleanable 
to add the state and cleanup

behavior.

http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/

Some of the harder cases will take more time to disentangle the 
cleanup code.
For example, ZipFile, and FileIn/OutputStream (Peter has 
prototyped this).


Roger

Hi Roger,

It's good to see some actual uses of the API and how it is supposed 
to be used in migration from finalize() methods. I think empty 
protected finalize() method is almost always safe to remove. If the 
class is not subclassed, it serves no purpose (unless some other 
same-package class or itself is calling it, which can be checked 
and those calls removed). If subclass overrides finalize() and 
calls super.finalize(), its ok (it will call Object.finalize then 
when empty finalize() is

Re: [concurrency-interest] Spin Loop Hint support: Draft JEP proposal

2015-10-15 Thread Doug Lea

On 10/14/2015 11:53 PM, Gil Tene wrote:

I agree on the separation between spin-hinting and monitor-like constructs.
But not so much on the analogy to or use of the term "yield" to describe what
is intended y spin hints.



I've been focussing on the spec, which still seems to best support
this naming. Let's try fleshing out some more (for no-arg version).

  /**
   * A hint to the platform that the current thread is momentarily
   * unable to progress until the occurrence of one or more actions
   * of one or more other threads. The method is mainly applicable
   * in spin-then-block constructions entailing a bounded number
   * of re-checks of a condition, separated by spinYield(), followed
   * if necessary with use of a blocking synchronization mechanism.
   */
  public static void spinYield();

What should be the response to this hint? When applicable
and available, the JVM should just issue PAUSE. But on a uniprocessor,
or when load average is easily detected to be high, or
on a tightly packed cloud node, a plain yield or something
along those lines might be a better use of this hint, that
the spec should not rule out. Also, I believe that some x86
hypervisors intercept PAUSE and do something roughly similar
after repeated invocations.


While the spinYield() example in your e-mail below can work from a semantic
point of view in the same code, IMO the word "yield" suggests the exact
opposite of what spnLoopHint() is intending to do or hint at


Maybe. If you are on a system with load > #cpus, or with
certain forms of hypervisor,  or without a PAUSE instruction,
spinYield might not improve responsiveness but might still
improve system throughput.

-Doug



Re: RFC: draft API for JEP 269 Convenience Collection Factories

2015-10-15 Thread Remi Forax
Hi Stephen,
you can use Arrays.setAll() which do something similar.

The code seems really simple,
  public static  List of(int size, IntFunction valueFunction) {
T[] array = (T[])new Object[size];
Arrays.setAll(array, valueFunction);
return List.of(array);
  }
or I miss something ?

Rémi

- Mail original -
> De: "Stephen Colebourne" 
> À: "core-libs-dev" 
> Envoyé: Jeudi 15 Octobre 2015 16:28:00
> Objet: Re: RFC: draft API for JEP 269 Convenience Collection Factories
> 
> I've been working on a Java 8 wrapper class around double[] in my day
> job, and added the following factory method:
> 
>   /**
>* Obtains an instance with entries filled using a function.
>* 
>* The function is passed the array index and returns the value for
> that index.
>*
>* @param size  the number of elements
>* @param valueFunction  the function used to populate the value
>* @return an array initialized using the function
>*/
>   public static DoubleMatrix1D of(int size, IntToDoubleFunction
>   valueFunction) {
> if (size == 0) {
>   return EMPTY;
> }
> double[] array = new double[size];
> for (int i = 0; i < array.length; i++) {
>   array[i] = valueFunction.applyAsDouble(i);
> }
> return new DoubleMatrix1D(array);
>   }
> 
> View on GitHub here:
> https://github.com/OpenGamma/Strata/commit/63e73652194a3dd94e37fbc407f4933c10abadda#diff-2a9787868cf0654b4a6a07e75c1a6286R199
> 
> It occurs to me that it would be a *very* good idea to add a similar
> method to List.
> 
> public static  List of(int size, IntFunction valueFunction) { ... }
> 
> Stephen
> 
> 
> 
> On 14 October 2015 at 11:39, Stephen Colebourne  wrote:
> > On 14 October 2015 at 10:38, Paul Sandoz  wrote:
> >>> On 14 Oct 2015, at 06:18, Stuart Marks  wrote:
> >>> I'm not entirely sure what to take from this. If it were clearly
> >>> exponential, we could say with confidence that above a certain threshold
> >>> there would be vanishingly little benefit adding more arguments. But
> >>> since the curve seems to flatten out, maybe this is pushing us to add
> >>> more pairs than we had originally thought. The current draft API has 8
> >>> pairs; that seems to leave a few percent of cases on the table.
> >>> Obviously we can't get to 100%, but is 97% good enough?
> >
> > I'd say 5 is definitely too little, without an easy builder fallback
> > (as Guava provides). I'd say the right number is between 8 and 10.
> >
> > Stephen
> 


Re: RFC: draft API for JEP 269 Convenience Collection Factories

2015-10-15 Thread Paul Sandoz

> On 15 Oct 2015, at 16:28, Stephen Colebourne  wrote:
> 
> I've been working on a Java 8 wrapper class around double[] in my day
> job, and added the following factory method:
> 
>  /**
>   * Obtains an instance with entries filled using a function.
>   * 
>   * The function is passed the array index and returns the value for
> that index.
>   *
>   * @param size  the number of elements
>   * @param valueFunction  the function used to populate the value
>   * @return an array initialized using the function
>   */
>  public static DoubleMatrix1D of(int size, IntToDoubleFunction valueFunction) 
> {
>if (size == 0) {
>  return EMPTY;
>}
>double[] array = new double[size];
>for (int i = 0; i < array.length; i++) {
>  array[i] = valueFunction.applyAsDouble(i);
>}
>return new DoubleMatrix1D(array);
>  }
> 

Within that method you can use Arrays.setAll(array, valueFunction). In 
hindsight it would have been useful for that method to return the filled array.

Paul.


Re: RFC: draft API for JEP 269 Convenience Collection Factories

2015-10-15 Thread Stephen Colebourne
I've been working on a Java 8 wrapper class around double[] in my day
job, and added the following factory method:

  /**
   * Obtains an instance with entries filled using a function.
   * 
   * The function is passed the array index and returns the value for
that index.
   *
   * @param size  the number of elements
   * @param valueFunction  the function used to populate the value
   * @return an array initialized using the function
   */
  public static DoubleMatrix1D of(int size, IntToDoubleFunction valueFunction) {
if (size == 0) {
  return EMPTY;
}
double[] array = new double[size];
for (int i = 0; i < array.length; i++) {
  array[i] = valueFunction.applyAsDouble(i);
}
return new DoubleMatrix1D(array);
  }

View on GitHub here:
https://github.com/OpenGamma/Strata/commit/63e73652194a3dd94e37fbc407f4933c10abadda#diff-2a9787868cf0654b4a6a07e75c1a6286R199

It occurs to me that it would be a *very* good idea to add a similar
method to List.

public static  List of(int size, IntFunction valueFunction) { ... }

Stephen



On 14 October 2015 at 11:39, Stephen Colebourne  wrote:
> On 14 October 2015 at 10:38, Paul Sandoz  wrote:
>>> On 14 Oct 2015, at 06:18, Stuart Marks  wrote:
>>> I'm not entirely sure what to take from this. If it were clearly 
>>> exponential, we could say with confidence that above a certain threshold 
>>> there would be vanishingly little benefit adding more arguments. But since 
>>> the curve seems to flatten out, maybe this is pushing us to add more pairs 
>>> than we had originally thought. The current draft API has 8 pairs; that 
>>> seems to leave a few percent of cases on the table. Obviously we can't get 
>>> to 100%, but is 97% good enough?
>
> I'd say 5 is definitely too little, without an easy builder fallback
> (as Guava provides). I'd say the right number is between 8 and 10.
>
> Stephen


Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-10-15 Thread Peter Levart



On 10/15/2015 03:21 PM, Roger Riggs wrote:

Peter, Chris,

Plausible but getting complicated as Chris observed.

We can be on the lookout for specific cases in the JDK like that. I 
expect most

can be resolved by specific cooperation between the super/subclasses.


How? Such "cooperation" can not use the tracked object's instance 
methods as the instance is gone when cleanup happens, so no overriding 
is possible. Can you rewrite this example:


class SuperClass {
protected final Cleanup cleanup = 
XXX.getCleaner().phantomCleanup(this);


SuperClass() {
cleanup.append(() -> {... super class cleanup ...});
}
}

class SubClass extends SuperClass {
SubClass() {
super();
cleanup.prepend(() -> {... pre-actions ...})
.append(() -> {... post-actions ...});
}
}


...using just current simple Cleaner API?

I think that if we design an API to replace finalize(), then it should 
support use cases that are possible with finalize(). Not just use cases 
in the JDK itself, but also elsewhere on the planet.


The cleanup of each class should *only* be doing cleanup for their own 
class.
Because it is invoked after finalization, there can be *no* access to 
the instance
being cleaned.  Any shared state will be duplicated between the two 
cleanable behaviors.
If the subclass shares state with the superclass (protected fields or 
references)

then the cleanup needs to be co-designed.


Even if state needed for clean-up is duplicated (it can just be doubly 
referenced, for example: superclass establishes state and exposes it to 
subclass that just references it), registering separate Cleanup 
instances per super/subclass means that you can not control the order of 
them fire-ing. If for example, the subclass cleanup logic needs the 
superclass still be fully functional, it can do that now by:


@Override protected void finalize() {
... subclass cleanup ... // flushing something - needs 
superclass functionality

super.finalize();
}

With separate Cleanable(s) for super/subclass, the order is not guaranteed.



Canceling cleanup is easier, because the object has not been finalized.
If such a function is needed, it should be part of the API of the 
superclass

and part of the contract.


Ideally such methods would be in superclass only and be final and just 
delegate to Cleanable.clear() or Cleanable.clean(). The Cleanable should 
be the only place where the cleanup logic is encapsulated.




Also, I've seen a few calls to super.finalize() where there were no 
finalizers
in any of the superclasses.  It would be considered good design to 
always include it.

I don't know if the optimization for empty finalize methods includes the
case where it only calls super.finalize().


Ah, I forgot that empty finalize() is optimized away (the Object is not 
even registered for finalization).


Regards, Peter



Roger

On 10/15/15 7:43 AM, Chris Hegarty wrote:

Peter,

On 15 Oct 2015, at 09:12, Peter Levart  wrote:


On 10/14/2015 07:43 PM, Roger Riggs wrote:

Hi Alan, Mandy,

I looked at a few of the many uses of finalize and the likely changes.
The zip Inflater and Deflater are relatively simple cases.
Some finalizers are not used and can be removed.
The sun.net.www.MeteredStream example subclasses PhantomCleanable 
to add the state and cleanup

behavior.

http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/

Some of the harder cases will take more time to disentangle the 
cleanup code.
For example, ZipFile, and FileIn/OutputStream (Peter has prototyped 
this).


Roger

Hi Roger,

It's good to see some actual uses of the API and how it is supposed 
to be used in migration from finalize() methods. I think empty 
protected finalize() method is almost always safe to remove. If the 
class is not subclassed, it serves no purpose (unless some other 
same-package class or itself is calling it, which can be checked and 
those calls removed). If subclass overrides finalize() and calls 
super.finalize(), its ok (it will call Object.finalize then when 
empty finalize() is removed). The same holds if a subclass calls 
finalize() as a virtual method regardless of whether it also 
overrides it or not.


One thing to watch for is in case a subclass overrides finalize() 
like this:


class Subclass extends Superclass {
...
@Override protected finalize() {
 pre-actions ...
super.finalize();
... post-actions...
}

... where the order of cleanup actions has to be orchestrated 
between super and subclass. Having a PhantomCleanable replace the 
finalize() in a superclass has a similar effect as the following 
re-ordering in subclass:


@Override protected finalize() {
 pre-actions ...
... post-actions...
super.finalize();
}

...since finalization is performed before PhantomReference is 
enqueued. This re-ordering is luckily often safe as post-actions 
usually can't use superclass resources any more and usually don't 
depend on the state

Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-10-15 Thread Chris Hegarty

On 15 Oct 2015, at 14:21, Roger Riggs  wrote:

> ...
> 
> Also, I've seen a few calls to super.finalize() where there were no finalizers
> in any of the superclasses.  It would be considered good design to always 
> include it.
> I don't know if the optimization for empty finalize methods includes the
> case where it only calls super.finalize().

I believe it is only for empty finalize methods.

http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/tip/src/share/vm/classfile/classFileParser.cpp#l2506
http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/tip/src/share/vm/classfile/classFileParser.cpp#l4509

-Chris.



Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-10-15 Thread Roger Riggs

Peter, Chris,

Plausible but getting complicated as Chris observed.

We can be on the lookout for specific cases in the JDK like that.  I 
expect most

can be resolved by specific cooperation between the super/subclasses.
The cleanup of each class should *only* be doing cleanup for their own 
class.
Because it is invoked after finalization, there can be *no* access to 
the instance
being cleaned.  Any shared state will be duplicated between the two 
cleanable behaviors.
If the subclass shares state with the superclass (protected fields or 
references)

then the cleanup needs to be co-designed.

Canceling cleanup is easier, because the object has not been finalized.
If such a function is needed, it should be part of the API of the superclass
and part of the contract.

Also, I've seen a few calls to super.finalize() where there were no 
finalizers
in any of the superclasses.  It would be considered good design to 
always include it.

I don't know if the optimization for empty finalize methods includes the
case where it only calls super.finalize().

Roger

On 10/15/15 7:43 AM, Chris Hegarty wrote:

Peter,

On 15 Oct 2015, at 09:12, Peter Levart  wrote:


On 10/14/2015 07:43 PM, Roger Riggs wrote:

Hi Alan, Mandy,

I looked at a few of the many uses of finalize and the likely changes.
The zip Inflater and Deflater are relatively simple cases.
Some finalizers are not used and can be removed.
The sun.net.www.MeteredStream example subclasses PhantomCleanable to add the 
state and cleanup
behavior.

http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/

Some of the harder cases will take more time to disentangle the cleanup code.
For example, ZipFile, and FileIn/OutputStream (Peter has prototyped this).

Roger

Hi Roger,

It's good to see some actual uses of the API and how it is supposed to be used 
in migration from finalize() methods. I think empty protected finalize() method 
is almost always safe to remove. If the class is not subclassed, it serves no 
purpose (unless some other same-package class or itself is calling it, which 
can be checked and those calls removed). If subclass overrides finalize() and 
calls super.finalize(), its ok (it will call Object.finalize then when empty 
finalize() is removed). The same holds if a subclass calls finalize() as a 
virtual method regardless of whether it also overrides it or not.

One thing to watch for is in case a subclass overrides finalize() like this:

class Subclass extends Superclass {
...
@Override protected finalize() {
 pre-actions ...
super.finalize();
... post-actions...
}

... where the order of cleanup actions has to be orchestrated between super and 
subclass. Having a PhantomCleanable replace the finalize() in a superclass has 
a similar effect as the following re-ordering in subclass:

@Override protected finalize() {
 pre-actions ...
... post-actions...
super.finalize();
}

...since finalization is performed before PhantomReference is enqueued. This 
re-ordering is luckily often safe as post-actions usually can't use superclass 
resources any more and usually don't depend on the state of superclass. In 
addition, when superclass actions do happen, they can't invoke any instance 
methods if they are refactored to use Cleaner.

This brings up an interesting question. finalize() method allows subclasses to 
override it and augment cleanup logic to include any state changes or resources 
used by subclass.

Or for a subclass to effectively cancel any clean up, by
providing an empty finalize() method. Which I think is
also supported by your proposal, or at least a side-effect
of having the Cleanup as a protected field ( you can call
clear on it, right? ).

Having the Cleanup as a protected field looks a little odd,
but no more so than the public/protected finalize method.

This is now getting even more complicated. There are
potentially multiple object references being tracked as
part of the cleanup of a single “significant” object ?

-Chris.


How about Cleanup API? Subclass can register it's own Cleanable for own 
resources, but order of execution of superclass and subclass Cleanable(s) is 
arbitrary then. Cleanables will often be established in constructors and 
super/subclass constructors have a defined order of execution. So what about 
the following:

public class Cleaner {

public Cleanup phantomCleanup(Object referent);

public interface Cleanable {
void clean();
void clear();
}

public interface Cleanup extends Cleanable {
Cleanable append(Runnable action);
Cleanable prepend(Runnable action);
}

public static abstract class PhantomCleanable extends PhantomReference 
implements Cleanable { ... }

private static final class PhantomCleanup extends PhantomCleanable implements 
Cleanup { ... }

...use...

class SuperClass {
protected final Cleanup cleanup = XXX.getCleaner().phantomCleanup(this);

SuperClass() {
cleanup.append(() -> {... super class cleanup ...});
}
}

class 

Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-10-15 Thread Roger Riggs

Hi Chris,

I think this warrants some discussion related to the normal deprecation 
practice.


If there is specified behavior related to finalize, then the behavior 
has to be replicated
using the Cleaner to ensure backward compatibility.  It is a spec change 
(at least a
documentation change) and may move from the finalize method to the class 
javadoc.


Since finalize is an overridden method, removing a particular override 
does not
remove the method.  There is no issue with source compatibility. I'll 
need to

check binary compatibility that the VM when it sees a call to a specific
finalize method that it will search up for the super method.  I think 
this is the case.


If it is decided that the finalize method overrides need to follow the 
standard

2 release deprecation cycle there is not much of a down side.  There is an
optimization in the VM that looks for empty finalize methods.  Only if a 
type
has at least one non-empty finalize method does the overhead for a 
finalizable

reference kick in.

yes, finalize methods in non-public classes can be removed.

BTW, I expect there are cases where it is not practical or urgent to replace
the finalizers and they will stay the same, perhaps indefinitely.

Roger


On 10/15/15 6:47 AM, Chris Hegarty wrote:

Roger,

On 14 Oct 2015, at 18:43, Roger Riggs  wrote:


Hi Alan, Mandy,

I looked at a few of the many uses of finalize and the likely changes.
The zip Inflater and Deflater are relatively simple cases.
Some finalizers are not used and can be removed.

It is not immediately clear to me. Are you saying that some
finalize() methods, like the ones on Inflater and Deflater,
are part of Java SE spec and should be deprecated in 9,
then removed in 10. While others, that are just
implementation, can be removed now?

-Chris.


The sun.net.www.MeteredStream example subclasses PhantomCleanable to add the 
state and cleanup
behavior.

http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/

Some of the harder cases will take more time to disentangle the cleanup code.
For example, ZipFile, and FileIn/OutputStream (Peter has prototyped this).

Roger



On 10/14/2015 10:23 AM, Alan Bateman wrote:

On 14/10/2015 15:03, Roger Riggs wrote:

Hi Alan,

So any user of the Cleaner can take advantage of the mechanism, for example in 
a different package or module.
For example, Netbeans.

Cleaner + Cleanable need to be public of course so maybe we should wait for the 
examples that extend WeakCleanableRef or cast the Cleanable to a 
WeakCleanableRef before seeing if this is the right thing or not.

-Alan




Re: Array equality, comparison and mismatch

2015-10-15 Thread Andrew Haley
On 10/15/2015 11:52 AM, Paul Sandoz wrote:
> I would be very interested in your opinion on being able to make
> intrinsic on ARM the method ArraysSupport.vectorizedMismatch [1].

Pretty easy, I'm sure.  We already have MacroAssembler::string_compare
which does essentially the same thing but with char arrays.

Andrew.


Re: RFR (xs) : 8038502: Deflater.needsInput() should use synchronization

2015-10-15 Thread Chris Hegarty
Looks good to me Sean. All other state seems to be accessed when holding zsRef.

-Chris.

On 15 Oct 2015, at 10:02, Seán Coffey  wrote:

> bug report : https://bugs.openjdk.java.net/browse/JDK-8038502
> 
> The len instance variable should be read/written while holding the zsRef lock.
> 
> needsInput() seems to be missing that. Simple change :
> 
> diff --git a/src/java.base/share/classes/java/util/zip/Deflater.java 
> b/src/java.base/share/classes/java/util/zip/Deflater.java
> --- a/src/java.base/share/classes/java/util/zip/Deflater.java
> +++ b/src/java.base/share/classes/java/util/zip/Deflater.java
> @@ -318,8 +318,10 @@
>  * should be called in order to provide more input
>  */
> public boolean needsInput() {
> +synchronized (zsRef) {
> return len <= 0;
> }
> +}
> 
> /**
>  * When called, indicates that compression should end with the current
> 
> -- 
> Regards,
> Sean.
> 



Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-10-15 Thread Chris Hegarty
Peter,

On 15 Oct 2015, at 09:12, Peter Levart  wrote:

> On 10/14/2015 07:43 PM, Roger Riggs wrote:
>> Hi Alan, Mandy,
>> 
>> I looked at a few of the many uses of finalize and the likely changes.
>> The zip Inflater and Deflater are relatively simple cases.
>> Some finalizers are not used and can be removed.
>> The sun.net.www.MeteredStream example subclasses PhantomCleanable to add the 
>> state and cleanup
>> behavior.
>> 
>> http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/
>> 
>> Some of the harder cases will take more time to disentangle the cleanup code.
>> For example, ZipFile, and FileIn/OutputStream (Peter has prototyped this).
>> 
>> Roger
> 
> Hi Roger,
> 
> It's good to see some actual uses of the API and how it is supposed to be 
> used in migration from finalize() methods. I think empty protected finalize() 
> method is almost always safe to remove. If the class is not subclassed, it 
> serves no purpose (unless some other same-package class or itself is calling 
> it, which can be checked and those calls removed). If subclass overrides 
> finalize() and calls super.finalize(), its ok (it will call Object.finalize 
> then when empty finalize() is removed). The same holds if a subclass calls 
> finalize() as a virtual method regardless of whether it also overrides it or 
> not.
> 
> One thing to watch for is in case a subclass overrides finalize() like this:
> 
> class Subclass extends Superclass {
> ...
> @Override protected finalize() {
> pre-actions ...
>super.finalize();
>... post-actions...
> }
> 
> ... where the order of cleanup actions has to be orchestrated between super 
> and subclass. Having a PhantomCleanable replace the finalize() in a 
> superclass has a similar effect as the following re-ordering in subclass:
> 
> @Override protected finalize() {
> pre-actions ...
>... post-actions...
>super.finalize();
> }
> 
> ...since finalization is performed before PhantomReference is enqueued. This 
> re-ordering is luckily often safe as post-actions usually can't use 
> superclass resources any more and usually don't depend on the state of 
> superclass. In addition, when superclass actions do happen, they can't invoke 
> any instance methods if they are refactored to use Cleaner.
> 
> This brings up an interesting question. finalize() method allows subclasses 
> to override it and augment cleanup logic to include any state changes or 
> resources used by subclass.

Or for a subclass to effectively cancel any clean up, by
providing an empty finalize() method. Which I think is
also supported by your proposal, or at least a side-effect
of having the Cleanup as a protected field ( you can call
clear on it, right? ).

Having the Cleanup as a protected field looks a little odd,
but no more so than the public/protected finalize method.

This is now getting even more complicated. There are
potentially multiple object references being tracked as 
part of the cleanup of a single “significant” object ?

-Chris.

> How about Cleanup API? Subclass can register it's own Cleanable for own 
> resources, but order of execution of superclass and subclass Cleanable(s) is 
> arbitrary then. Cleanables will often be established in constructors and 
> super/subclass constructors have a defined order of execution. So what about 
> the following:
> 
> public class Cleaner {
> 
> public Cleanup phantomCleanup(Object referent);
> 
> public interface Cleanable {
>void clean();
>void clear();
> }
> 
> public interface Cleanup extends Cleanable {
>Cleanable append(Runnable action);
>Cleanable prepend(Runnable action);
> }
> 
> public static abstract class PhantomCleanable extends PhantomReference 
> implements Cleanable { ... }
> 
> private static final class PhantomCleanup extends PhantomCleanable implements 
> Cleanup { ... }
> 
> ...use...
> 
> class SuperClass {
>protected final Cleanup cleanup = XXX.getCleaner().phantomCleanup(this);
> 
>SuperClass() {
>cleanup.append(() -> {... super class cleanup ...});
>}
> }
> 
> class SubClass extends SuperClass {
>SubClass() {
>super();
>cleanup.prepend(() -> {... pre-actions ...})
>.append(() -> {... post-actions ...});
>}
> }
> 
> 
> Regards, Peter
> 
>> 
>> 
>> 
>> On 10/14/2015 10:23 AM, Alan Bateman wrote:
>>> 
>>> On 14/10/2015 15:03, Roger Riggs wrote:
 Hi Alan,
 
 So any user of the Cleaner can take advantage of the mechanism, for 
 example in a different package or module.
 For example, Netbeans.
>>> Cleaner + Cleanable need to be public of course so maybe we should wait for 
>>> the examples that extend WeakCleanableRef or cast the Cleanable to a 
>>> WeakCleanableRef before seeing if this is the right thing or not.
>>> 
>>> -Alan
>> 
> 



Re: Array equality, comparison and mismatch

2015-10-15 Thread Paul Sandoz

> On 13 Oct 2015, at 12:03, Andrew Haley  wrote:
> 
> On 13/10/15 10:22, Paul Sandoz wrote:
>> Analysis so far indicate big gains are to be had on larger arrays with 
>> better or no impact on small arrays if i do the following instead:
>> 
>>  if (Double.doubleToRawLongBits(a[i]) !=
>>  Double.doubleToRawLongBits(b[i])) {
>>  int c = Double.compare(a[i], b[i]);
>>  if (c != 0) return c;
>>  }
> 
> I was about to make a similar comment.  My experiment was with
> 
>if (Double.doubleToRawLongBits(a[i]) != 
> Double.doubleToRawLongBits(b[i])
>&& (Double.doubleToLongBits(a[i]) !=
>Double.doubleToLongBits(b[i])))
>return Double.compare(a[i], b[i]);
>}
> 
> which is about twice as fast as the original version, as is yours.
> But yours is more elegant.  :-)
> 

Thanks :-) updated:

  
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8033148-Arrays-lexico-compare/webrev/


> It's a shame that HotSpot doesn't see through the load of a
> double and then the conversion through doubleToRawLongBits:
> it could just load directly into the integer registers.
> 

The following webrev explicitly does what you mention above and is faster for 
mismatching double arrays (with no differing NaN values):

  
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8136924-arrays-mismatch-vectorized-unsafe/webrev/

I would be very interested in your opinion on being able to make intrinsic on 
ARM the method ArraysSupport.vectorizedMismatch [1].

Paul.

[1] 
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8136924-arrays-mismatch-vectorized-unsafe/webrev/src/java.base/share/classes/java/util/ArraysSupport.java.html


Re: RFR: JDK-8046565: Platform Logger API and Service

2015-10-15 Thread Stephen Colebourne
On 14 October 2015 at 12:20, Daniel Fuchs  wrote:
> On 14/10/15 07:21, Mandy Chung wrote:
>> Several log methods throws NPE if the level is legible and the parameter
>> is null.  E.g.
>> * @throws NullPointerException if {@code level} is {@code null}, or if the
>> level
>> * is loggable and {@code msgSupplier} is {@code null}.
>>
>> Should it throw NPE if the input parameter is null regardless of whether
>> the level is loggable?
>
>
> Yes probably. The reason it is like that is that it mimics
> what java.util.logging.Logger does (at least for message suppliers), but
> it is probably not a good idea to propagate this bad practice.
>
> I will change System.Logger spec to mandate to always throw NPE
> if Supplier is null - and ensure that the implementations
> we have follow the spec.

I believe the desire in logging is to a sensible degree write
defensively to avoid getting an exception due to logging which
obscures the actual error.

Thus, log(level, msg) should accept a null message (but not a null level)
But, log(level, supplier) should not accept a null supplier or level


The Javadoc for 'System.Logger' starts with "The minimum set of
methods that a logger returned by..." which isn't a good description
of the interface.


I have a major concern that the class names 'Logger' and 'Level'
duplicate those of java.util.logging. While they are inner classes as
opposed to top level classes, both IntelliJ and Eclipse will find the
inner class and top level class when typing "Logger". This will no
doubt cause many users to import the wrong one. I propose that these
classes are renamed to avoid this problem. The simplest would be to
change them from inner classes to top level classes "System.Logger" ->
"SystemLogger". Alternatively, they could stay as inner classes and be
prefixed "System.Logger" -> "System.SysLogger" or "System.Logger" ->
"System.BasicLogger".

Stephen


Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-10-15 Thread Chris Hegarty
Roger,

On 14 Oct 2015, at 18:43, Roger Riggs  wrote:

> Hi Alan, Mandy,
> 
> I looked at a few of the many uses of finalize and the likely changes.
> The zip Inflater and Deflater are relatively simple cases.
> Some finalizers are not used and can be removed.

It is not immediately clear to me. Are you saying that some
finalize() methods, like the ones on Inflater and Deflater,
are part of Java SE spec and should be deprecated in 9,
then removed in 10. While others, that are just
implementation, can be removed now?

-Chris.

> The sun.net.www.MeteredStream example subclasses PhantomCleanable to add the 
> state and cleanup
> behavior.
> 
> http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/
> 
> Some of the harder cases will take more time to disentangle the cleanup code.
> For example, ZipFile, and FileIn/OutputStream (Peter has prototyped this).
> 
> Roger
> 
> 
> 
> On 10/14/2015 10:23 AM, Alan Bateman wrote:
>> 
>> On 14/10/2015 15:03, Roger Riggs wrote:
>>> Hi Alan,
>>> 
>>> So any user of the Cleaner can take advantage of the mechanism, for example 
>>> in a different package or module.
>>> For example, Netbeans.
>> Cleaner + Cleanable need to be public of course so maybe we should wait for 
>> the examples that extend WeakCleanableRef or cast the Cleanable to a 
>> WeakCleanableRef before seeing if this is the right thing or not.
>> 
>> -Alan
> 



RFR (xs) : 8038502: Deflater.needsInput() should use synchronization

2015-10-15 Thread Seán Coffey

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

The len instance variable should be read/written while holding the zsRef 
lock.


needsInput() seems to be missing that. Simple change :

diff --git a/src/java.base/share/classes/java/util/zip/Deflater.java 
b/src/java.base/share/classes/java/util/zip/Deflater.java

--- a/src/java.base/share/classes/java/util/zip/Deflater.java
+++ b/src/java.base/share/classes/java/util/zip/Deflater.java
@@ -318,8 +318,10 @@
  * should be called in order to provide more input
  */
 public boolean needsInput() {
+synchronized (zsRef) {
 return len <= 0;
 }
+}

 /**
  * When called, indicates that compression should end with the current

--
Regards,
Sean.



Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-15 Thread Paul Sandoz

> On 15 Oct 2015, at 05:00, Xueming Shen  wrote:
> 
> I'm not sure if it is a good idea, from performance perspective, to add a 
> "versionEntry" field into the JarEntry
> to support this feature, given most of the jar files might not be 
> multi-release-jar aware, and the Jar input&
> output streams dont work with a multi-release jar directly. Why should they 
> all pay a runtime price for it. If
> we really have to add an extra field, the JarFileEntry might be a better 
> place, and it might be desired to
> define a new subclass JarFileEntryMR to use when the MR is enabled, instead 
> of adding directly into the existing
> JarFileEntry.
> 

According to jol there is currently space available due to alignment. If there 
was not it would add about 4% in direct instance size. But the actual footprint 
is likely to be chunkier because of the string character storage for the name 
so the % increase in size would be smaller e.g. perhaps on average < 2% which 
might be ok given that i presume such entries are unlikely to be cached.

So i am not concerned about the size. If there was a way to design it to avoid 
modification of existing classes all the better, but i dunno if it is possible. 
Steve will surely know more.

Paul.






Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-10-15 Thread Peter Levart



On 10/14/2015 07:43 PM, Roger Riggs wrote:

Hi Alan, Mandy,

I looked at a few of the many uses of finalize and the likely changes.
The zip Inflater and Deflater are relatively simple cases.
Some finalizers are not used and can be removed.
The sun.net.www.MeteredStream example subclasses PhantomCleanable to 
add the state and cleanup

behavior.

http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/

Some of the harder cases will take more time to disentangle the 
cleanup code.
For example, ZipFile, and FileIn/OutputStream (Peter has prototyped 
this).


Roger


Hi Roger,

It's good to see some actual uses of the API and how it is supposed to 
be used in migration from finalize() methods. I think empty protected 
finalize() method is almost always safe to remove. If the class is not 
subclassed, it serves no purpose (unless some other same-package class 
or itself is calling it, which can be checked and those calls removed). 
If subclass overrides finalize() and calls super.finalize(), its ok (it 
will call Object.finalize then when empty finalize() is removed). The 
same holds if a subclass calls finalize() as a virtual method regardless 
of whether it also overrides it or not.


One thing to watch for is in case a subclass overrides finalize() like this:

class Subclass extends Superclass {
...
@Override protected finalize() {
 pre-actions ...
super.finalize();
... post-actions...
}

... where the order of cleanup actions has to be orchestrated between 
super and subclass. Having a PhantomCleanable replace the finalize() in 
a superclass has a similar effect as the following re-ordering in subclass:


@Override protected finalize() {
 pre-actions ...
... post-actions...
super.finalize();
}

...since finalization is performed before PhantomReference is enqueued. 
This re-ordering is luckily often safe as post-actions usually can't use 
superclass resources any more and usually don't depend on the state of 
superclass. In addition, when superclass actions do happen, they can't 
invoke any instance methods if they are refactored to use Cleaner.


This brings up an interesting question. finalize() method allows 
subclasses to override it and augment cleanup logic to include any state 
changes or resources used by subclass. How about Cleanup API? Subclass 
can register it's own Cleanable for own resources, but order of 
execution of superclass and subclass Cleanable(s) is arbitrary then. 
Cleanables will often be established in constructors and super/subclass 
constructors have a defined order of execution. So what about the following:


public class Cleaner {

public Cleanup phantomCleanup(Object referent);

public interface Cleanable {
void clean();
void clear();
}

public interface Cleanup extends Cleanable {
Cleanable append(Runnable action);
Cleanable prepend(Runnable action);
}

public static abstract class PhantomCleanable extends PhantomReference 
implements Cleanable { ... }


private static final class PhantomCleanup extends PhantomCleanable 
implements Cleanup { ... }


...use...

class SuperClass {
protected final Cleanup cleanup = 
XXX.getCleaner().phantomCleanup(this);


SuperClass() {
cleanup.append(() -> {... super class cleanup ...});
}
}

class SubClass extends SuperClass {
SubClass() {
super();
cleanup.prepend(() -> {... pre-actions ...})
.append(() -> {... post-actions ...});
}
}


Regards, Peter





On 10/14/2015 10:23 AM, Alan Bateman wrote:


On 14/10/2015 15:03, Roger Riggs wrote:

Hi Alan,

So any user of the Cleaner can take advantage of the mechanism, for 
example in a different package or module.

For example, Netbeans.
Cleaner + Cleanable need to be public of course so maybe we should 
wait for the examples that extend WeakCleanableRef or cast the 
Cleanable to a WeakCleanableRef before seeing if this is the right 
thing or not.


-Alan






RFR (JAXP) : 8081248: Implement JEP 268: XML Catalog API

2015-10-15 Thread huizhe wang

Hi all,

Please help review JEP 268 implementation:
https://bugs.openjdk.java.net/browse/JDK-8081248

Catalog specification:
https://www.oasis-open.org/committees/download.php/14809/xml-catalogs.html

webrevs:
http://cr.openjdk.java.net/~joehw/jdk9/8081248/webrev/


A Catalog is basically an ordered list of entries that map external 
references to local resources using string identifiers called 
systemId/publicId. The main function of the Catalog API therefore is 1) 
parse a catalog; 2) use it to find a matching entry and resolve the 
resource referenced in an XML document.


For common use cases, an application would simply acquire a 
CatalogResolver and set it on a parser to be used for resource 
resolution, refer to the test for an example.


The unit test contains test cases for the simple entries. Since SQE test 
development has been in parallel with the dev work, this unit test 
didn't have to cover all of the functions. What it does is to test and 
demonstrate the use in a real environment, using a CatalogResolver to 
actually resolves resources in XML documents, while the SQE tests focus 
on having a full coverage by matching literal strings without involving 
XML documents.  The SQE test suite has gone through internal reviews and 
will start public review following this review.


Thanks,
Joe