Re: Fw: Generics in enums

2013-05-29 Thread Joe Darcy

Hello Victor,

On 5/29/2013 11:25 PM, Victor Polischuk wrote:

Greetings,

I beg pardon for the previous HTML mail.

Some time ago I wanted to migrate our "commons-lang" enums to "java 5" 
enumerations, but I encountered an issue that it cannot be done without discarding generics since 
java enums do not support them. Let me show an example:

//--
public final class ColorEnum extends 
org.apache.commons.lang.enums.Enum {
 public static final ColorEnum RED = new 
ColorEnum("Red");
 public static final ColorEnum GREEN = new 
ColorEnum("Green");
 public static final ColorEnum�BLUE = new 
ColorEnum("Blue");
 public static final ColorEnum WHITE = new 
ColorEnum("White") {
 @Override
 public MixedPixel make() {...}�
 };
 
 private ColorEnum(String color) {super(color);}
 
 public boolean filter(T pixel) {...}
 
 public T make() {...}

}
//--

And I wonder if there is a specific reason why I cannot convert it into 
something like:�

//--
public enum Color {
 RED("Red"),
 GREEN("Green"),
 BLUE("Blue"),
 WHITE("White") {
 @Override
 public MixedPixel make() {...}�
 };

 private Color(String color) {...}

 public boolean filter(T pixel) {...}

 public T make() {...}
}
//--

Thank you in advance.

Sincerely yours,
Victor Polischuk


You can approximate this effect by having your enum implement an 
interface or even a generic interface. For some examples in the JDK see, 
javax.tools.StandardLocation and java.nio.file.LinkOption.


HTH,

-Joe


Re: PING! - RFR 7032154: Performance tuning of sun.misc.FloatingDecimal/FormattedFloatingDecimal

2013-05-29 Thread Martin Buchholz
On Wed, May 29, 2013 at 5:20 PM, Brian Burkhalter <
brian.burkhal...@oracle.com> wrote:

> On May 29, 2013, at 4:57 PM, Martin Buchholz wrote:
>
>
> Use third person in the first sentence of a javadoc - e.g.
> s/Retrieve/Retrieves/.
>
> ---
>
> Don't use the denigrated C style
> char zero[] =
> Instead use
> char[] zero =
>
> ---
>
>
> Are the two preceding comments generic, i.e., occurring in multiple
> places, or each unique?
>
>
Mutliple.


Fw: Generics in enums

2013-05-29 Thread Victor Polischuk
Greetings,

I beg pardon for the previous HTML mail.

Some time ago I wanted to migrate our "commons-lang" enums to "java 5" 
enumerations, but I encountered an issue that it cannot be done without 
discarding generics since java enums do not support them. Let me show an 
example:

//--
public final class ColorEnum extends 
org.apache.commons.lang.enums.Enum {
public static final ColorEnum RED = new 
ColorEnum("Red");
public static final ColorEnum GREEN = new 
ColorEnum("Green");
public static final ColorEnum�BLUE = new 
ColorEnum("Blue");
public static final ColorEnum WHITE = new 
ColorEnum("White") {
@Override
public MixedPixel make() {...}�
};

private ColorEnum(String color) {super(color);}

public boolean filter(T pixel) {...}

public T make() {...}
}
//--

And I wonder if there is a specific reason why I cannot convert it into 
something like:�

//--
public enum Color {
RED("Red"),
GREEN("Green"),
BLUE("Blue"),
WHITE("White") {
@Override
public MixedPixel make() {...}�
};

private Color(String color) {...}

public boolean filter(T pixel) {...}

public T make() {...}
}
//--

Thank you in advance.

Sincerely yours,
Victor Polischuk


RFR: JDK-8015628 - Test Failure in closed/java/io/pathNames/GeneralSolaris.java

2013-05-29 Thread Dan Xu

Hi All,

The fix to JDK-8009258 solves the failure in GeneralWin32.java. But the 
changes in Line 311, 312 in checkNames() method of General.java file, 
lead to the new failure of GeneralSolaris.java testcase. Because it 
changes the implicit value of an empty "ask" parameter.


This fix is to revert the relevant changes of the above two lines in 
General.java. And accordingly, it accommodates the code in 
GeneralWin32.java to the original General.checkNames() method.


webrev: http://cr.openjdk.java.net/~dxu/8015628/webrev.00/

Thanks for your review!

-Dan


hg: jdk8/tl/jdk: 4759491: method ZipEntry.setTime(long) works incorrectly; ...

2013-05-29 Thread xueming . shen
Changeset: 90df6756406f
Author:sherman
Date:  2013-05-29 19:50 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/90df6756406f

4759491: method ZipEntry.setTime(long) works incorrectly
6303183: Support NTFS and Unix-style timestamps for entries in Zip files
7012856: (zipfs) Newly created entry in zip file system should set all file 
times non-null values.
7012868: (zipfs) file times of entry in zipfs should always be the same 
regardless of TimeZone.
Summary: to add suuport of Info-ZIP extended timestamp in extra data fields
Reviewed-by: martin, alanb

! src/share/classes/java/util/zip/ZipConstants.java
! src/share/classes/java/util/zip/ZipEntry.java
! src/share/classes/java/util/zip/ZipFile.java
! src/share/classes/java/util/zip/ZipInputStream.java
! src/share/classes/java/util/zip/ZipOutputStream.java
+ src/share/classes/java/util/zip/ZipUtils.java
! src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipFileSystem.java
! src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipInfo.java
! test/demo/zipfs/ZipFSTester.java
! test/demo/zipfs/basic.sh
! test/java/util/jar/TestExtra.java
! test/java/util/zip/StoredCRC.java
+ test/java/util/zip/TestExtraTime.java
! test/java/util/zip/ZipFile/Assortment.java



Re: RFR 8015395: NumberFormatException during startup if java.lang.Integer.IntegerCache.high set to bad value

2013-05-29 Thread Mandy Chung


On 5/29/2013 3:44 PM, Brian Burkhalter wrote:

On May 29, 2013, at 2:35 PM, Alan Bateman wrote:


It would be good to do a few experiments with -XX:AutoBoxCacheMax= to 
make sure that bad values dp startup to fail, I expect they should.

Yes, bad values do indeed cause startup to fail, for example:

$ java -XX:AutoBoxCacheMax=1024\-Xmx2g HelloWorld
Improperly specified VM option 'AutoBoxCacheMax=1024-Xmx2g'
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.


AutoBoxCacheMax is a signed integer argument and I think the VM will 
check if it contains any non-digit character but it doesn't validate for 
the range.  This could cause the VM to throw OOM:


java -XX:AutoBoxCacheMax=888
Error occurred during initialization of VM
java.lang.OutOfMemoryError: Java heap space
at java.lang.Integer$IntegerCache.(Integer.java:780)
at java.lang.Integer.valueOf(Integer.java:808)
at sun.misc.Signal.handle(Signal.java:169)
at java.lang.Terminator.setup(Terminator.java:60)
at java.lang.System.initializeSystemClass(System.java:1192)

The VM might truncate the specified AutoBoxCacheMax value as INTX_FORMAT
when it passes to the library:

   // Feed the cache size setting into the JDK
   char buffer[1024];
   sprintf(buffer, "java.lang.Integer.IntegerCache.high=" INTX_FORMAT, 
AutoBoxCacheMax);

   add_property(buffer);

Probably best to file a bug too.


As regards setting the property directly (which was never the intention and is 
not supported) then the issue is that the property value is being parsed 
lazily. If you want to check it early then it requires parsing it in 
VM.saveAndRemoveProperties, that is the method that is called early in the 
initialization to stash away these properties.

Whether this property is parsed early or lazily it seems like the fundamental 
question remains: given a non-parseable value does one fall back to the default 
or throw an exception and prevent VM startup?


Throwing an exception is a better solution that will make it easier to 
diagnose the problem.


Mandy



Re: PING! - RFR 7032154: Performance tuning of sun.misc.FloatingDecimal/FormattedFloatingDecimal

2013-05-29 Thread Brian Burkhalter
On May 29, 2013, at 4:57 PM, Martin Buchholz wrote:

> I would like to see the real world-class experts on this scary math stuff 
> (Alan Eliasen, Tim Buktu) be made honorary jdk reviewers, just for their area 
> of specialization, if that was bureaucratically possible.

I'll leave serious comments on this topic to those vested with godlike powers. 
It would be nice however to have domain experts here, that's for sure.

> Shouldn't the original authors be cc'ed?

I think (hope) all are on core-libs-dev.

> The code is awesome.  My thorough review found only these defects:
> 
> Use third person in the first sentence of a javadoc - e.g. 
> s/Retrieve/Retrieves/.
> 
> ---
> 
> Don't use the denigrated C style
> char zero[] =
> Instead use 
> char[] zero =
> 
> ---

Are the two preceding comments generic, i.e., occurring in multiple places, or 
each unique?

> This looks odd:
> normalized as a//binary*
> 
> The original comment looked more normal:
> 
> normalized as a *binary*
> ---
> 
> Otherwise, approved!

Thanks much!

Brian

> 
> 
> On Fri, May 24, 2013 at 10:09 AM, Brian Burkhalter 
>  wrote:
> Originally posted one month ago today.
> 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-April/016355.html
> 
> https://www.youtube.com/watch?v=D9kv_V5lhiE
> 
> Thanks,
> 
> Brian
> 



Re: PING! - RFR 7032154: Performance tuning of sun.misc.FloatingDecimal/FormattedFloatingDecimal

2013-05-29 Thread Martin Buchholz
I would like to see the real world-class experts on this scary math stuff
(Alan Eliasen, Tim Buktu) be made honorary jdk reviewers, just for their
area of specialization, if that was bureaucratically possible.  Shouldn't
the original authors be cc'ed?

The code is awesome.  My thorough review found only these defects:

Use third person in the first sentence of a javadoc - e.g.
s/Retrieve/Retrieves/.

---

Don't use the denigrated C style
char zero[] =
Instead use
char[] zero =

---
This looks odd:

normalized as a//binary*


The original comment looked more normal:

normalized as a *binary*

---

Otherwise, approved!



On Fri, May 24, 2013 at 10:09 AM, Brian Burkhalter <
brian.burkhal...@oracle.com> wrote:

> Originally posted one month ago today.
>
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-April/016355.html
>
> https://www.youtube.com/watch?v=D9kv_V5lhiE
>
> Thanks,
>
> Brian
>


Re: RFR 8015395: NumberFormatException during startup if java.lang.Integer.IntegerCache.high set to bad value

2013-05-29 Thread Remi Forax

On 05/30/2013 12:44 AM, Brian Burkhalter wrote:

On May 29, 2013, at 2:35 PM, Alan Bateman wrote:


It would be good to do a few experiments with -XX:AutoBoxCacheMax= to 
make sure that bad values dp startup to fail, I expect they should.

Yes, bad values do indeed cause startup to fail, for example:

$ java -XX:AutoBoxCacheMax=1024\-Xmx2g HelloWorld
Improperly specified VM option 'AutoBoxCacheMax=1024-Xmx2g'
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.


As regards setting the property directly (which was never the intention and is 
not supported) then the issue is that the property value is being parsed 
lazily. If you want to check it early then it requires parsing it in 
VM.saveAndRemoveProperties, that is the method that is called early in the 
initialization to stash away these properties.

Whether this property is parsed early or lazily it seems like the fundamental 
question remains: given a non-parseable value does one fall back to the default 
or throw an exception and prevent VM startup?


In Java, we are used too check arguments of a method and throws an 
exception if something is wrong,
As a user of a method or here of the VM, it's far easier when the VM 
stop otherwise you have to learn what is the default value that will be 
used or worst what is the "smart" algorithm that will be used to recover 
from this problem. That's not the good way to design 
method/class/sofware, they are and should stay dumb, the intelligence 
come from the way you connect the things, not from each piece.




Brian


cheers,
Rémi



hg: jdk8/tl/langtools: 8015641: genstubs needs to cope with static interface methods

2013-05-29 Thread jonathan . gibbons
Changeset: d685b12b62a4
Author:jjg
Date:  2013-05-29 15:34 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/d685b12b62a4

8015641: genstubs needs to cope with static interface methods
Reviewed-by: ksrini

! make/tools/genstubs/GenStubs.java



Re: RFR 8015395: NumberFormatException during startup if java.lang.Integer.IntegerCache.high set to bad value

2013-05-29 Thread Brian Burkhalter
On May 29, 2013, at 2:35 PM, Alan Bateman wrote:

> It would be good to do a few experiments with -XX:AutoBoxCacheMax= to 
> make sure that bad values dp startup to fail, I expect they should.

Yes, bad values do indeed cause startup to fail, for example:

$ java -XX:AutoBoxCacheMax=1024\-Xmx2g HelloWorld
Improperly specified VM option 'AutoBoxCacheMax=1024-Xmx2g'
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

> As regards setting the property directly (which was never the intention and 
> is not supported) then the issue is that the property value is being parsed 
> lazily. If you want to check it early then it requires parsing it in 
> VM.saveAndRemoveProperties, that is the method that is called early in the 
> initialization to stash away these properties.

Whether this property is parsed early or lazily it seems like the fundamental 
question remains: given a non-parseable value does one fall back to the default 
or throw an exception and prevent VM startup?

Brian

Re: RFR 6303183: Support NTFS and Unix-style timestamps for entries in Zip files

2013-05-29 Thread Martin Buchholz
On Wed, May 29, 2013 at 3:17 PM, Xueming Shen wrote:

> Hi Martin,
>
> Somehow most of your comments are "blank lines" in my mailbox, just wonder
> if
> you can send them again. Based on the "hints", I updated those javadoc
> comments
> in ZipUtils to "**".
>

Sorry, my mail program is a bit too innovative for old fogeys like me. Just
that annoying accidnetly typo was missed.

Looks OK!


Re: RFR 6303183: Support NTFS and Unix-style timestamps for entries in Zip files

2013-05-29 Thread Xueming Shen

Hi Martin,

Somehow most of your comments are "blank lines" in my mailbox, just wonder if
you can send them again. Based on the "hints", I updated those javadoc comments
in ZipUtils to "**".

The code has been updated slightly (mainly to not output the extra time stamp if
it already exists in the passed in/read in/user defined "extra' field. I added 
a test
TestExtraTime to test couple use scenario and also updated couple existing tests
(which depend on the hard coded extra field size...).

Now all unit/regression tests are passed.

http://cr.openjdk.java.net/~sherman/6303183/webrev/

As we discussed in other thread, I will continue to see the possibility of 
adding
atime/ctime support and better time granularity (via NTFS extra field).

Thanks!
-Sherman

On 05/28/2013 08:36 PM, Martin Buchholz wrote:

Looks good to me.

---
While you're moving code around, you could fix the typo below and change method 
comments to javadoc style ('*' => '**'')
accidnetly
and




Re: RFR 8005704: Update ConcurrentHashMap to v8

2013-05-29 Thread Martin Buchholz
On Wed, May 29, 2013 at 7:45 AM, Chris Hegarty wrote:

> decouple it from the other changes going on here. (Says me who wanted to
> resync, in one pass, the complete j.u.c last week!)
>

However it gets done, Chris, thanks very much for taking on the big task of
syncing jsr166.


Re: RFR 8015395: NumberFormatException during startup if java.lang.Integer.IntegerCache.high set to bad value

2013-05-29 Thread Alan Bateman

On 29/05/2013 20:54, Brian Burkhalter wrote:

So if an InternalError were to be used instead, the code would be like:

 try {
 int i = parseInt(integerCacheHighPropValue);
 i = Math.max(i, 127);
 // Maximum array size is Integer.MAX_VALUE
 h = Math.min(i, Integer.MAX_VALUE - (-low) -1);
 } catch(NumberFormatException nfe) {
 // Recast the exception to an InternalError with a 
sensible message.
 throw new InternalError(nfe.toString() +
 ": cannot parse value specified for 
java.lang.Integer.IntegerCache.high property");
 }

But that avoids the real question here, which is whether VM startup should be 
prevented by the value supplied for an internal property not being able to be 
parsed.

Brian

It would be good to do a few experiments with -XX:AutoBoxCacheMax= 
to make sure that bad values dp startup to fail, I expect they should.


As regards setting the property directly (which was never the intention 
and is not supported) then the issue is that the property value is being 
parsed lazily. If you want to check it early then it requires parsing it 
in VM.saveAndRemoveProperties, that is the method that is called early 
in the initialization to stash away these properties.


-Alan.


Re: RFR 8005698 : Handle Frequent HashMap Collisions with Balanced Trees

2013-05-29 Thread Brent Christian

Updated webrev is here:
http://cr.openjdk.java.net/~bchristi/8005698/webrev.03/

It contains the following changes from Mike's review:

* HashMap.comparableClassFor(): corrected reference to TreeBin docs

* fixed @run tag in InPlaceOpsCollisions.java

* Hashtable & HashMap: hashSeed made final, initialized in constructor, 
UNSAFE restored and used in readObject().


Thanks,
-Brent

On 5/29/13 11:39 AM, Brent Christian wrote:

On 5/28/13 9:13 PM, Mike Duigou wrote:

Hashtable/WeakHashMap::

- I assume that the JVM falls over dead if the security manager
doesn't allow access to the property, correct? I hadn't considered
what should happen in the event of a security exception when I
originally copied the GetPropertyAction idiom from elsewhere in the
JDK.


AFAIK a security exception won't happen here as it's called in a
doPrivileged().


Perhaps add a security manager test to CheckRandomHashSeed?
Or two if we want to make sure that


...that what? :)

I ran a test to confirm that maps can be created when useRandomSeed=true
and a security manager is running (-Djava.security.manager).  Attempting
to read the property from the test code throws an
AccessControlException, as one would expect.

CheckRandomHashSeed probably isn't the right place to test this, as the
test itself requires permissions to read the property and confirm (via
reflection) the value of the hash seed.  But I can add a new test case,
if you think it's important.


- initHashSeed could now return the value with assignment happening
in the constructor if we wanted to make hashSeed final. This would
mean the unsafe stuff would have to return in Hashtable/HashMap to
allow for deserialization.


It would be nice to keep hashSeed final.  It would no longer be lazy,
but we'll still get the main bottleneck relief of 8006593 by using
ThreadLocalRandom in sun.misc.Hashing.

I'll work on this.


HashMap::

- TreeBin.comparableClassFor() includes "see above for explanation."
but it's not clear where that explanation is.


Things got moved around - its referring to the TreeBin docs, which are
now "below".  Fixed.


Hashing::

- Do we know if ThreadLocalRandom requires that the VM be booted? It
may be possible to remove even more stuff here.


Indeed.  I don't know the answer.



InPlaceOpsCollisions::

- The @run directives run the wrong class (an error I have made
myself...).


Dang!  Thanks - fixed.


-Brent


On May 28 2013, at 13:03 , Brent Christian wrote: >

On 5/28/13 3:09 AM, Doug Lea wrote:


To better enable and simplify future improvements, could you
do this -- nest the tree classes within HashMap?


Done.


Also, a note on spliterators: I had added these in the
least disruptive way (knowing that major changes were coming)
by checking exact class match for HashMap.class. This is because
there aren't LinkedHashMap view classes to attach overrides to.
While not wrong, and OK for now, at some point this should be redone.


OK.  I will file a bug so this doesn't get forgotten.


I also applied the change to how HashMap.putAll() resizes the table
(to account for splitTreeBin() only handling doubling of tables).

The updated webrev is here:

http://cr.openjdk.java.net/~bchristi/8005698/webrev.02/

Thanks,
-Brent




Re: RFR 8015395: NumberFormatException during startup if java.lang.Integer.IntegerCache.high set to bad value

2013-05-29 Thread Brian Burkhalter
So if an InternalError were to be used instead, the code would be like:

try {
int i = parseInt(integerCacheHighPropValue);
i = Math.max(i, 127);
// Maximum array size is Integer.MAX_VALUE
h = Math.min(i, Integer.MAX_VALUE - (-low) -1);
} catch(NumberFormatException nfe) {
// Recast the exception to an InternalError with a sensible 
message.
throw new InternalError(nfe.toString() +
": cannot parse value specified for 
java.lang.Integer.IntegerCache.high property");
}

But that avoids the real question here, which is whether VM startup should be 
prevented by the value supplied for an internal property not being able to be 
parsed.

Brian

On May 29, 2013, at 11:49 AM, Brian Burkhalter wrote:

> Problem is that the String in the NumberFormatException in this case does not 
> contain the reason, only the offending String. The parseInt() would need to 
> be changed accordingly to make this intelligible.
> 
> I have no strong preference but it does not seem to me that failing to start 
> the VM is a great plan.



Re: RFR (S) CR 8014966: Add the proper Javadoc to @Contended

2013-05-29 Thread Martin Buchholz
Looks good enough to me for a non-public API.  I expect more contentious
arguments if this is ever made public, since the problems remain very
tricky.  Only a VM that monitors memory "hot spots" can really get this
right ... but the VM we have is already named "hotspot" ...


Re: RFR 8015395: NumberFormatException during startup if java.lang.Integer.IntegerCache.high set to bad value

2013-05-29 Thread Alan Bateman

On 29/05/2013 19:49, Brian Burkhalter wrote:

:
Problem is that the String in the NumberFormatException in this case does not 
contain the reason, only the offending String. The parseInt() would need to be 
changed accordingly to make this intelligible.

I have no strong preference but it does not seem to me that failing to start 
the VM is a great plan.

Brian

I think what you have in the webrev is okay because the purpose of this 
property is to communicate the value of the AutoBoxCacheMax option. In 
other words, this is a special property that only HotSpot should set, it 
is not documented/supported and nobody should be setting it on the 
command as in the bug report.  So if 
-XX:AutoBoxCacheMax= is caught and emits a useful 
error then I think we are okay.


-Alan.



Re: RFR (S) CR 8014966: Add the proper Javadoc to @Contended

2013-05-29 Thread roger riggs

Hi Aleksey,

Thanks for the references.  Currently, you are correct, I don't need it.

Has it been considered that highly contended field should not be allocated
in the object itself anyway but in some pool or structure better suited
to its access characteristics?  A read-only indirection could provide
some ability to allocate contended locks elsewhere where they would
not disrupt the allocation mechanism.  For example, in a pool
of write-only or write-mostly values for the processor/core/thread.
Of course, that has an access cost too.

But given the credentials of the discussion authors I expect this has been
well vetted and I have not much to offer.
Limiting the scope of @contended groups prevents being able to
groups fields from different classes/subclasses together that might
reasonably share access patterns.

The language related to 'intrinsically worthwhile' deserves a caution label.
History is full of premature optimizations.  Since it is mentioned that
instrumentation is not available to compute the costs of contention
in a particular use case, then only macro level performance of the 
application

would be available to judge the effectiveness of a particular @Contended
usage.  But the cost in memory is immediate and measurable.

Thanks, Roger



On 5/29/2013 2:02 PM, Aleksey Shipilev wrote:

Hi Roger,

On 05/29/2013 09:12 PM, roger riggs wrote:

I'm not sure I quite understand the purpose or semantics of this
annotation.

That's the early sign you don't need it! :) I'm actually envious.


Since it is in sun.misc, it may not be so important and is just an
implementation artifact but does not have enough of a standalone
description nor connection to the other elements.

See:
   http://openjdk.java.net/jeps/142
   https://blogs.oracle.com/dave/entry/java_contented_annotation_to_help

http://mail.openjdk.java.net/pipermail/hotspot-dev/2012-November/007309.html

...for more rationale.


The description of the annotation is an odd mix of hints about the
usage of fields and specification of required behavior of an
implementation.

Hm. If there is the issue with the javadoc, I'm failing to see it; maybe
because I am exposed to false sharing enough to immediately understand
what @C is about.

Even though this is an internal API, and we just want to specify @C
barely enough to be useful for those who need it, specific examples what
is broken with the docs are welcome.


I would expect an implementation to be able to ignore the annotation
or perhaps to improve on the performance by utilizing runtime
available information.  It is more useful to say what the annotation
does mean than to say what it does not mean.

Yes, hints are by definition ignorable. Should the runtime be able to
figure out the memory contention issues on its own, we will just "noop"
@C, and be done. But, that Holy Grail is not here.


Contention groups are a useful semantic but to say that a contention
  group " must be isolated from all other contention groups." is
something for the implementation to determine.

I guess the valid concern is the use of "must"? Implementations may
choose to protect the grouped fields on their own if it chooses to do so.

Yep.


-Aleksey.




Re: RFR 8015395: NumberFormatException during startup if java.lang.Integer.IntegerCache.high set to bad value

2013-05-29 Thread Brian Burkhalter
On May 29, 2013, at 11:40 AM, Mike Duigou wrote:

> In other cases similar to this an InternalError is thrown. It seems that for 
> a property like this which can have a dramatic effect on performance silently 
> ignoring bad input may not be the right choice.
> 
> java  -Djava.lang.Integer.IntegerCache.high=foobar
> Exception in thread "main" java.lang.ExceptionInInitializerError
>   at java.lang.Integer.valueOf(Integer.java:640)
>   at sun.launcher.LauncherHelper.initHelpMessage(LauncherHelper.java:330)
> Caused by: java.lang.NumberFormatException: For input string: "foobar"
>   at 
> java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
>   at java.lang.Integer.parseInt(Integer.java:492)
>   at java.lang.Integer.parseInt(Integer.java:527)
>   at java.lang.Integer$IntegerCache.(Integer.java:607)
>   ... 2 more
> 
> Doesn't seem to be that bad to me, but throwing internal error with an 
> appropriate "system property java.lang.Integer.IntegerCache.high must be 
> non-negative integer" message

Problem is that the String in the NumberFormatException in this case does not 
contain the reason, only the offending String. The parseInt() would need to be 
changed accordingly to make this intelligible.

I have no strong preference but it does not seem to me that failing to start 
the VM is a great plan.

Brian

> would seem to be superior than silently doing the wrong thing when:
> 
> java  -Djava.lang.Integer.IntegerCache.high=1024-Xmx2g
> 
> or the less obviously wrong:
> 
> java  -Djava.lang.Integer.IntegerCache.high=1024\
> -Xmx2g
> 
> is encountered.
> 
> Mike
> 
> On May 29 2013, at 11:00 , Brian Burkhalter wrote:
> 
>> http://cr.openjdk.java.net/~bpb/8015395/
>> 
>> Fix is to ignore bad value passed for java.lang.Integer.IntegerCache.high 
>> property and devolve to the default.
>> 
>> Thanks,
>> 
>> Brian
> 



Re: RFR 8015395: NumberFormatException during startup if java.lang.Integer.IntegerCache.high set to bad value

2013-05-29 Thread Mike Duigou
In other cases similar to this an InternalError is thrown. It seems that for a 
property like this which can have a dramatic effect on performance silently 
ignoring bad input may not be the right choice.

java  -Djava.lang.Integer.IntegerCache.high=foobar
Exception in thread "main" java.lang.ExceptionInInitializerError
at java.lang.Integer.valueOf(Integer.java:640)
at sun.launcher.LauncherHelper.initHelpMessage(LauncherHelper.java:330)
Caused by: java.lang.NumberFormatException: For input string: "foobar"
at 
java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
at java.lang.Integer.parseInt(Integer.java:492)
at java.lang.Integer.parseInt(Integer.java:527)
at java.lang.Integer$IntegerCache.(Integer.java:607)
... 2 more

Doesn't seem to be that bad to me, but throwing internal error with an 
appropriate "system property java.lang.Integer.IntegerCache.high must be 
non-negative integer" message would seem to be superior than silently doing the 
wrong thing when:

java  -Djava.lang.Integer.IntegerCache.high=1024-Xmx2g

or the less obviously wrong:

java  -Djava.lang.Integer.IntegerCache.high=1024\
-Xmx2g

is encountered.

Mike

On May 29 2013, at 11:00 , Brian Burkhalter wrote:

> http://cr.openjdk.java.net/~bpb/8015395/
> 
> Fix is to ignore bad value passed for java.lang.Integer.IntegerCache.high 
> property and devolve to the default.
> 
> Thanks,
> 
> Brian



Re: RFR 8005698 : Handle Frequent HashMap Collisions with Balanced Trees

2013-05-29 Thread Brent Christian

On 5/28/13 9:13 PM, Mike Duigou wrote:

Hashtable/WeakHashMap::

- I assume that the JVM falls over dead if the security manager
doesn't allow access to the property, correct? I hadn't considered
what should happen in the event of a security exception when I
originally copied the GetPropertyAction idiom from elsewhere in the
JDK.


AFAIK a security exception won't happen here as it's called in a 
doPrivileged().



Perhaps add a security manager test to CheckRandomHashSeed?
Or two if we want to make sure that


...that what? :)

I ran a test to confirm that maps can be created when useRandomSeed=true 
and a security manager is running (-Djava.security.manager).  Attempting 
to read the property from the test code throws an 
AccessControlException, as one would expect.


CheckRandomHashSeed probably isn't the right place to test this, as the 
test itself requires permissions to read the property and confirm (via 
reflection) the value of the hash seed.  But I can add a new test case, 
if you think it's important.



- initHashSeed could now return the value with assignment happening
in the constructor if we wanted to make hashSeed final. This would
mean the unsafe stuff would have to return in Hashtable/HashMap to
allow for deserialization.


It would be nice to keep hashSeed final.  It would no longer be lazy, 
but we'll still get the main bottleneck relief of 8006593 by using 
ThreadLocalRandom in sun.misc.Hashing.


I'll work on this.


HashMap::

- TreeBin.comparableClassFor() includes "see above for explanation."
but it's not clear where that explanation is.


Things got moved around - its referring to the TreeBin docs, which are 
now "below".  Fixed.



Hashing::

- Do we know if ThreadLocalRandom requires that the VM be booted? It
may be possible to remove even more stuff here.


Indeed.  I don't know the answer.



InPlaceOpsCollisions::

- The @run directives run the wrong class (an error I have made
myself...).


Dang!  Thanks - fixed.


-Brent


On May 28 2013, at 13:03 , Brent Christian wrote: >

On 5/28/13 3:09 AM, Doug Lea wrote:


To better enable and simplify future improvements, could you
do this -- nest the tree classes within HashMap?


Done.


Also, a note on spliterators: I had added these in the
least disruptive way (knowing that major changes were coming)
by checking exact class match for HashMap.class. This is because
there aren't LinkedHashMap view classes to attach overrides to.
While not wrong, and OK for now, at some point this should be redone.


OK.  I will file a bug so this doesn't get forgotten.


I also applied the change to how HashMap.putAll() resizes the table (to account 
for splitTreeBin() only handling doubling of tables).

The updated webrev is here:

http://cr.openjdk.java.net/~bchristi/8005698/webrev.02/

Thanks,
-Brent




Re: RFR (S) CR 8014966: Add the proper Javadoc to @Contended

2013-05-29 Thread Aleksey Shipilev
Hi Roger,

On 05/29/2013 09:12 PM, roger riggs wrote:
> I'm not sure I quite understand the purpose or semantics of this 
> annotation.

That's the early sign you don't need it! :) I'm actually envious.

> Since it is in sun.misc, it may not be so important and is just an
> implementation artifact but does not have enough of a standalone
> description nor connection to the other elements.

See:
  http://openjdk.java.net/jeps/142
  https://blogs.oracle.com/dave/entry/java_contented_annotation_to_help

http://mail.openjdk.java.net/pipermail/hotspot-dev/2012-November/007309.html

...for more rationale.

> The description of the annotation is an odd mix of hints about the 
> usage of fields and specification of required behavior of an 
> implementation.

Hm. If there is the issue with the javadoc, I'm failing to see it; maybe
because I am exposed to false sharing enough to immediately understand
what @C is about.

Even though this is an internal API, and we just want to specify @C
barely enough to be useful for those who need it, specific examples what
is broken with the docs are welcome.

> I would expect an implementation to be able to ignore the annotation
> or perhaps to improve on the performance by utilizing runtime
> available information.  It is more useful to say what the annotation
> does mean than to say what it does not mean.

Yes, hints are by definition ignorable. Should the runtime be able to
figure out the memory contention issues on its own, we will just "noop"
@C, and be done. But, that Holy Grail is not here.

> Contention groups are a useful semantic but to say that a contention
>  group " must be isolated from all other contention groups." is 
> something for the implementation to determine.

I guess the valid concern is the use of "must"? Implementations may
choose to protect the grouped fields on their own if it chooses to do so.

-Aleksey.


RFR 8015395: NumberFormatException during startup if java.lang.Integer.IntegerCache.high set to bad value

2013-05-29 Thread Brian Burkhalter
http://cr.openjdk.java.net/~bpb/8015395/

Fix is to ignore bad value passed for java.lang.Integer.IntegerCache.high 
property and devolve to the default.

Thanks,

Brian

Re: RFR (S) CR 8014966: Add the proper Javadoc to @Contended

2013-05-29 Thread roger riggs

Hi Alexey,

I'm not sure I quite understand the purpose or semantics of this annotation.
Since it is in sun.misc, it may not be so important and is just an 
implementation

artifact but does not have enough of a standalone description nor connection
to the other elements.

The description of the annotation is an odd mix of hints about the
usage of fields and specification of required behavior of an implementation.
I would expect an implementation to be able to ignore the annotation
or perhaps to improve on the performance by utilizing runtime available
information.  It is more useful to say what the annotation does mean
than to say what it does not mean.

Contention groups are a useful semantic but to say that a contention
group " must be isolated from all other contention groups." is something
for the implementation to determine.  It would be more productive to say 
that

fields in the contention group benefit from being grouped together and
have similar access patterns.

Thanks for any clarifications or connections to the rest of the 
functionality,


Roger


On 5/29/2013 7:20 AM, Aleksey Shipilev wrote:


The new webrev is here:
   http://cr.openjdk.java.net/~shade/8014966/webrev.03/

Thanks everyone, writing specs is fun!

-Aleksey.




RFR JDK-8015271: Conversion table for EUC-KR is incorrect

2013-05-29 Thread Xueming Shen

Hi,

Here is proposal to add one mapping entry into the mapping table for EUC-KR
to add the "newly" added KS X 1001:2002 code point

2-71 (EUC_KR:0xA2E8 Unicode: U+0x327E)

Info regarding this code point
http://www.unicode.org/L2/L2004/04267-n2815.pdf

Here is the mapping change webrev

http://cr.openjdk.java.net/~sherman/8015271/webrev

The mapping test webrev, these tests are still in closed repo, hope I can find
time to update and bring them to the public repo soon.

http://cr.openjdk.java.net/~sherman/8015271/closed

Thanks!
-Sherman


hg: jdk8/tl/nashorn: 9 new changesets

2013-05-29 Thread james . laskey
Changeset: 0bf451c0678d
Author:hannesw
Date:  2013-05-27 12:26 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/0bf451c0678d

8015348: RegExp("[") results in StackOverflowError
Reviewed-by: sundar, attila

! src/jdk/nashorn/internal/runtime/regexp/RegExpScanner.java
+ test/script/basic/JDK-8015348.js
+ test/script/basic/JDK-8015348.js.EXPECTED

Changeset: 1f57afd14cc1
Author:lagergren
Date:  2013-05-27 13:11 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/1f57afd14cc1

8014219: Make the run-octane harness more deterministic by not measuring 
elapsed time every iteration. Also got rid of most of the run logic in base.js 
and call benchmarks directly for the same purpose
Reviewed-by: jlaskey, attila

! make/build-benchmark.xml
! src/jdk/nashorn/internal/runtime/AccessorProperty.java
! src/jdk/nashorn/internal/runtime/Property.java
! src/jdk/nashorn/internal/runtime/UserAccessorProperty.java
! test/script/basic/compile-octane.js.EXPECTED
! test/script/basic/run-octane.js

Changeset: 910fd2849c4c
Author:lagergren
Date:  2013-05-27 13:12 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/910fd2849c4c

Merge


Changeset: 343fd0450802
Author:sundar
Date:  2013-05-27 20:41 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/343fd0450802

8015352: "i".toUpperCase() => currently returns "Ä°", but should be "I" (with 
Turkish locale)
Reviewed-by: jlaskey, lagergren

! src/jdk/nashorn/internal/objects/NativeString.java
! src/jdk/nashorn/internal/runtime/ScriptEnvironment.java
! src/jdk/nashorn/internal/runtime/options/OptionTemplate.java
! src/jdk/nashorn/internal/runtime/options/Options.java
! src/jdk/nashorn/internal/runtime/resources/Options.properties
+ test/script/basic/JDK-8015352.js

Changeset: e6193dcfe36c
Author:lagergren
Date:  2013-05-27 17:57 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/e6193dcfe36c

8015447: Octane harness fixes for rhino and entire test runs: ant octane, ant 
octane-v8, ant octane-rhino
Reviewed-by: sundar, jlaskey

! make/build-benchmark.xml
! test/script/basic/run-octane.js

Changeset: d56168970de1
Author:sundar
Date:  2013-05-28 16:37 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/d56168970de1

8015459: Octane test run fails on Turkish locale
Reviewed-by: lagergren, attila

! src/jdk/nashorn/internal/codegen/CodeGenerator.java
! src/jdk/nashorn/internal/objects/DateParser.java
! src/jdk/nashorn/internal/parser/TokenType.java
! src/jdk/nashorn/internal/runtime/GlobalFunctions.java
! src/jdk/nashorn/internal/runtime/JSType.java
! src/jdk/nashorn/internal/runtime/Logging.java
! src/jdk/nashorn/internal/runtime/ScriptRuntime.java
! src/jdk/nashorn/internal/runtime/options/OptionTemplate.java

Changeset: f472f7046ec9
Author:sundar
Date:  2013-05-29 15:41 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/f472f7046ec9

8005979: A lot of tests are named "runTest" in reports
Reviewed-by: jlaskey

! make/project.properties

Changeset: f69e76417211
Author:lagergren
Date:  2013-05-29 14:08 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/f69e76417211

8011023: Math round didn't conform to ECMAScript 5 spec
Reviewed-by: jlaskey, attila

! src/jdk/nashorn/internal/objects/NativeMath.java
+ test/script/basic/JDK-8011023.js
+ test/script/basic/JDK-8011023.js.EXPECTED

Changeset: a2e2797392b3
Author:sundar
Date:  2013-05-29 21:27 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/a2e2797392b3

8015349: "abc".lastIndexOf("a",-1) should evaluate to 0 and not -1
Reviewed-by: lagergren, attila, jlaskey

! src/jdk/nashorn/internal/objects/NativeString.java
+ test/script/basic/JDK-8015349.js
+ test/script/basic/JDK-8015349.js.EXPECTED



Re: RFR JDK-8015299

2013-05-29 Thread Chris Hegarty

On 29/05/2013 02:50, David Holmes wrote:

Looks good to me.


+1. If you need a sponsor, I can push this for you John.

-Chris.



David

On 28/05/2013 11:07 PM, John Zavgren wrote:

Greetings:

I fixed the comment line... to correct the "nul", "NULL", "NUL" issue
and I disambiguated the readlink() issue.

Thanks!
John

On 05/26/2013 11:03 PM, David Holmes wrote:

I concur with Martin on all counts.

Confusing NUL and NULL is also a pet peeve of mine :)

David

On 25/05/2013 2:15 AM, Martin Buchholz wrote:

Hi John,

The memory leak fix looks good.
---
IIRC I was the author of the comment, so it is not surprising that I
might
prefer that version. "nul" is ___not___ a typo for NULL. It makes no
sense to speak of character arrays being NULL-terminated. It also isn't
quite right to say that readlink "generates" an array - it simply
writes
into one.

However, it is more correct to spell "nul" "NUL" as in ASCII(7).

If you wanted to be more precise about which readlink, you could write
readlink(2) instead of readlink() to disambiguate from readlink(1)


On Fri, May 24, 2013 at 8:04 AM, John Zavgren
wrote:


Greetings:

I fixed two memory leaks in the file: jdk/src/solaris/bin/java_md_**
solinux.c
and I posted a webrev image of the changes at:
http://cr.openjdk.java.net/~**jzavgren/8015299/webrev.01/<


http://cr.openjdk.java.net/%**7Ejzavgren/8015299/webrev.01/






(I also edited a comment.)

Thanks!
John

--
John Zavgren
john.zavg...@oracle.com
603-821-0904
US-Burlington-MA







Re: RFR 8005704: Update ConcurrentHashMap to v8

2013-05-29 Thread Chris Hegarty

On 29/05/2013 15:28, Peter Levart wrote:

.

I don't feel strongly about this either, but I think it deserves
possibly its own bug number and consideration. I have removed it from
this review request, and will a file a new bug to track it.


Hi,

Why not using Unsafe (which is already used in CHM) to re-use the
AbstractMap.keySet/values fields? They could even be accessed with
normal non-volatile read/write although they are declared volatile in
AbstractMap. Is this to "hacky"?


Possibly a little hacky, and I guess may sacrifice a little performance? 
But this kind of analysis/investigation, and any compatibility concerns, 
are exactly why I think this deserves its own bug. I'm not saying that 
it's not worth pursuing, just that we should decouple it from the other 
changes going on here. (Says me who wanted to resync, in one pass, the 
complete j.u.c last week!)


-Chris.



Regards, Peter



Re: RFR 8005698 : Handle Frequent HashMap Collisions with Balanced Trees

2013-05-29 Thread Doug Lea


Back to this. (which applies to ConcurrentHashMap, under review).

On 05/24/13 07:18, Doug Lea wrote:

On 05/23/13 16:47, Jeff Hain wrote:



Implementing some RB-tree I found out that

a lot of the null checks from CLR code could be removed from balancing
operations.


Yes and no. Some of those not logically needed prevent infinite looping in
the case of concurrent interference.


But still, there are several that can go away without forcing
so much rare-trap handling. I updated the analogous
code in ConcurrentHashMap version to omit a few of them, and also
retained in this version an internal checkInvariants method that
can be used to further refine. (This is possibly useful to
other developers, so worth leaving in rather than pulling out
each time I update this code.) I also left asserts to it enabled
in a couple of spots to allow more testing with "-ea".  If/when I
offer an update to plain HashMap, I'll also include.



it seemed that null checks could actually be removed for all remaining calls
to leftOf/rightOf/setColor (not for getColor), but since I didn't figure out
why I didn't  touch these.



Right. TreeMap could use some similar improvement someday. That code
was based on red-black algorithms that assume the tree is expanded such
that all null links go to dummy nodes, which I long ago
emulated with these calls in some classes I wrote that in turn
got adapted in TreeMap and elsewhere.

-Doug


Re: RFR 8015008: Primitive iterator over empty sequence, null consumer: forEachRemaining methods do not throw NPE

2013-05-29 Thread Chris Hegarty

Looks fine to me Paul.

On 29/05/2013 12:35, Paul Sandoz wrote:

Hi,

Please review these changes to j.u.PrimitiveIterator to ensure the default 
forEachRemaining methods consistently throw an NPE when the consumer is null.

I almost produced a webrev for this, but i thought it was just about acceptable 
size-wise and i hope easy to review in textual form. If this is considered 
impolite, awkward to review etc please say so and i will produce a webrev.


I found it a little difficult to review as it lacked some context around 
the changes. I found myself having to track down the source and compare 
line numbers. Not a problem, just that you happen to mention it ;-)


-Chris



Paul.

# HG changeset patch
# User psandoz
# Date 1369825083 -7200
# Node ID 7ded996200218791c885c0aae4c474066101c7bd
# Parent  bfdc1ed75460c9e6869827cf9acabd4b1a5e9d29
8015008: Primitive iterator over empty sequence, null consumer: 
forEachRemaining methods do not throw NPE
Reviewed-by:

diff -r bfdc1ed75460 -r 7ded99620021 
src/share/classes/java/util/PrimitiveIterator.java
--- a/src/share/classes/java/util/PrimitiveIterator.javaWed May 29 
12:58:02 2013 +0200
+++ b/src/share/classes/java/util/PrimitiveIterator.javaWed May 29 
12:58:03 2013 +0200
@@ -91,6 +91,7 @@
   * @throws NullPointerException if the specified action is null
   */
  default void forEachRemaining(IntConsumer action) {
+Objects.requireNonNull(action);
  while (hasNext())
  action.accept(nextInt());
  }
@@ -123,6 +124,8 @@
  forEachRemaining((IntConsumer) action);
  }
  else {
+// The method reference action::accept is never null
+Objects.requireNonNull(action);
  if (Tripwire.ENABLED)
  Tripwire.trip(getClass(), "{0} calling 
PrimitiveIterator.OfInt.forEachRemainingInt(action::accept)");
  forEachRemaining((IntConsumer) action::accept);
@@ -162,6 +165,7 @@
   * @throws NullPointerException if the specified action is null
   */
  default void forEachRemaining(LongConsumer action) {
+Objects.requireNonNull(action);
  while (hasNext())
  action.accept(nextLong());
  }
@@ -194,6 +198,8 @@
  forEachRemaining((LongConsumer) action);
  }
  else {
+// The method reference action::accept is never null
+Objects.requireNonNull(action);
  if (Tripwire.ENABLED)
  Tripwire.trip(getClass(), "{0} calling 
PrimitiveIterator.OfLong.forEachRemainingLong(action::accept)");
  forEachRemaining((LongConsumer) action::accept);
@@ -232,6 +238,7 @@
   * @throws NullPointerException if the specified action is null
   */
  default void forEachRemaining(DoubleConsumer action) {
+Objects.requireNonNull(action);
  while (hasNext())
  action.accept(nextDouble());
  }
@@ -265,6 +272,8 @@
  forEachRemaining((DoubleConsumer) action);
  }
  else {
+// The method reference action::accept is never null
+Objects.requireNonNull(action);
  if (Tripwire.ENABLED)
  Tripwire.trip(getClass(), "{0} calling 
PrimitiveIterator.OfDouble.forEachRemainingDouble(action::accept)");
  forEachRemaining((DoubleConsumer) action::accept);
diff -r bfdc1ed75460 -r 7ded99620021 
test/java/util/Iterator/PrimitiveIteratorDefaults.java
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/test/java/util/Iterator/PrimitiveIteratorDefaults.javaWed May 29 
12:58:03 2013 +0200
@@ -0,0 +1,115 @@
+/*
+ * Copyright (c) 2013, 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.
+ *
+ * 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.
+ */
+
+import org.testng.annotations.Test;
+
+import java.util.PrimitiveIterator;
+import ja

Re: RFR 8005704: Update ConcurrentHashMap to v8

2013-05-29 Thread Peter Levart

On 05/29/2013 04:07 PM, Chris Hegarty wrote:

 ... and the links to the updated spedcdiff / webrev


http://cr.openjdk.java.net/~chegar/8005704/ver.01/specdiff/java/util/concurrent/package-summary.html 


  http://cr.openjdk.java.net/~chegar/8005704/ver.01/webrev/

-Chris.

On 29/05/2013 15:06, Chris Hegarty wrote:

Mike, Doug,

On 28/05/2013 20:07, Mike Duigou wrote:

Hi Chris& Doug;

- I don't feel strongly about the removal of AbstractMap. I don't see
this as very likely to cause problems in real world code though there
is probably some test code somewhere that assigns CHM to an 
AbstractMap.


I don't feel strongly about this either, but I think it deserves
possibly its own bug number and consideration. I have removed it from
this review request, and will a file a new bug to track it.


Hi,

Why not using Unsafe (which is already used in CHM) to re-use the 
AbstractMap.keySet/values fields? They could even be accessed with 
normal non-volatile read/write although they are declared volatile in 
AbstractMap. Is this to "hacky"?


Regards, Peter




- I am reluctant to deprecate contains(Object) here unless we
deprecate it in Hashtable as well. I recognize that this has been a
source of errors
(https://issues.apache.org/bugzilla/show_bug.cgi?id=48755 for one
example). Is it time to deprecate it there as well?


Dito for this, removed from this request and should be revisited
separately.

-Chris.




Re: RFR 8005698 : Handle Frequent HashMap Collisions with Balanced Trees

2013-05-29 Thread Mike Duigou
Understood that it is necessary to have some mechanism to have reproducible 
results. The command line system property is by default disabled. Enabling it 
has only the effect of setting the hashSeed to a non-zero XOR value for each 
instance. Since the randomness is per instance there's no single value to print 
out. We considered a switch, like python, to allow using the same non-zero seed 
value to all instances but it didn't seem to offer much additional debugging 
capability so we opted to not implement it.

Mike

On May 29 2013, at 05:36 , David Chase wrote:

> It looks like there's no way to set the random seed from the command line.  
> That's going to make Heisenbugs even less fun than they already are.  
> Ideally, if a VM crashes, that random seed could get mentioned somewhere in 
> the crash, and someone who is debugging might then be able to specify it 
> through a command-line property.  It's not a guarantee of a repeatable run, 
> but it at least removes an obstacle.

> David
> 
> On 2013-05-29, at 12:13 AM, Mike Duigou  wrote:
> 
>> Hashtable/WeakHashMap::
>> 
>> - I assume that the JVM falls over dead if the security manager doesn't 
>> allow access to the property, correct? I hadn't considered what should 
>> happen in the event of a security exception when I originally copied the 
>> GetPropertyAction idiom from elsewhere in the JDK. Perhaps add a security 
>> manager test to CheckRandomHashSeed? Or two if we want to make sure that 
>> 
>> - initHashSeed could now return the value with assignment happening in the 
>> constructor if we wanted to make hashSeed final. This would mean the unsafe 
>> stuff would have to return in Hashtable/HashMap to allow for deserialization.
>> 
>> 
>> HashMap::
>> 
>> - TreeBin.comparableClassFor() includes "see above for explanation." but 
>> it's not clear where that explanation is.
>> 
>> - I was really worried about the cost and correctness issues with changing 
>> null key handling. I have now put my mind at ease. All of the cases I could 
>> think of seem to be handled.
>> 
>> - I've spent less time in this round looking at the new Map default 
>> operations.
>> 
>> Hashing::
>> 
>> - Do we know if ThreadLocalRandom requires that the VM be booted? It may be 
>> possible to remove even more stuff here.
>> 
>> 
>> InPlaceOpsCollisions::
>> 
>> - The @run directives run the wrong class (an error I have made myself...).
>> 
>> Mike
>> 
>> On May 28 2013, at 13:03 , Brent Christian wrote:
>> 
>>> On 5/28/13 3:09 AM, Doug Lea wrote:
 
 To better enable and simplify future improvements, could you
 do this -- nest the tree classes within HashMap?
>>> 
>>> Done.
>>> 
 Also, a note on spliterators: I had added these in the
 least disruptive way (knowing that major changes were coming)
 by checking exact class match for HashMap.class. This is because
 there aren't LinkedHashMap view classes to attach overrides to.
 While not wrong, and OK for now, at some point this should be redone.
>>> 
>>> OK.  I will file a bug so this doesn't get forgotten.
>>> 
>>> 
>>> I also applied the change to how HashMap.putAll() resizes the table (to 
>>> account for splitTreeBin() only handling doubling of tables).
>>> 
>>> The updated webrev is here:
>>> 
>>> http://cr.openjdk.java.net/~bchristi/8005698/webrev.02/
>>> 
>>> Thanks,
>>> -Brent
>> 
> 



Re: RFR 8014409: Spec typo: extra } in the spec for j.u.s.StreamBuilder

2013-05-29 Thread Mike Duigou
Looks fine to me with the space removed as well.

Mike

On May 29 2013, at 06:10 , Paul Sandoz wrote:

> 
> On May 29, 2013, at 2:49 PM, Chris Hegarty  wrote:
> 
>> Looks fine to me Paul.
>> 
>> Trivially, there is an extra space which could be removed.
>> 
>> "builder__has..."
>> 
> 
> Thanks, i updated the patch/lambda repo.
> 
> Paul.



Re: RFR 8005704: Update ConcurrentHashMap to v8

2013-05-29 Thread Chris Hegarty

Mike, Doug,

On 28/05/2013 20:07, Mike Duigou wrote:

Hi Chris&  Doug;

- I don't feel strongly about the removal of AbstractMap. I don't see this as 
very likely to cause problems in real world code though there is probably some 
test code somewhere that assigns CHM to an AbstractMap.


I don't feel strongly about this either, but I think it deserves 
possibly its own bug number and consideration. I have removed it from 
this review request, and will a file a new bug to track it.



- I am reluctant to deprecate contains(Object) here unless we deprecate it in 
Hashtable as well. I recognize that this has been a source of errors 
(https://issues.apache.org/bugzilla/show_bug.cgi?id=48755 for one example). Is 
it time to deprecate it there as well?


Dito for this, removed from this request and should be revisited separately.

-Chris.


Re: RFR 8005704: Update ConcurrentHashMap to v8

2013-05-29 Thread Chris Hegarty

 ... and the links to the updated spedcdiff / webrev


http://cr.openjdk.java.net/~chegar/8005704/ver.01/specdiff/java/util/concurrent/package-summary.html
  http://cr.openjdk.java.net/~chegar/8005704/ver.01/webrev/

-Chris.

On 29/05/2013 15:06, Chris Hegarty wrote:

Mike, Doug,

On 28/05/2013 20:07, Mike Duigou wrote:

Hi Chris& Doug;

- I don't feel strongly about the removal of AbstractMap. I don't see
this as very likely to cause problems in real world code though there
is probably some test code somewhere that assigns CHM to an AbstractMap.


I don't feel strongly about this either, but I think it deserves
possibly its own bug number and consideration. I have removed it from
this review request, and will a file a new bug to track it.


- I am reluctant to deprecate contains(Object) here unless we
deprecate it in Hashtable as well. I recognize that this has been a
source of errors
(https://issues.apache.org/bugzilla/show_bug.cgi?id=48755 for one
example). Is it time to deprecate it there as well?


Dito for this, removed from this request and should be revisited
separately.

-Chris.


hg: jdk8/tl/jdk: 7174966: With OCSP enabled on Java 7 get error 'Wrong key usage' with Comodo certificate

2013-05-29 Thread vincent . x . ryan
Changeset: 00ad19610e75
Author:vinnie
Date:  2013-05-29 14:57 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/00ad19610e75

7174966: With OCSP enabled on Java 7 get error 'Wrong key usage' with Comodo 
certificate
Reviewed-by: xuelei

! src/share/classes/sun/security/provider/certpath/OCSPResponse.java



Re: RFR 8005704: Update ConcurrentHashMap to v8

2013-05-29 Thread Doug Lea

On 05/28/13 16:03, Martin Buchholz wrote:

A long-returning size method seems like a framework-level decision.  We could
add a default method to Collection.java and Map.java

default long mappingCount() { return size(); }


The lambda EG discussed this, and decided not to do it now,
and to put off support for 64bit collections until JDK9.
But CHM needs this now, so we picked a name that is usable
but not likely to clash with some better future name, like length().

-Doug



I'm not sure mappingCount is the best name, but all names will be confusing.
  Using the name "length()" at least has an onomatopoeic mnemonic advantage - it
suggests that it might return a long.

 /**
  * Returns the number of mappings. This method should be used
  * instead of {@link #size} because a ConcurrentHashMap may
  * contain more mappings than can be represented as an int. The
  * value returned is an estimate; the actual count may differ if
  * there are concurrent insertions or removals.
  *
  * @return the number of mappings
  */
 public long mappingCount() {
 long n = sumCount();
 return (n < 0L) ? 0L : n; // ignore transient negative values
 }





Re: RFR 8005704: Update ConcurrentHashMap to v8

2013-05-29 Thread Doug Lea

On 05/28/13 15:07, Mike Duigou wrote:

Hi Chris & Doug;



- I don't see the advantage to exposing the ConcurrentHashMap.KeySetView type
particularly for newKeySet(). Why not return Set? The additional methods
don't seem to offer much that's desirable for the newKeySet() case.


Since we don't have a ConcurrentSet interface, people are reduced to
instanceof checks to see if they have a concurrent one. But without
exposing the class, they couldn't even do this, so complained. This
will arise more frequently with  j.u.streams.Collectors.




- I am reluctant to deprecate contains(Object) here unless we deprecate it in
Hashtable as well. I recognize that this has been a source of errors
(https://issues.apache.org/bugzilla/show_bug.cgi?id=48755 for one example).
Is it time to deprecate it there as well?


Sure, but why bother. Anyone still using Hashtable is not going to care
if they get more deprecation warnings :-)




- I think there could be more complete description of the
parallelismThreshold and interaction with common pool. i.e. does "maximal
parallelism" mean one thread per element or "size() /
getCommonPoolParallelism()". Some advice for choosing in-between values would
be good unless "1" is the best advice for cases where you just don't know. It
would be a shame to see people shooting themselves in the foot with this.



Changed to:

 * These bulk operations accept a {@code parallelismThreshold}
 * argument. Methods proceed sequentially if the current map size is
 * estimated to be less than the given threshold. Using a value of
 * {@code Long.MAX_VALUE} suppresses all parallelism.  Using a value
 * of {@code 1} results in maximal parallelism by partitioning into
 * enough subtasks to utilize all processors. Normally, you would
 * initially choose one of these extreme values, and then measure
 * performance of using in-between values that trade off overhead
 * versus throughput. Parallel forms use the {@link
 * ForkJoinPool#commonPool()}.
 *

I'd rather not explicitly mention here any particular values for people
to try, since any number we might put is likely to be misused.
Sometime, we should write up a little how-to document about
tuning parallelism separately.

-Doug




Re: RFR 8014409: Spec typo: extra } in the spec for j.u.s.StreamBuilder

2013-05-29 Thread Paul Sandoz

On May 29, 2013, at 2:49 PM, Chris Hegarty  wrote:

> Looks fine to me Paul.
> 
> Trivially, there is an extra space which could be removed.
> 
>  "builder__has..."
> 

Thanks, i updated the patch/lambda repo.

Paul.


Re: RFR 8014732: Minor spec issue: java.util.Spliterator.getExactSizeIfKnown

2013-05-29 Thread Chris Hegarty

Looks good to me.

-Chris.

On 29/05/2013 12:36, Paul Sandoz wrote:

Hi,

Please review this JavaDoc fixe to j.u.Spliterator.getExactSizeIfKnown.

Paul.

# HG changeset patch
# User psandoz
# Date 1369825085 -7200
# Node ID 840469ba82ab8d8238414a5333aa99b8d4035a9b
# Parent  7ded996200218791c885c0aae4c474066101c7bd
8014732: Minor spec issue: java.util.Spliterator.getExactSizeIfKnown
Summary: A minor documentation issue (not a spec issue).
Reviewed-by:

diff -r 7ded99620021 -r 840469ba82ab 
src/share/classes/java/util/Spliterator.java
--- a/src/share/classes/java/util/Spliterator.java  Wed May 29 12:58:03 
2013 +0200
+++ b/src/share/classes/java/util/Spliterator.java  Wed May 29 12:58:05 
2013 +0200
@@ -394,9 +394,9 @@
   * Convenience method that returns {@link #estimateSize()} if this
   * Spliterator is {@link #SIZED}, else {@code -1}.
   * @implSpec
- * The default returns the result of {@code estimateSize()} if the
- * Spliterator reports a characteristic of {@code SIZED}, and {@code -1}
- * otherwise.
+ * The default implementation returns the result of {@code estimateSize()}
+ * if the Spliterator reports a characteristic of {@code SIZED}, and
+ * {@code -1} otherwise.
   *
   * @return the exact size, if known, else {@code -1}.
   */


Re: RFR 8014731: j.u.stream.StreamSupport class has default constructor generated

2013-05-29 Thread Chris Hegarty

Looks fine to me.

-Chris.

On 29/05/2013 12:12, Paul Sandoz wrote:

Hi,

Please review these changes to j.u.s. StreamSupport to make it final and 
non-instantiable, i also tacked on some JavaDoc broken link fixes made by Henry.

Paul.

# HG changeset patch
# User psandoz
# Date 1369825082 -7200
# Node ID bfdc1ed75460c9e6869827cf9acabd4b1a5e9d29
# Parent  6be3ce51e61dbdab6766c74223c076a7b3472be6
8014731: j.u.stream.StreamSupport class has default constructor generated
Summary: This change set also fixes broken links
Reviewed-by:
Contributed-by: Paul Sandoz, Henry 
Jen

diff -r 6be3ce51e61d -r bfdc1ed75460 
src/share/classes/java/util/stream/StreamSupport.java
--- a/src/share/classes/java/util/stream/StreamSupport.java Tue May 28 
15:22:30 2013 +0200
+++ b/src/share/classes/java/util/stream/StreamSupport.java Wed May 29 
12:58:02 2013 +0200
@@ -41,7 +41,11 @@
   *
   * @since 1.8
   */
-public class StreamSupport {
+public final class StreamSupport {
+
+// Suppresses default constructor, ensuring non-instantiability.
+private StreamSupport() {}
+
  /**
   * Creates a new sequential {@code Stream} from a {@code Spliterator}.
   *
@@ -50,7 +54,7 @@
   *
   *It is strongly recommended the spliterator report a characteristic of
   * {@code IMMUTABLE} or {@code CONCURRENT}, or be
- *late-binding.  Otherwise,
+ *late-binding.  Otherwise,
   * {@link #stream(Supplier, int)} should be used to
   * reduce the scope of potential interference with the source.  See
   *Non-Interference  
for
@@ -75,7 +79,7 @@
   *
   *It is strongly recommended the spliterator report a characteristic of
   * {@code IMMUTABLE} or {@code CONCURRENT}, or be
- *late-binding.  Otherwise,
+ *late-binding.  Otherwise,
   * {@link #stream(Supplier, int)} should be used to
   * reduce the scope of potential interference with the source.  See
   *Non-Interference  
for
@@ -102,7 +106,7 @@
   *
   *For spliterators that report a characteristic of {@code IMMUTABLE}
   * or {@code CONCURRENT}, or that are
- *late-binding, it is likely
+ *late-binding, it is likely
   * more efficient to use {@link #stream(java.util.Spliterator)} instead.
   * The use of a {@code Supplier} in this form provides a level of
   * indirection that reduces the scope of potential interference with the
@@ -138,7 +142,7 @@
   *
   *For spliterators that report a characteristic of {@code IMMUTABLE}
   * or {@code CONCURRENT}, or that are
- *late-binding, it is likely
+ *late-binding, it is likely
   * more efficient to use {@link #stream(Spliterator)} instead.
   * The use of a {@code Supplier} in this form provides a level of
   * indirection that reduces the scope of potential interference with the
@@ -172,7 +176,7 @@
   *
   *It is strongly recommended the spliterator report a characteristic of
   * {@code IMMUTABLE} or {@code CONCURRENT}, or be
- *late-binding.  Otherwise,
+ *late-binding.  Otherwise,
   * {@link #stream(Supplier, int)}} should be used to
   * reduce the scope of potential interference with the source.  See
   *Non-Interference  
for
@@ -195,7 +199,7 @@
   *
   *It is strongly recommended the spliterator report a characteristic of
   * {@code IMMUTABLE} or {@code CONCURRENT}, or be
- *late-binding.  Otherwise,
+ *late-binding.  Otherwise,
   * {@link #stream(Supplier, int)}} should be used to
   * reduce the scope of potential interference with the source.  See
   *Non-Interference  
for
@@ -220,7 +224,7 @@
   *
   *For spliterators that report a characteristic of {@code IMMUTABLE}
   * or {@code CONCURRENT}, or that are
- *late-binding, it is likely
+ *late-binding, it is likely
   * more efficient to use {@link #intStream(Spliterator.OfInt)} instead.
   * The use of a {@code Supplier} in this form provides a level of
   * indirection that reduces the scope of potential interference with the
@@ -254,7 +258,7 @@
   *
   *For spliterators that report a characteristic of {@code IMMUTABLE}
   * or {@code CONCURRENT}, or that are
- *late-binding, it is likely
+ *late-binding, it is likely
   * more efficient to use {@link #intStream(Spliterator.OfInt)} instead.
   * The use of a {@code Supplier} in this form provides a level of
   * indirection that reduces the scope of potential interference with the
@@ -286,7 +290,7 @@
   *
   *It is strongly recommended the spliterator report a characteristic of
   * {@code IMMUTABLE} or {@code CONCURRENT}, or be
- *late-binding.  Otherwise,
+ *late-binding.  Otherwise,
   * {@link #stream(Supplier, int)} should be used to
   * reduce the scope of potential interference with the source.  See
   *Non-Interference  
for
@@ -310,7 +314,7 @@
   *
   *It is strongly recommended the spliterator report a chara

Re: RFR 8014409: Spec typo: extra } in the spec for j.u.s.StreamBuilder

2013-05-29 Thread Chris Hegarty

Looks fine to me Paul.

Trivially, there is an extra space which could be removed.

  "builder__has..."

-Chris.

On 29/05/2013 12:08, Paul Sandoz wrote:

Hi,

Please review these JavaDoc fixes to j.u.s.StreamBuilder.

Paul.

# HG changeset patch
# User psandoz
# Date 1369825073 -7200
# Node ID 303e9d2aff3cbaf27823b2591f2e8570b77afcce
# Parent  bd6d3801347bfd912507d16dc14488f47e94e626
8014409: Spec typo: extra } in the spec for j.u.s.StreamBuilder
Summary: Also fixes documentation on StreamBuilder.OfDouble
Reviewed-by:

diff -r bd6d3801347b -r 303e9d2aff3c 
src/share/classes/java/util/stream/StreamBuilder.java
--- a/src/share/classes/java/util/stream/StreamBuilder.java Wed May 29 
09:42:39 2013 +0200
+++ b/src/share/classes/java/util/stream/StreamBuilder.java Wed May 29 
12:57:53 2013 +0200
@@ -38,7 +38,7 @@
   *A {@code StreamBuilder} has a lifecycle, where it starts in a building
   * phase, during which elements can be added, and then transitions to a built
   * phase, after which elements may not be added.  The built phase begins
- * when the {@link #build()}} method is called, which creates an ordered
+ * when the {@link #build()} method is called, which creates an ordered
   * {@code Stream} whose elements are the elements that were added to the 
stream
   * builder, in the order they were added.
   *
@@ -98,7 +98,7 @@
   *A stream builder has a lifecycle, where it starts in a building
   * phase, during which elements can be added, and then transitions to a
   * built phase, after which elements may not be added.  The built phase
- * begins when the {@link #build()}} method is called, which creates an
+ * begins when the {@link #build()} method is called, which creates an
   * ordered stream whose elements are the elements that were added to the
   * stream builder, in the order they were added.
   *
@@ -155,7 +155,7 @@
   *A stream builder has a lifecycle, where it starts in a building
   * phase, during which elements can be added, and then transitions to a
   * built phase, after which elements may not be added.  The built phase
- * begins when the {@link #build()}} method is called, which creates an
+ * begins when the {@link #build()} method is called, which creates an
   * ordered stream whose elements are the elements that were added to the
   * stream builder, in the order they were added.
   *
@@ -209,6 +209,13 @@
  /**
   * A mutable builder for a {@code DoubleStream}.
   *
+ *A stream builder  has a lifecycle, where it starts in a building
+ * phase, during which elements can be added, and then transitions to a
+ * built phase, after which elements may not be added.  The built phase
+ * begins when the {@link #build()} method is called, which creates an
+ * ordered stream whose elements are the elements that were added to the
+ * stream builder, in the order they were added.
+ *
   * @see LongStream#builder()
   * @since 1.8
   */
@@ -217,13 +224,6 @@
  /**
   * Adds an element to the stream being built.
   *
- *A stream builder  has a lifecycle, where it starts in a building
- * phase, during which elements can be added, and then transitions to a
- * built phase, after which elements may not be added.  The built phase
- * begins when the {@link #build()}} method is called, which creates an
- * ordered stream whose elements are the elements that were added to 
the
- * stream builder, in the order they were added.
- *
   * @throws IllegalStateException if the builder has already 
transitioned
   * to the built state
   */


Re: RFR 8014393: Minor typo in the spec for j.u.stream.Stream.findFirst()

2013-05-29 Thread Chris Hegarty

On 29/05/2013 13:00, Alan Bateman wrote:

On 29/05/2013 12:10, Paul Sandoz wrote:

Hi,

Please review these JavaDoc fixes to j.u.s.{Xxxx}Stream.findFirst.

Paul.

I had to look at the diff twice to spot it :-)


Not bad, it took me three times ;-)


Looks fine to me.


+1

-Chris.


Re: RFR 8005698 : Handle Frequent HashMap Collisions with Balanced Trees

2013-05-29 Thread David Chase
It looks like there's no way to set the random seed from the command line.  
That's going to make Heisenbugs even less fun than they already are.  Ideally, 
if a VM crashes, that random seed could get mentioned somewhere in the crash, 
and someone who is debugging might then be able to specify it through a 
command-line property.  It's not a guarantee of a repeatable run, but it at 
least removes an obstacle.

David

On 2013-05-29, at 12:13 AM, Mike Duigou  wrote:

> Hashtable/WeakHashMap::
> 
> - I assume that the JVM falls over dead if the security manager doesn't allow 
> access to the property, correct? I hadn't considered what should happen in 
> the event of a security exception when I originally copied the 
> GetPropertyAction idiom from elsewhere in the JDK. Perhaps add a security 
> manager test to CheckRandomHashSeed? Or two if we want to make sure that 
> 
> - initHashSeed could now return the value with assignment happening in the 
> constructor if we wanted to make hashSeed final. This would mean the unsafe 
> stuff would have to return in Hashtable/HashMap to allow for deserialization.
> 
> 
> HashMap::
> 
> - TreeBin.comparableClassFor() includes "see above for explanation." but it's 
> not clear where that explanation is.
> 
> - I was really worried about the cost and correctness issues with changing 
> null key handling. I have now put my mind at ease. All of the cases I could 
> think of seem to be handled.
> 
> - I've spent less time in this round looking at the new Map default 
> operations.
> 
> Hashing::
> 
> - Do we know if ThreadLocalRandom requires that the VM be booted? It may be 
> possible to remove even more stuff here.
> 
> 
> InPlaceOpsCollisions::
> 
> - The @run directives run the wrong class (an error I have made myself...).
> 
> Mike
> 
> On May 28 2013, at 13:03 , Brent Christian wrote:
> 
>> On 5/28/13 3:09 AM, Doug Lea wrote:
>>> 
>>> To better enable and simplify future improvements, could you
>>> do this -- nest the tree classes within HashMap?
>> 
>> Done.
>> 
>>> Also, a note on spliterators: I had added these in the
>>> least disruptive way (knowing that major changes were coming)
>>> by checking exact class match for HashMap.class. This is because
>>> there aren't LinkedHashMap view classes to attach overrides to.
>>> While not wrong, and OK for now, at some point this should be redone.
>> 
>> OK.  I will file a bug so this doesn't get forgotten.
>> 
>> 
>> I also applied the change to how HashMap.putAll() resizes the table (to 
>> account for splitTreeBin() only handling doubling of tables).
>> 
>> The updated webrev is here:
>> 
>> http://cr.openjdk.java.net/~bchristi/8005698/webrev.02/
>> 
>> Thanks,
>> -Brent
> 



Re: RFR 8014393: Minor typo in the spec for j.u.stream.Stream.findFirst()

2013-05-29 Thread Alan Bateman

On 29/05/2013 12:10, Paul Sandoz wrote:

Hi,

Please review these JavaDoc fixes to j.u.s.{Xxxx}Stream.findFirst.

Paul.

I had to look at the diff twice to spot it :-)

Looks fine to me.

-Alan.


Re: RFR 8014409: Spec typo: extra } in the spec for j.u.s.StreamBuilder

2013-05-29 Thread Alan Bateman

On 29/05/2013 12:08, Paul Sandoz wrote:

Hi,

Please review these JavaDoc fixes to j.u.s.StreamBuilder.

Paul.


This looks fine to me.

-Alan


Re: RFR: JDK-8013827 and JDK-8011950, , java.io.File.createTempFile enters infinite loop when passed invalid data

2013-05-29 Thread Alan Bateman

On 28/05/2013 19:39, Dan Xu wrote:

Hi All,

When File.createTempFile() is called with some special parameters, it 
runs into infiniteloop and hangs. It is because it does not always 
mean a file exists when the method, createFileExclusively(), returns 
false. This fix is going to solve such issues reported in JDK-8013827 
and JDK-8011950.And I also added some testcases to verify it.


webrev: http://cr.openjdk.java.net/~dxu/8013827/webrev.00/
bug: http://bugs.sun.com/view_bug.do?bug_id=8011950

-Dan

Thanks for taking this one.

The changes mean that IAE is now thrown for cases where it wasn't thrown 
previously and I'm wondering if this is worth doing. It might be simpler 
to just throw IOException on the grounds that creating the file will fail.


One other thing to consider is that "\" is a valid name of a file on 
some platforms. I suspect you'll need to delegate to the FileSystem to 
check the suffix/prefix. Alternatively in TempDirectory.generateFile you 
could create a File from prefix/n/suffix and call getName to check that 
its matches. If not then throw an exception.


-Alan.


Re: RFR 8014731: j.u.stream.StreamSupport class has default constructor generated

2013-05-29 Thread Paul Sandoz

On May 29, 2013, at 1:34 PM, Alan Bateman  wrote:

> On 29/05/2013 12:12, Paul Sandoz wrote:
>> Hi,
>> 
>> Please review these changes to j.u.s. StreamSupport to make it final and 
>> non-instantiable, i also tacked on some JavaDoc broken link fixes made by 
>> Henry.
>> 
>> Paul.
>> 
> This looks okay to me but just to note that making it final is technically an 
> API change to track.
> 

Thanks, i forgot about that, CCC has been kicked off.

Paul.

PhantomReference: why not cleared by GC when enqueued?

2013-05-29 Thread Dmytro Sheyko
Hello,

Why phantom references are not automatically cleared by the garbage collector 
as they are enqueued?

Keeping phantom reachable objects in heap has some drawbacks:
1. At least 2 GC are required in order to reclaim them, even in case when 
application code pulls references from reference queue and clears them promptly.
2. GC pauses are increased since phantom reachable objects are still to be 
marked.

On the other hand, benefits are not obvious. How we can use referent if it's 
not accessible?

Regards,
Dmytro

  

RFR 8014732: Minor spec issue: java.util.Spliterator.getExactSizeIfKnown

2013-05-29 Thread Paul Sandoz
Hi,

Please review this JavaDoc fixe to j.u.Spliterator.getExactSizeIfKnown.

Paul.

# HG changeset patch
# User psandoz
# Date 1369825085 -7200
# Node ID 840469ba82ab8d8238414a5333aa99b8d4035a9b
# Parent  7ded996200218791c885c0aae4c474066101c7bd
8014732: Minor spec issue: java.util.Spliterator.getExactSizeIfKnown
Summary: A minor documentation issue (not a spec issue).
Reviewed-by:

diff -r 7ded99620021 -r 840469ba82ab 
src/share/classes/java/util/Spliterator.java
--- a/src/share/classes/java/util/Spliterator.java  Wed May 29 12:58:03 
2013 +0200
+++ b/src/share/classes/java/util/Spliterator.java  Wed May 29 12:58:05 
2013 +0200
@@ -394,9 +394,9 @@
  * Convenience method that returns {@link #estimateSize()} if this
  * Spliterator is {@link #SIZED}, else {@code -1}.
  * @implSpec
- * The default returns the result of {@code estimateSize()} if the
- * Spliterator reports a characteristic of {@code SIZED}, and {@code -1}
- * otherwise.
+ * The default implementation returns the result of {@code estimateSize()}
+ * if the Spliterator reports a characteristic of {@code SIZED}, and
+ * {@code -1} otherwise.
  *
  * @return the exact size, if known, else {@code -1}.
  */

RFR 8015008: Primitive iterator over empty sequence, null consumer: forEachRemaining methods do not throw NPE

2013-05-29 Thread Paul Sandoz
Hi,

Please review these changes to j.u.PrimitiveIterator to ensure the default 
forEachRemaining methods consistently throw an NPE when the consumer is null.

I almost produced a webrev for this, but i thought it was just about acceptable 
size-wise and i hope easy to review in textual form. If this is considered 
impolite, awkward to review etc please say so and i will produce a webrev.

Paul.

# HG changeset patch
# User psandoz
# Date 1369825083 -7200
# Node ID 7ded996200218791c885c0aae4c474066101c7bd
# Parent  bfdc1ed75460c9e6869827cf9acabd4b1a5e9d29
8015008: Primitive iterator over empty sequence, null consumer: 
forEachRemaining methods do not throw NPE
Reviewed-by:

diff -r bfdc1ed75460 -r 7ded99620021 
src/share/classes/java/util/PrimitiveIterator.java
--- a/src/share/classes/java/util/PrimitiveIterator.javaWed May 29 
12:58:02 2013 +0200
+++ b/src/share/classes/java/util/PrimitiveIterator.javaWed May 29 
12:58:03 2013 +0200
@@ -91,6 +91,7 @@
  * @throws NullPointerException if the specified action is null
  */
 default void forEachRemaining(IntConsumer action) {
+Objects.requireNonNull(action);
 while (hasNext())
 action.accept(nextInt());
 }
@@ -123,6 +124,8 @@
 forEachRemaining((IntConsumer) action);
 }
 else {
+// The method reference action::accept is never null
+Objects.requireNonNull(action);
 if (Tripwire.ENABLED)
 Tripwire.trip(getClass(), "{0} calling 
PrimitiveIterator.OfInt.forEachRemainingInt(action::accept)");
 forEachRemaining((IntConsumer) action::accept);
@@ -162,6 +165,7 @@
  * @throws NullPointerException if the specified action is null
  */
 default void forEachRemaining(LongConsumer action) {
+Objects.requireNonNull(action);
 while (hasNext())
 action.accept(nextLong());
 }
@@ -194,6 +198,8 @@
 forEachRemaining((LongConsumer) action);
 }
 else {
+// The method reference action::accept is never null
+Objects.requireNonNull(action);
 if (Tripwire.ENABLED)
 Tripwire.trip(getClass(), "{0} calling 
PrimitiveIterator.OfLong.forEachRemainingLong(action::accept)");
 forEachRemaining((LongConsumer) action::accept);
@@ -232,6 +238,7 @@
  * @throws NullPointerException if the specified action is null
  */
 default void forEachRemaining(DoubleConsumer action) {
+Objects.requireNonNull(action);
 while (hasNext())
 action.accept(nextDouble());
 }
@@ -265,6 +272,8 @@
 forEachRemaining((DoubleConsumer) action);
 }
 else {
+// The method reference action::accept is never null
+Objects.requireNonNull(action);
 if (Tripwire.ENABLED)
 Tripwire.trip(getClass(), "{0} calling 
PrimitiveIterator.OfDouble.forEachRemainingDouble(action::accept)");
 forEachRemaining((DoubleConsumer) action::accept);
diff -r bfdc1ed75460 -r 7ded99620021 
test/java/util/Iterator/PrimitiveIteratorDefaults.java
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/test/java/util/Iterator/PrimitiveIteratorDefaults.javaWed May 29 
12:58:03 2013 +0200
@@ -0,0 +1,115 @@
+/*
+ * Copyright (c) 2013, 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.
+ *
+ * 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.
+ */
+
+import org.testng.annotations.Test;
+
+import java.util.PrimitiveIterator;
+import java.util.function.Consumer;
+import java.util.function.DoubleConsumer;
+import java.util.function.IntConsumer;
+import java.util.function.LongConsumer;
+
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+
+/**
+ * @test
+ * @run testng PrimitiveIteratorDefaults
+ * @summary test default

Re: RFR 8014731: j.u.stream.StreamSupport class has default constructor generated

2013-05-29 Thread Alan Bateman

On 29/05/2013 12:12, Paul Sandoz wrote:

Hi,

Please review these changes to j.u.s. StreamSupport to make it final and 
non-instantiable, i also tacked on some JavaDoc broken link fixes made by Henry.

Paul.

This looks okay to me but just to note that making it final is 
technically an API change to track.


-Alan


Re: RFR (S) CR 8014966: Add the proper Javadoc to @Contended

2013-05-29 Thread Aleksey Shipilev
Thanks for the review!

On 05/29/2013 04:28 AM, Martin Buchholz wrote:
> Yeah, we have the same problem with javadoc issuing very verbose
> warnings.  I've been ignoring/filtering them and hoping they get fixed
> before jdk8 ships.

Ok, reverted back to .

> > + * in concurrent contexts in which each instance of the annotated
> > + * object is often accessed by a different thread.
> >
> > Hmmm...  makes it sound like it applies even if each instance is
> > thread-confined (in a different thread) or is immutable.
> 
> I think all the magic of ignoring @C when VM can prove the
> instance/fields would not experience contention, should be omitted from
> the documentation. The practical considerations also apply: since we can
> not at the moment retransform the class after the publication, it seems
> a good idea to treat all instances as potentially shared. This eases the
> reasoning (and documentation) significantly.
> 
> > "objects" are not annotated.
> 
> Yes, should be "instances".
> 
> I was thinking "classes, not objects, are annotated".

Fixed.

>  > however, remain in force for all
>> + * superclass and subclass instances
>>
>>
>> "remain in force for superclass instances" doesn't make sense to me.  Do
>> you mean "remain in force when fields are inherited in subclasses"
> 
> Yes, that sounds better!

Reverted back to David's version. It does sound even cleaner in that
version.


> > Maybe "instances of the annotated class are frequently accessed by
> > multiple threads and have fields that are frequently written".
> 
> "Accessed" matters there. False sharing also happens on reads. Also, the
> innocent read-only classes sometimes also need the protection from the
> adjacent writers' racket :)
> 
> 
> Hmmm... tricky ... This also then applies to pure immutable popular
> objects like Boolean.TRUE. But you want those kinds of objects to all be
> colocated and tightly packed with other such objects, not give each of
> them their own cache line.

Yes, that's true. I do, however, think we have to have the provisioning
for the read-only protected fields/instances.

> Sounds like a Quality of Implementation issue.  In general, I think you
> *DO* want the subclass fields to be potentially in the same location as
> the superclass padding fields.
> A >: B
> A layout: 
> B layout: 
>  
> which is not asking too much of a jit.
> I believe it is already common for VMs to allocate subclass fields in
> natural padding space of superclasses.
> 

This gets very tricky even in a slightly complicated case:
  A >: B
  A layout: AACC
  B layout: AABBppCC

Note that we can't move CC in B because the fields are bound statically;
so we end up ruining the padding for both BB and CC. We can, of course,
have the gap in the padding to tolerate this, but before knowing in
advance what subclasses will be loaded next, we can not prepare enough;
or, we can redefine all the classes up the hierarchy... (Current HotSpot
classloader code is completely not ready for things like these).

However, implementation issues aside:

Allowing contended tags to be inherited will force users to look at the
entire hierarchy to search for the colliding tags; or (what's worse)
push the superclass maintainers to use cryptic tags so no possible
subclasses can collide with the protected fields. IMO, that is way
messier than pushing users to aggregate the semantically-close fields in
the same class. Hence, I left the inheritance clause as is.

The new webrev is here:
  http://cr.openjdk.java.net/~shade/8014966/webrev.03/

Thanks everyone, writing specs is fun!

-Aleksey.


RFR 8014409: Spec typo: extra } in the spec for j.u.s.StreamBuilder

2013-05-29 Thread Paul Sandoz
Hi,

Please review these JavaDoc fixes to j.u.s.StreamBuilder.

Paul.

# HG changeset patch
# User psandoz
# Date 1369825073 -7200
# Node ID 303e9d2aff3cbaf27823b2591f2e8570b77afcce
# Parent  bd6d3801347bfd912507d16dc14488f47e94e626
8014409: Spec typo: extra } in the spec for j.u.s.StreamBuilder
Summary: Also fixes documentation on StreamBuilder.OfDouble
Reviewed-by:

diff -r bd6d3801347b -r 303e9d2aff3c 
src/share/classes/java/util/stream/StreamBuilder.java
--- a/src/share/classes/java/util/stream/StreamBuilder.java Wed May 29 
09:42:39 2013 +0200
+++ b/src/share/classes/java/util/stream/StreamBuilder.java Wed May 29 
12:57:53 2013 +0200
@@ -38,7 +38,7 @@
  * A {@code StreamBuilder} has a lifecycle, where it starts in a building
  * phase, during which elements can be added, and then transitions to a built
  * phase, after which elements may not be added.  The built phase begins
- * when the {@link #build()}} method is called, which creates an ordered
+ * when the {@link #build()} method is called, which creates an ordered
  * {@code Stream} whose elements are the elements that were added to the stream
  * builder, in the order they were added.
  *
@@ -98,7 +98,7 @@
  * A stream builder has a lifecycle, where it starts in a building
  * phase, during which elements can be added, and then transitions to a
  * built phase, after which elements may not be added.  The built phase
- * begins when the {@link #build()}} method is called, which creates an
+ * begins when the {@link #build()} method is called, which creates an
  * ordered stream whose elements are the elements that were added to the
  * stream builder, in the order they were added.
  *
@@ -155,7 +155,7 @@
  * A stream builder has a lifecycle, where it starts in a building
  * phase, during which elements can be added, and then transitions to a
  * built phase, after which elements may not be added.  The built phase
- * begins when the {@link #build()}} method is called, which creates an
+ * begins when the {@link #build()} method is called, which creates an
  * ordered stream whose elements are the elements that were added to the
  * stream builder, in the order they were added.
  *
@@ -209,6 +209,13 @@
 /**
  * A mutable builder for a {@code DoubleStream}.
  *
+ * A stream builder  has a lifecycle, where it starts in a building
+ * phase, during which elements can be added, and then transitions to a
+ * built phase, after which elements may not be added.  The built phase
+ * begins when the {@link #build()} method is called, which creates an
+ * ordered stream whose elements are the elements that were added to the
+ * stream builder, in the order they were added.
+ *
  * @see LongStream#builder()
  * @since 1.8
  */
@@ -217,13 +224,6 @@
 /**
  * Adds an element to the stream being built.
  *
- * A stream builder  has a lifecycle, where it starts in a building
- * phase, during which elements can be added, and then transitions to a
- * built phase, after which elements may not be added.  The built phase
- * begins when the {@link #build()}} method is called, which creates an
- * ordered stream whose elements are the elements that were added to 
the
- * stream builder, in the order they were added.
- *
  * @throws IllegalStateException if the builder has already 
transitioned
  * to the built state
  */

RFR 8014731: j.u.stream.StreamSupport class has default constructor generated

2013-05-29 Thread Paul Sandoz
Hi,

Please review these changes to j.u.s. StreamSupport to make it final and 
non-instantiable, i also tacked on some JavaDoc broken link fixes made by Henry.

Paul.

# HG changeset patch
# User psandoz
# Date 1369825082 -7200
# Node ID bfdc1ed75460c9e6869827cf9acabd4b1a5e9d29
# Parent  6be3ce51e61dbdab6766c74223c076a7b3472be6
8014731: j.u.stream.StreamSupport class has default constructor generated
Summary: This change set also fixes broken links
Reviewed-by:
Contributed-by: Paul Sandoz , Henry Jen 


diff -r 6be3ce51e61d -r bfdc1ed75460 
src/share/classes/java/util/stream/StreamSupport.java
--- a/src/share/classes/java/util/stream/StreamSupport.java Tue May 28 
15:22:30 2013 +0200
+++ b/src/share/classes/java/util/stream/StreamSupport.java Wed May 29 
12:58:02 2013 +0200
@@ -41,7 +41,11 @@
  *
  * @since 1.8
  */
-public class StreamSupport {
+public final class StreamSupport {
+
+// Suppresses default constructor, ensuring non-instantiability.
+private StreamSupport() {}
+
 /**
  * Creates a new sequential {@code Stream} from a {@code Spliterator}.
  *
@@ -50,7 +54,7 @@
  *
  * It is strongly recommended the spliterator report a characteristic of
  * {@code IMMUTABLE} or {@code CONCURRENT}, or be
- * late-binding.  Otherwise,
+ * late-binding.  Otherwise,
  * {@link #stream(Supplier, int)} should be used to
  * reduce the scope of potential interference with the source.  See
  * Non-Interference for
@@ -75,7 +79,7 @@
  *
  * It is strongly recommended the spliterator report a characteristic of
  * {@code IMMUTABLE} or {@code CONCURRENT}, or be
- * late-binding.  Otherwise,
+ * late-binding.  Otherwise,
  * {@link #stream(Supplier, int)} should be used to
  * reduce the scope of potential interference with the source.  See
  * Non-Interference for
@@ -102,7 +106,7 @@
  *
  * For spliterators that report a characteristic of {@code IMMUTABLE}
  * or {@code CONCURRENT}, or that are
- * late-binding, it is likely
+ * late-binding, it is likely
  * more efficient to use {@link #stream(java.util.Spliterator)} instead.
  * The use of a {@code Supplier} in this form provides a level of
  * indirection that reduces the scope of potential interference with the
@@ -138,7 +142,7 @@
  *
  * For spliterators that report a characteristic of {@code IMMUTABLE}
  * or {@code CONCURRENT}, or that are
- * late-binding, it is likely
+ * late-binding, it is likely
  * more efficient to use {@link #stream(Spliterator)} instead.
  * The use of a {@code Supplier} in this form provides a level of
  * indirection that reduces the scope of potential interference with the
@@ -172,7 +176,7 @@
  *
  * It is strongly recommended the spliterator report a characteristic of
  * {@code IMMUTABLE} or {@code CONCURRENT}, or be
- * late-binding.  Otherwise,
+ * late-binding.  Otherwise,
  * {@link #stream(Supplier, int)}} should be used to
  * reduce the scope of potential interference with the source.  See
  * Non-Interference for
@@ -195,7 +199,7 @@
  *
  * It is strongly recommended the spliterator report a characteristic of
  * {@code IMMUTABLE} or {@code CONCURRENT}, or be
- * late-binding.  Otherwise,
+ * late-binding.  Otherwise,
  * {@link #stream(Supplier, int)}} should be used to
  * reduce the scope of potential interference with the source.  See
  * Non-Interference for
@@ -220,7 +224,7 @@
  *
  * For spliterators that report a characteristic of {@code IMMUTABLE}
  * or {@code CONCURRENT}, or that are
- * late-binding, it is likely
+ * late-binding, it is likely
  * more efficient to use {@link #intStream(Spliterator.OfInt)} instead.
  * The use of a {@code Supplier} in this form provides a level of
  * indirection that reduces the scope of potential interference with the
@@ -254,7 +258,7 @@
  *
  * For spliterators that report a characteristic of {@code IMMUTABLE}
  * or {@code CONCURRENT}, or that are
- * late-binding, it is likely
+ * late-binding, it is likely
  * more efficient to use {@link #intStream(Spliterator.OfInt)} instead.
  * The use of a {@code Supplier} in this form provides a level of
  * indirection that reduces the scope of potential interference with the
@@ -286,7 +290,7 @@
  *
  * It is strongly recommended the spliterator report a characteristic of
  * {@code IMMUTABLE} or {@code CONCURRENT}, or be
- * late-binding.  Otherwise,
+ * late-binding.  Otherwise,
  * {@link #stream(Supplier, int)} should be used to
  * reduce the scope of potential interference with the source.  See
  * Non-Interference for
@@ -310,7 +314,7 @@
  *
  * It is strongly recommended the spliterator report a characteristic of
  * {@code IMMUTABLE} or {@code CONCURRENT}, or be
- * late-binding.  Otherwise,
+

RFR 8014393: Minor typo in the spec for j.u.stream.Stream.findFirst()

2013-05-29 Thread Paul Sandoz
Hi,

Please review these JavaDoc fixes to j.u.s.{Xxxx}Stream.findFirst.

Paul.

# HG changeset patch
# User psandoz
# Date 1369747350 -7200
# Node ID 6be3ce51e61dbdab6766c74223c076a7b3472be6
# Parent  303e9d2aff3cbaf27823b2591f2e8570b77afcce
8014393: Minor typo in the spec for j.u.stream.Stream.findFirst()
Reviewed-by:

diff -r 303e9d2aff3c -r 6be3ce51e61d 
src/share/classes/java/util/stream/DoubleStream.java
--- a/src/share/classes/java/util/stream/DoubleStream.java  Wed May 29 
12:57:53 2013 +0200
+++ b/src/share/classes/java/util/stream/DoubleStream.java  Tue May 28 
15:22:30 2013 +0200
@@ -603,7 +603,7 @@
 /**
  * Returns an {@link OptionalDouble} describing the first element of this
  * stream (in the encounter order), or an empty {@code OptionalDouble} if
- * the stream is empty.  If the stream has no encounter order, than any
+ * the stream is empty.  If the stream has no encounter order, then any
  * element may be returned.
  *
  * This is a short-circuiting
diff -r 303e9d2aff3c -r 6be3ce51e61d 
src/share/classes/java/util/stream/IntStream.java
--- a/src/share/classes/java/util/stream/IntStream.java Wed May 29 12:57:53 
2013 +0200
+++ b/src/share/classes/java/util/stream/IntStream.java Tue May 28 15:22:30 
2013 +0200
@@ -588,7 +588,7 @@
 /**
  * Returns an {@link OptionalInt} describing the first element of this
  * stream (in the encounter order), or an empty {@code OptionalInt} if the
- * stream is empty.  If the stream has no encounter order, than any element
+ * stream is empty.  If the stream has no encounter order, then any element
  * may be returned.
  *
  * This is a short-circuiting
diff -r 303e9d2aff3c -r 6be3ce51e61d 
src/share/classes/java/util/stream/LongStream.java
--- a/src/share/classes/java/util/stream/LongStream.javaWed May 29 
12:57:53 2013 +0200
+++ b/src/share/classes/java/util/stream/LongStream.javaTue May 28 
15:22:30 2013 +0200
@@ -588,7 +588,7 @@
 /**
  * Returns an {@link OptionalLong} describing the first element of this
  * stream (in the encounter order), or an empty {@code OptionalLong} if the
- * stream is empty.  If the stream has no encounter order, than any element
+ * stream is empty.  If the stream has no encounter order, then any element
  * may be returned.
  *
  * This is a short-circuiting
diff -r 303e9d2aff3c -r 6be3ce51e61d 
src/share/classes/java/util/stream/Stream.java
--- a/src/share/classes/java/util/stream/Stream.javaWed May 29 12:57:53 
2013 +0200
+++ b/src/share/classes/java/util/stream/Stream.javaTue May 28 15:22:30 
2013 +0200
@@ -754,7 +754,7 @@
 /**
  * Returns an {@link Optional} describing the first element of this stream
  * (in the encounter order), or an empty {@code Optional} if the stream is
- * empty.  If the stream has no encounter order, than any element may be
+ * empty.  If the stream has no encounter order, then any element may be
  * returned.
  *
  * This is a short-circuiting

Re: 8015470: (ann) IncompleteAnnotationException does not need to call toString

2013-05-29 Thread Otávio Gonçalves de Santana
Thank you everyone.
I searched classes with this situation and I find these classes:


   - sun.tools.java.MemberDefinition
   - sun.rmi.rmic.Main
   - sun.tools.jconsole.inspector.Utils
   - com.sun.jndi.toolkit.dir.SearchFilter
   - javax.swing.text.html.FormView



The diffs is in attachment


On Tue, May 28, 2013 at 10:31 PM, David Holmes wrote:

> On 28/05/2013 11:08 PM, Remi Forax wrote:
>
>> On 05/28/2013 02:48 PM, David Holmes wrote:
>>
>>> Sorry it didn't register that getName() already returns a String -
>>> hence the toString() is redundant - but minimally so.
>>>
>>> David
>>>
>>
>> The second call to toString() also performs an implicit nullcheck
>> (elementName can not be null).
>> So if we have to fix something, it's only to remove the call to
>> toString() after the call to getName().
>>
>
> Good catch Remi!
>
> Otavio: I don't want to dissuade you from getting involved but as Remi
> indicates your suggested change becomes simply
>
>
> -super(annotationType.getName()**.toString() +
> +super(annotationType.getName() +
>
> and this is such a minor change to interpreted performance (the JIT will
> remove the toString call) that I don't think it is worth the process
> overhead (testing etc) to actually make this change. As I said in other
> email sometimes there are non-obvious reasons (like null checks :) ) that
> code has to be kept in its current form.
>
> David
> -
>
>
>  cheers,
>> Rémi
>>
>>
>>> On 28/05/2013 9:15 PM, David Holmes wrote:
>>>
 Please see my reply under your original subject line.

 This is not a bug.

 David

 On 28/05/2013 7:37 PM, Otávio Gonçalves de Santana wrote:

> diff --git
> a/src/share/classes/java/lang/**annotation/**
> IncompleteAnnotationException.**java
>
>
> b/src/share/classes/java/lang/**annotation/**
> IncompleteAnnotationException.**java
>
>
> ---
> a/src/share/classes/java/lang/**annotation/**
> IncompleteAnnotationException.**java
>
>
> +++
> b/src/share/classes/java/lang/**annotation/**
> IncompleteAnnotationException.**java
>
>
> @@ -55,9 +55,9 @@
>   public IncompleteAnnotationException(
>   Class annotationType,
>   String elementName) {
> -super(annotationType.getName()**.toString() +
> +super(annotationType.getName() +
> " missing element " +
> -  elementName.toString());
> +  elementName);
>
>   this.annotationType = annotationType;
>   this.elementName = elementName;
>
>
>
>>


-- 
Atenciosamente.

Otávio Gonçalves de Santana

blog: http://otaviosantana.blogspot.com.br/
twitter: http://twitter.com/otaviojava
site: http://www.otaviojava.com.br
(11) 98255-3513


hg: jdk8/tl/langtools: 7053059: VerifyError with double Assignment using a Generic Member of a Superclass

2013-05-29 Thread vicente . romero
Changeset: 92e420e9807d
Author:vromero
Date:  2013-05-29 10:56 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/92e420e9807d

7053059: VerifyError with double Assignment using a Generic Member of a 
Superclass
Reviewed-by: mcimadamore

! src/share/classes/com/sun/tools/javac/comp/TransTypes.java
+ test/tools/javac/T7053059/VerifyErrorWithDoubleAssignmentTest.java



hg: jdk8/tl/jdk: 8014928: (fs) Files.readAllBytes() copies content to new array when content completely read

2013-05-29 Thread alan . bateman
Changeset: 2b3242a69a44
Author:alanb
Date:  2013-05-29 10:24 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2b3242a69a44

8014928: (fs) Files.readAllBytes() copies content to new array when content 
completely read
Reviewed-by: martin

! src/share/classes/java/nio/file/Files.java



Re: RFR 8005704: Update ConcurrentHashMap to v8

2013-05-29 Thread David Holmes

On 29/05/2013 2:29 PM, Martin Buchholz wrote:

On Tue, May 28, 2013 at 9:00 PM, David Holmes mailto:david.hol...@oracle.com>> wrote:
On 29/05/2013 5:53 AM, Martin Buchholz wrote:

Is atomicity part of the contract of ConcurrentMap.getOrDefault?
   Currently, it doesn't say.
Actually, there are two possible guarantees it could make -
whether the
default implementation ConcurrentMap.getOrDefault is atomic
(when the map
does not accept nulls) and whether subclasses that override
getOrDefault
must make this guarantee.


I want to re-examine the new methods that ConcurrentMap is
inheriting from Map because I think more statements regarding
concurrency properties are required. This included getOrDefault even
though overridden.


Agreed.


---

Curiously, ConcurrentMap doesn't guarantee that get() is atomic.
Perhaps it should?


Only composite operations (those that could expressed as a sequence
of other operations) need an atomicity guarantee. Operations like
get() simply need to be thread-safe.


You're right.  Let me rephrase:

Curiously, ConcurrentMap doesn't guarantee that get() is thread-safe, or
that a ConcurrentMap can be used from multiple threads.  Perhaps it should?


Right. I find this:

 * A {@link java.util.Map} providing additional atomic
 * {@code putIfAbsent}, {@code remove}, and {@code replace} methods.

rather lacking looking at it today. First the list needs updating - 
though it was suggested by Doug that we stop listing the methods and 
simply make a general statement. Somewhat disturbingly I now see that 
some of the new Map methods may not in fact be "atomic".


Second it says nothing about thread-safety. The only reference to 
thread-safety is in the j.u.c package-info docs:


" A concurrent collection is thread-safe, but not governed by a single 
exclusion lock."


I went back to my email archives from when we put this altogether back 
in 2002/2003 and I found no reference to ever actually saying in 
ConcurrentMap that the Map was thread-safe. This may in part be because 
what we called ConcurrentMap simply started out as an interface in which 
to put putIfAbsent and we really only cared about the fact that 
ConcurrentHashMap would implement it - and we couldn't call it Map.


David


Generics in enums

2013-05-29 Thread Victor Polischuk



Greetings,

>


>

Some time ago I wanted to migrate our "commons-lang" enums to "java 5"
enumerations, but I encountered an issue that it cannot be done without
discarding generics since java enums do not support them. Let me show an
example:

>

//--

>

public final class ColorEnum extends
org.apache.commons.lang.enums.Enum {
public static final ColorEnum RED = new
ColorEnum("Red");
public static final ColorEnum GREEN = new
ColorEnum("Green");
public static final ColorEnum BLUE = new
ColorEnum("Blue");
public static final ColorEnum WHITE = new
ColorEnum("White") {

>

@Override

>

public MixedPixel make() {...} 

>

};

private ColorEnum(String color) {super(color);}

>


>

public boolean filter(T pixel) {...}

>


>

public T make() {...}
}

>

//--

>

And I wonder if there is a specific reason why I cannot convert it into
something like: 

>

//--

>

>

public enum Color {
RED("Red"),
GREEN("Green"),

>

BLUE ("Blue"),

>

WHITE("White") {

>

>

@Override

>

public MixedPixel make() {...} 

> };


private Color(String color) {...}

>


>

public boolean filter(T pixel) {...}

>


>

public T make() {...}
}

> //--

Thank you in advance.

>


>


>

Sincerely yours,

>

Victor Polischuk

>


hg: jdk8/tl/jdk: 8015440: java/lang/management/MemoryMXBean/ResetPeakMemoryUsage.java fails with RuntimeException

2013-05-29 Thread staffan . larsen
Changeset: bd6d3801347b
Author:sla
Date:  2013-05-29 09:42 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bd6d3801347b

8015440: java/lang/management/MemoryMXBean/ResetPeakMemoryUsage.java fails with 
RuntimeException
Summary: Make sure serial gc compacts heap every time
Reviewed-by: mchung, brutisso, nloodin

! test/java/lang/management/MemoryMXBean/ResetPeakMemoryUsage.java