date:20130419


Hi,

I get the following compile-time error:

/home/peter/work/hg/jdk8-tl/langtools/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java:264: 
error: illegal start of expression

if (qualifiedName == null) }
   ^


...isn't this '}' supposed to be '{' ?

http://dl.dropboxusercontent.com/u/101777488/jdk8-tl/tools-FileDocImpl/webrev/index.html


Regards, Peter

Re: can't build current jdk8/tl tip


On 19/04/2013 08:08, Peter Levart wrote:

Hi,

I get the following compile-time error:

/home/peter/work/hg/jdk8-tl/langtools/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java:264: 
error: illegal start of expression

if (qualifiedName == null) }
I'm seeing the same thing, looks like the typo crept in with this change 
last night:


http://hg.openjdk.java.net/jdk8/tl/langtools/rev/95d29b99e5b3

-Alan

hg: jdk8/tl/jdk: 8009636: JARSigner including TimeStamp PolicyID (TSAPolicyID) as defined in RFC3161

2013-04-19 Thread weijun . wang

Changeset: 778b16225d85
Author:weijun
Date:  2013-04-19 15:41 +0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/778b16225d85

8009636: JARSigner including TimeStamp PolicyID (TSAPolicyID) as defined in 
RFC3161
Reviewed-by: mullan

! src/share/classes/com/sun/jarsigner/ContentSignerParameters.java
! src/share/classes/sun/security/pkcs/PKCS7.java
! src/share/classes/sun/security/timestamp/TSRequest.java
! src/share/classes/sun/security/timestamp/TimestampToken.java
! src/share/classes/sun/security/tools/jarsigner/Main.java
! src/share/classes/sun/security/tools/jarsigner/Resources.java
! src/share/classes/sun/security/tools/jarsigner/TimestampedSigner.java
! test/sun/security/tools/jarsigner/TimestampCheck.java
! test/sun/security/tools/jarsigner/ts.sh

RFR 8012681: Commit for JDK-8012656 breaks tl build Was: can't build current jdk8/tl tip

2013-04-19 Thread Joel Borggrén-Franck


Hi

On 04/19/2013 09:30 AM, Alan Bateman wrote:> On 19/04/2013 08:08, Peter 
Levart wrote:

>> Hi,
>>
>> I get the following compile-time error:
>>
>> 
/home/peter/work/hg/jdk8-tl/langtools/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java:264: 


>> error: illegal start of expression
>> if (qualifiedName == null) }
> I'm seeing the same thing, looks like the typo crept in with this change
> last night:
>
> http://hg.openjdk.java.net/jdk8/tl/langtools/rev/95d29b99e5b3
>

See inline patch to fix this. Since TL isn't building I think we can review 
this on core-libs-dev although technically a compiler issue.


With this fix langtools builds and all langtools tests passes.

cheers
/Joel

diff --git a/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java 
b/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java

--- a/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java
+++ b/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java
@@ -261,7 +261,7 @@
 private String name;

 public String qualifiedName() {
-if (qualifiedName == null) }
+if (qualifiedName == null) {
 qualifiedName = sym.enclClass().getQualifiedName() + "." + name();
 }
 return qualifiedName;

Re: RFR 8012681: Commit for JDK-8012656 breaks tl build Was: can't build current jdk8/tl tip

2013-04-19 Thread Chris Hegarty


Looks good to me Joel,

-Chris.

On 19/04/2013 09:49, Joel Borggrén-Franck wrote:

Hi

On 04/19/2013 09:30 AM, Alan Bateman wrote:> On 19/04/2013 08:08, Peter
Levart wrote:
 >> Hi,
 >>
 >> I get the following compile-time error:
 >>
 >>
/home/peter/work/hg/jdk8-tl/langtools/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java:264:

 >> error: illegal start of expression
 >> if (qualifiedName == null) }
 > I'm seeing the same thing, looks like the typo crept in with this change
 > last night:
 >
 > http://hg.openjdk.java.net/jdk8/tl/langtools/rev/95d29b99e5b3
 >

See inline patch to fix this. Since TL isn't building I think we can
review this on core-libs-dev although technically a compiler issue.

With this fix langtools builds and all langtools tests passes.

cheers
/Joel

diff --git a/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java
b/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java
--- a/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java
+++ b/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java
@@ -261,7 +261,7 @@
private String name;

public String qualifiedName() {
- if (qualifiedName == null) }
+ if (qualifiedName == null) {
qualifiedName = sym.enclClass().getQualifiedName() + "." + name();
}
return qualifiedName;

Re: RFR JDK-8011426: java.util collection Spliterator implementations

On Apr 19, 2013, at 6:52 AM, Mike Duigou  wrote:

> I reversed this change :
> 
> -final Collection c;
> -
> +final Collection c;
> 
> 
> in Collections.UnmodifiableCollection instead opting or casts in the forEach 
> and spliterator Methods.
> 

OK, i prefer the former but i ain't gonna argue this one :-)

> 
> - I wonder if it's worth it to have the NONNULL characteristic change in 
> Collections::singletonSpliterator depending on element. 
> 

My preference is to be accurate if it is cheap to do.

> LinkedHashMap::
> 
> - needs an overridden spliterator providing ORDERED for it's entry set. (I 
> can do this tomorrow if needed).
> 

Right, the key/value/entry collections all have an encounter order. 

Note that one cannot leverage the HashMap spliterator implementations, we need 
to create a spliterator from the collection (basically delayed use of the 
collection's iterator), hence the following check in HashMap:

if (HashMap.this.getClass() == HashMap.class)

Doing something more optimal for LinkedHashMap would require a lot more work.

> PriorityQueue::
> 
> - I am surprised the spliterator is not ORDERED.
> 

Yes, that surprised me at first.  Guarantees on order are only made for the 
head, plus the order is not stable for two values that tie for the least value.

Paul.

Re: RFR 8012681: Commit for JDK-8012656 breaks tl build Was: can't build current jdk8/tl tip


On 19/04/2013 09:49, Joel Borggrén-Franck wrote:


See inline patch to fix this. Since TL isn't building I think we can 
review this on core-libs-dev although technically a compiler issue.


With this fix langtools builds and all langtools tests passes.

Looks fine, this is what Peter Levart suggested too.

-Alan

hg: jdk8/tl/jdk: 8010505: HTTP DIGEST implementation incorrectly quotes header values, fails auth

2013-04-19 Thread chris . hegarty

Changeset: 90b03f9a2e77
Author:jzavgren
Date:  2013-04-17 11:47 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/90b03f9a2e77

8010505: HTTP DIGEST implementation incorrectly quotes header values, fails auth
Summary: The extraneous quotes were removed.
Reviewed-by: chegar

! src/share/classes/sun/net/www/protocol/http/DigestAuthentication.java

hg: jdk8/tl/langtools: 8012681: Commit for JDK-8012656 breaks tl build

2013-04-19 Thread joel . franck

Changeset: a3655c24e232
Author:jfranck
Date:  2013-04-19 11:57 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/a3655c24e232

8012681: Commit for JDK-8012656 breaks tl build
Reviewed-by: vromero, chegar, alanb

! src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java

Re: RFR 8012681: Commit for JDK-8012656 breaks tl build Was: can't build current jdk8/tl tip

2013-04-19 Thread Joel Borggrén-Franck


Fix pushed, thanks for the reviews.

Peter, I missed your patch. My fix should probably have been attributed to 
you, sorry!


cheers
/Joel

On 04/19/2013 10:49 AM, Joel Borggrén-Franck wrote:

Hi

On 04/19/2013 09:30 AM, Alan Bateman wrote:> On 19/04/2013 08:08, Peter
Levart wrote:
 >> Hi,
 >>
 >> I get the following compile-time error:
 >>
 >>
/home/peter/work/hg/jdk8-tl/langtools/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java:264:

 >> error: illegal start of expression
 >> if (qualifiedName == null) }
 > I'm seeing the same thing, looks like the typo crept in with this change
 > last night:
 >
 > http://hg.openjdk.java.net/jdk8/tl/langtools/rev/95d29b99e5b3
 >

See inline patch to fix this. Since TL isn't building I think we can review
this on core-libs-dev although technically a compiler issue.

With this fix langtools builds and all langtools tests passes.

cheers
/Joel

diff --git a/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java
b/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java
--- a/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java
+++ b/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java
@@ -261,7 +261,7 @@
  private String name;

  public String qualifiedName() {
-if (qualifiedName == null) }
+if (qualifiedName == null) {
  qualifiedName = sym.enclClass().getQualifiedName() + "." +
name();
  }
  return qualifiedName;

Re: RFR : 8008632 : Additional JavaDoc tags @apiNote, @implSpec and @implNote for JDK API Docs


On 18/04/2013 16:12, Mike Duigou wrote:

:


@implSpec is very welcome, more reasons now with the addition of default 
methods.

Clearly distinguishing normative from non-normative text is also been a long standing 
problem so having a tag to identify the non-normative text is very useful. I had to look 
at a few examples in the lambda/jdk repo to convince myself that both @apiNote and 
@implNote are needed. One thing that isn't clear to me is whether the 
"official" Java SE documentation should include the text tagged @implNote or 
not.

Once we got going we decided to fill out the matrix. There's no obligation to 
use all of the tags.

Where @implSpec is used, then do you have a view on whether this should 
be filtered out of the Java SE specs?


-Alan

Re: RFR: 8001647: In-place methods on Collection/List


On 18/04/2013 19:49, Akhil Arora wrote:

Looks like the stars are aligning on getting on this into TL... the
refreshed webrev is -

http://cr.openjdk.java.net/~akhil/8001647.8/webrev/

A minor comment on Collection.removeIf is "that satisifies the given 
predicate" might be better than "which matches the provided predicate". 
Also for completeness, you could say "RuntimeExceptions and Errors 
thrown by the predicate are propagated ...".


In List.replaceAll then @throws NullPointerException is listed twice, 
which is okay, but might be better to combine them. A typo in the second 
NPE description "if the an element".


In the implementation then the only thing that puzzled me is checking 
the modification count in legacy Vector, that seems unnecessary.


I see Mike has looked through the tests in detail. One additional 
comment is just location and trying to keep it consistent with the 
current organization if possible.


-Alan.

Re: RFR: 8001647: In-place methods on Collection/List


On Apr 19, 2013, at 1:15 PM, Alan Bateman  wrote:

> On 18/04/2013 19:49, Akhil Arora wrote:
>> Looks like the stars are aligning on getting on this into TL... the
>> refreshed webrev is -
>> 
>> http://cr.openjdk.java.net/~akhil/8001647.8/webrev/
>> 
> A minor comment on Collection.removeIf is "that satisifies the given 
> predicate" might be better than "which matches the provided predicate". Also 
> for completeness, you could say "RuntimeExceptions and Errors thrown by the 
> predicate are propagated ...".
> 
> In List.replaceAll then @throws NullPointerException is listed twice, which 
> is okay, but might be better to combine them. A typo in the second NPE 
> description "if the an element".
> 
> In the implementation then the only thing that puzzled me is checking the 
> modification count in legacy Vector, that seems unnecessary.
> 

The function value could structurally modify the Vector instance.

Paul.

Re: RFR: 8001647: In-place methods on Collection/List

2013-04-19 Thread David Holmes


Hi Akhil,

This is only a partial review so far.

I have some issues with the ConcurrentModificationException strategy.

Taking ArrayList, the basic idea is that you want to detect 
modifications that are concurrent with iteration - so the mutators up 
the modCount and the iterator functions check to see if the modCount has 
changed since the iterator was constructed. So you've then treated the 
bulk operations as "iterations" and you check the modCount each time 
through the loop. Problem is the modCount is not volatile so in all 
likelihood the JIT is going to hoist the check of modCount out of the 
loop and it will only be checked once. That's not incorrect as it is 
"best effort" detection, but it might be surprising. (Note if existing 
iterator methods get inlined the same problem can exist today.) Maybe it 
is not worth trying to check this? The reality is that if the ArrayList 
is really being modified concurrently - particularly if the size changes 
- then you will just get exceptions (NPE, AIOOBE etc) not a nice neat CME.


It's late and I may have overlooked ssomething but in Vector all the 
methods are synchronized yet you still check the modCount for changes ?? 
But it is impossible for anyone to change modCount during a bullk 
operation. It is only possible with iteration because a mutating method 
in one thread can execute between the separate iterator hasNext()/next() 
calls in another thread. So all the CME code can be dropped from the new 
bulk methods.


Cheers,
David

On 19/04/2013 4:49 AM, Akhil Arora wrote:

Looks like the stars are aligning on getting on this into TL... the
refreshed webrev is -

http://cr.openjdk.java.net/~akhil/8001647.8/webrev/

Please review

Thanks

On 12/10/2012 09:31 PM, Akhil Arora wrote:

http://cr.openjdk.java.net/~akhil/8001647.3/webrev/

- now with synchronized and unmodifiable wrappers in Collections.java
for the default methods being added

On 12/10/2012 01:48 PM, Akhil Arora wrote:

Updated with yours and Alan's comments -

http://cr.openjdk.java.net/~akhil/8001647.2/webrev/

- removed null check for removeSet
- cache this.size in removeAll, replaceAll
 (for ArrayList, Vector and CopyOnWriteArrayList)
- calculate removeCount instead of BitCount.cardinality()
- removed unnecessary @library from test support classes

Re: RFR: 8001647: In-place methods on Collection/List


On 19/04/2013 13:14, Paul Sandoz wrote:

:
The function value could structurally modify the Vector instance.


Ah yes, got it - thanks.

For removeIf then I assume it isn't required in the pass over the removeSet.

-Alan.

Re: RFR: 8001647: In-place methods on Collection/List

2013-04-19 Thread David Holmes

On 19/04/2013 10:14 PM, Paul Sandoz wrote:

On Apr 19, 2013, at 1:15 PM, Alan Bateman wrote:

On 18/04/2013 19:49, Akhil Arora wrote:

Looks like the stars are aligning on getting on this into TL... the
refreshed webrev is -

http://cr.openjdk.java.net/~akhil/8001647.8/webrev/

A minor comment on Collection.removeIf is "that satisifies the given predicate" might be better
than "which matches the provided predicate". Also for completeness, you could say
"RuntimeExceptions and Errors thrown by the predicate are propagated ...".

In List.replaceAll then @throws NullPointerException is listed twice, which is okay, but
might be better to combine them. A typo in the second NPE description "if the an
element".

In the implementation then the only thing that puzzled me is checking the
modification count in legacy Vector, that seems unnecessary.

The function value could structurally modify the Vector instance.

So this came through while I was writing my similar comments ...

My reaction to this is simply "well don't do that". If the
function/predicate/comparator is mutating the Vector then the user gets
what they deserve in my opinion. Trying to account for that is somewhat
futile. As per my other email the loop check for
modCount==expectedModCount will get hoisted from the loop. Further in
removeIf you need to be a lot more defensive during the first iteration
as you haven't kept a reference to the original size and array. That
aside the second part of removeIf (the actual removal) doesn't invoke
any external code so no concurrent modification is possible then.

This seems like overkill to me.

Cheers,
David

Paul.

Re: RFR: 8001647: In-place methods on Collection/List


On Apr 19, 2013, at 2:29 PM, David Holmes  wrote:

> Hi Akhil,
> 
> This is only a partial review so far.
> 
> I have some issues with the ConcurrentModificationException strategy.
> 
> Taking ArrayList, the basic idea is that you want to detect 
> modifications that are concurrent with iteration - so the mutators up 
> the modCount and the iterator functions check to see if the modCount has 
> changed since the iterator was constructed. So you've then treated the 
> bulk operations as "iterations" and you check the modCount each time 
> through the loop. Problem is the modCount is not volatile so in all 
> likelihood the JIT is going to hoist the check of modCount out of the 
> loop and it will only be checked once. That's not incorrect as it is 
> "best effort" detection, but it might be surprising. (Note if existing 
> iterator methods get inlined the same problem can exist today.) Maybe it 
> is not worth trying to check this? The reality is that if the ArrayList 
> is really being modified concurrently - particularly if the size changes 
> - then you will just get exceptions (NPE, AIOOBE etc) not a nice neat CME.
> 
> It's late and I may have overlooked ssomething but in Vector all the 
> methods are synchronized yet you still check the modCount for changes ?? 
> But it is impossible for anyone to change modCount during a bullk 
> operation. It is only possible with iteration because a mutating method 
> in one thread can execute between the separate iterator hasNext()/next() 
> calls in another thread. So all the CME code can be dropped from the new 
> bulk methods.
> 

  Vector v = new Vector<>(Arrays.asList(1, 2, 3));

  // Any of the following will throw a CME
  v.forEach(e -> v.add(e));
  v.iterator().forEachRemaining(e -> v.add(e));
  v.spliterator().forEachRemaining(e -> v.add(e));

Paul.

Re: Proxy.isProxyClass scalability


Hi Mandy,

On 04/19/2013 07:33 AM, Mandy Chung wrote:


https://dl.dropboxusercontent.com/u/101777488/jdk8-tl/proxy-wc/webrev.02/index.html 

What about package-private in java.lang.reflect? It makes Proxy 
itself much easier to read. When we decide which way to go, I can 
remove the interface and only leave a single package-private class...




thanks.  Moving it to a single package-private classin 
java.lang.reflectand remove the interface sounds good.


Right.



I have merged your patch with the recent TL repo and did some clean up 
while reviewing it.  Some comments:
1. getProxyClass0 should validate the input interfaces and throw IAE 
if any illegal argument (e.g. interfaces are not visible to the given 
loader) before calling proxyClassCache.get(loader, interfaces). I 
moved back the validation code from ProxyClassFactory.apply to 
getProxyClass0.


Ops, you're right. There could be a request with interface(s) with same 
name(s) but loaded by different loader(s) and such code could return 
wrong pre-cached proxy class instead of throwing exception. 
Unfortunately, moving validation to slow-path was the cause of major 
performance and scalability improvement observed. With validation moved 
back to getProxyClass0 (in spite of using two-level WeakCache), we have 
the following performance (WeakCache#1):



Summary (4 Cores x 2 Threads i7 CPU):

Test Threads  ns/op Original WeakCache#1
===  ===  == ===
Proxy_getProxyClass12,403.27 2,174.51
   43,039.01 2,555.00
   85,193.58 4,273.42

Proxy_isProxyClassTrue 1   95.02 43.14
   42,266.29 43.23
   84,782.29 72.06

Proxy_isProxyClassFalse1   95.02 1.36
   42,186.59 1.36
   84,891.15 2.72

Annotation_equals  1  240.10 195.68
   41,864.06 201.41
   88,639.20 337.46

It's a little better than original getProxyClass(), but not much. The 
isProxyClass() and consequently Annotation.equals() are far better 
though. But as you might have guessed, I kind of solved that too...


My first attempt was to optimize the interface validation. Only the 
"visibility" part is necessary to be performed on fast-path. Uniqueness 
and other things can be performed on slow-path. With split-validation 
and hacks like:


private static final MethodHandle findLoadedClass0MH, 
findBootstrapClassMH;

private static final ClassLoader dummyCL = new ClassLoader() {};

static {
try {
Method method = 
ClassLoader.class.getDeclaredMethod("findLoadedClass0", String.class);

method.setAccessible(true);
findLoadedClass0MH = MethodHandles.lookup().unreflect(method);

method = 
ClassLoader.class.getDeclaredMethod("findBootstrapClass", String.class);

method.setAccessible(true);
findBootstrapClassMH = 
MethodHandles.lookup().unreflect(method);

} catch (NoSuchMethodException e) {
throw (Error) new 
NoSuchMethodError(e.getMessage()).initCause(e);

} catch (IllegalAccessException e) {
throw (Error) new 
IllegalAccessError(e.getMessage()).initCause(e);

}
}

static Class findLoadedClass(ClassLoader loader, String name) {
try {
if (VM.isSystemDomainLoader(loader)) {
return (Class) 
findBootstrapClassMH.invokeExact(dummyCL, name);

} else {
return (Class) 
findLoadedClass0MH.invokeExact(loader, name);

}
} catch (RuntimeException | Error e) {
throw e;
} catch (Throwable t) {
throw new UndeclaredThrowableException(t);
}
}


... using findLoadedClass(loader, intf.getName()) and only doing 
Class.forName(intf.getName(), false, loader) if the former returned null 
... I managed to reclaim some performance (WeakCache#1+):



Test Threads  ns/op Original  WeakCache#1 WeakCache#1+
===  ===  == ===  
Proxy_getProxyClass12,403.27 2,174.51  1,589.36
   43,039.01 2,555.00  1,929.11
   85,193.58 4,273.42  3,409.77


...but that was still not very satisfactory.

I modified the KeyFactory to create keys that weakly-reference interface 
Class objects and implement hashCode/equals in terms of comparing those 
Class objects. With such keys, no false aliasing can occur and the whole 
validation can be pushed back to slow-path. I tried hard to create keys 
with as little space overhead as possible:


http://dl.dropboxusercontent.com/u/101777488/jdk8-tl/prox

hg: jdk8/tl/jaxp: 8005954: JAXP Plugability Layer should use java.util.ServiceLoader

2013-04-19 Thread daniel . fuchs

Changeset: fad6560cb32a
Author:dfuchs
Date:  2013-04-17 15:23 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jaxp/rev/fad6560cb32a

8005954: JAXP Plugability Layer should use java.util.ServiceLoader
Summary: This fix replaces manual processing of files under META-INF/services 
in JAXP factories by calls to java.util.ServiceLoader.
Reviewed-by: alanb, joehw, mchung

! src/javax/xml/datatype/DatatypeFactory.java
! src/javax/xml/datatype/FactoryFinder.java
! src/javax/xml/parsers/DocumentBuilderFactory.java
! src/javax/xml/parsers/FactoryFinder.java
! src/javax/xml/parsers/SAXParserFactory.java
! src/javax/xml/stream/FactoryFinder.java
! src/javax/xml/stream/XMLEventFactory.java
! src/javax/xml/stream/XMLInputFactory.java
! src/javax/xml/stream/XMLOutputFactory.java
! src/javax/xml/transform/FactoryFinder.java
! src/javax/xml/transform/TransformerFactory.java
! src/javax/xml/validation/SchemaFactory.java
+ src/javax/xml/validation/SchemaFactoryConfigurationError.java
! src/javax/xml/validation/SchemaFactoryFinder.java
! src/javax/xml/xpath/XPathFactory.java
! src/javax/xml/xpath/XPathFactoryFinder.java

Re: RFR: 8001647: In-place methods on Collection/List

On Apr 19, 2013, at 2:42 PM, David Holmes  wrote:

> On 19/04/2013 10:14 PM, Paul Sandoz wrote:
>> 
>> On Apr 19, 2013, at 1:15 PM, Alan Bateman  wrote:
>> 
>>> On 18/04/2013 19:49, Akhil Arora wrote:
 Looks like the stars are aligning on getting on this into TL... the
 refreshed webrev is -

 http://cr.openjdk.java.net/~akhil/8001647.8/webrev/

>>> A minor comment on Collection.removeIf is "that satisifies the given 
>>> predicate" might be better than "which matches the provided predicate". 
>>> Also for completeness, you could say "RuntimeExceptions and Errors thrown 
>>> by the predicate are propagated ...".
>>> 
>>> In List.replaceAll then @throws NullPointerException is listed twice, which 
>>> is okay, but might be better to combine them. A typo in the second NPE 
>>> description "if the an element".
>>> 
>>> In the implementation then the only thing that puzzled me is checking the 
>>> modification count in legacy Vector, that seems unnecessary.
>>> 
>> 
>> The function value could structurally modify the Vector instance.
> 
> So this came through while I was writing my similar comments ...
> 
> My reaction to this is simply "well don't do that".

FWIW in the past this functionality has helped me detect bugs, so yes i agree 
"don't do it" but sometimes it unintentionally happens and it can be harder to 
track down the source of the problem without a CME.

> If the function/predicate/comparator is mutating the Vector then the user 
> gets what they deserve in my opinion. Trying to account for that is somewhat 
> futile. As per my other email the loop check for modCount==expectedModCount 
> will get hoisted from the loop. Further in removeIf you need to be a lot more 
> defensive during the first iteration as you haven't kept a reference to the 
> original size and array. That aside the second part of removeIf (the actual 
> removal) doesn't invoke any external code so no concurrent modification is 
> possible then.
> 
> This seems like overkill to me.
> 

I think the mod count checking should be explicitly hoisted outside the loops 
and performed at the end of the loop/method (see the 
spliterator.forEachReamining implementations) since a CME should only be used 
to detect bugs. Doug's comments on ArrayList's spliterator [*] are relevant.

Probably the most important methods for such checking are forEach, to be 
consistent with the default method implementation, 
Iterator/Spliterator.forEachRemaining, and Stream.forEach. 

If we can do such checking easily and efficiently for the other methods, which 
seems possible, then i think it worth doing and is consistent with the default 
methods, since they use iterator (List.sort defers to Collections.sort uses the 
iterator to update the list).

Paul.

[*]
/*
 * If ArrayLists were immutable, or structurally immutable (no
 * adds, removes, etc), we could implement their spliterators
 * with Arrays.spliterator. Instead we detect as much
 * interference during traversal as practical without
 * sacrificing much performance. We rely primarily on
 * modCounts. These are not guaranteed to detect concurrency
 * violations, and are sometimes overly conservative about
 * within-thread interference, but detect enough problems to
 * be worthwhile in practice. To carry this out, we (1) lazily
 * initialize fence and expectedModCount until the latest
 * point that we need to commit to the state we are checking
 * against; thus improving precision.  (This doesn't apply to
 * SubLists, that create spliterators with current non-lazy
 * values).  (2) We perform only a single
 * ConcurrentModificationException check at the end of forEach
 * (the most performance-sensitive method). When using forEach
 * (as opposed to iterators), we can normally only detect
 * interference after actions, not before. Further
 * CME-triggering checks apply to all other possible
 * violations of assumptions for example null or too-small
 * elementData array given its size(), that could only have
 * occurred due to interference.  This allows the inner loop
 * of forEach to run without any further checks, and
 * simplifies lambda-resolution. While this does entail a
 * number of checks, note that in the common case of
 * list.stream().forEach(a), no checks or other computation
 * occur anywhere other than inside forEach itself.  The other
 * less-often-used methods cannot take advantage of most of
 * these streamlinings.
 */

Re: RFR : 8008632 : Additional JavaDoc tags @apiNote, @implSpec and @implNote for JDK API Docs

2013-04-19 Thread Mike Duigou


On Apr 19 2013, at 03:40 , Alan Bateman wrote:

> On 18/04/2013 16:12, Mike Duigou wrote:
>> :
>> 
>>> @implSpec is very welcome, more reasons now with the addition of default 
>>> methods.
>>> 
>>> Clearly distinguishing normative from non-normative text is also been a 
>>> long standing problem so having a tag to identify the non-normative text is 
>>> very useful. I had to look at a few examples in the lambda/jdk repo to 
>>> convince myself that both @apiNote and @implNote are needed. One thing that 
>>> isn't clear to me is whether the "official" Java SE documentation should 
>>> include the text tagged @implNote or not.
>> Once we got going we decided to fill out the matrix. There's no obligation 
>> to use all of the tags.
>> 
> Where @implSpec is used, then do you have a view on whether this should be 
> filtered out of the Java SE specs?

Not as currently used, no. The @implSpec is being used to describe required 
behaviour of the implementation, usually defaults. ie. Conforming defaults 
implementations must throw ISE (or UOE, etc.).  

Mike

Re: RFR : 8008632 : Additional JavaDoc tags @apiNote, @implSpec and @implNote for JDK API Docs


On 19/04/2013 16:13, Mike Duigou wrote:

:
Not as currently used, no. The @implSpec is being used to describe required 
behaviour of the implementation, usually defaults. ie. Conforming defaults 
implementations must throw ISE (or UOE, etc.).

Sorry, I meant @implNote where it may not always be desirable to have 
that show up in the generated javadoc.


-Alan.

Re: RFR : 8008632 : Additional JavaDoc tags @apiNote, @implSpec and @implNote for JDK API Docs

2013-04-19 Thread Mike Duigou

On Apr 19 2013, at 08:15 , Alan Bateman wrote:

> On 19/04/2013 16:13, Mike Duigou wrote:
>> :
>> Not as currently used, no. The @implSpec is being used to describe required 
>> behaviour of the implementation, usually defaults. ie. Conforming defaults 
>> implementations must throw ISE (or UOE, etc.).
>> 
> Sorry, I meant @implNote where it may not always be desirable to have that 
> show up in the generated javadoc.

Unsure. I haven't looked at how the @implNote tag has been used. According to 
the definition it certainly could be. Both the @apiNote and @implNote are 
non-normative and could technically be removed from the spec.

I have a task today of generate samples with the @implSpec and @implNote tags 
moved to the end of the docs rather than before @param. That thread would be a 
good place to have the discussion about inclusion (along with placement).

Mike

> 
> -Alan.

Re: Proxy.isProxyClass scalability


On 04/19/2013 04:36 PM, Peter Levart wrote:


http://dl.dropboxusercontent.com/u/101777488/jdk8-tl/proxy-wc-wi/webrev.01/index.html 





Hi Mandy,

I changed the copyright header of WeakCache to GPLv2 with ClassPath 
exception and corrected a minor formatting inconsistency:


http://dl.dropboxusercontent.com/u/101777488/jdk8-tl/proxy-wc-wi/webrev.02/index.html

Regards, Peter

Re: Proxy.isProxyClass scalability


Hi Mandy,

I just noticed the following (have been thinking of it already, but then 
forgot) in original code:


/*
 512  * Note that we need not worry about reaping the cache for
 513  * entries with cleared weak references because if a proxy 
class
 514  * has been garbage collected, its class loader will have been
 515  * garbage collected as well, so the entire cache will be 
reaped
 516  * from the loaderToCache map.
 517  */


Each ClassLoader maintains explicit hard-references to all Class objects 
for classes defined by the loader. So proxy Class object can not be 
GC-ed until the ClassLoader is GC-ed. So we need not register the 
CacheValue objects in WeakCache with a refQueue. The expunging of 
reverseMap entries is already performed with CacheKey when it is cleared 
and equeued. There's no harm as it is, since the clean-up is performed 
with all the checks and is idempotent, but it need not be done for 
ClassValue objects holding weak references to proxy Class objects.


I'll do that change to WeakCache in next webrev together with any other 
possible changes that might be needed.


Regards, Peter


On 04/19/2013 05:43 PM, Peter Levart wrote:

On 04/19/2013 04:36 PM, Peter Levart wrote:


http://dl.dropboxusercontent.com/u/101777488/jdk8-tl/proxy-wc-wi/webrev.01/index.html 





Hi Mandy,

I changed the copyright header of WeakCache to GPLv2 with ClassPath 
exception and corrected a minor formatting inconsistency:


http://dl.dropboxusercontent.com/u/101777488/jdk8-tl/proxy-wc-wi/webrev.02/index.html 



Regards, Peter

RFR(XS) : JDK-8008687 - MethodHandle code: allow static and invokespecial calls to interface methods

2013-04-19 Thread Bharadwaj Yadavalli

I would like to request for a review of code changes made to support 
lambda related modifications to allow static and invokespecial calls to 
interface methods per spec version 0.6.2 
(http://cr.openjdk.java.net/~dlsmith/jsr335-0.6.2.html). These changes 
support an upcoming change that compiles lambdas as private static 
methods in conjunction with hotspot changes pushed to address JDK-8006267.


JBS: https://jbs.oracle.com/bugs/browse/JDK-8008687
Webrev : http://cr.openjdk.java.net/~bharadwaj/8008687/webrev/

Thanks,

Bharadwaj

Re: RFR: String.join(), StringJoiner additions

2013-04-19 Thread Jim Gish


Hi Henry,  Can you please comment on the simplifications you did ?

Thanks,
Jim

On 04/18/2013 07:38 PM, Ulf Zibis wrote:

Am 18.04.2013 19:37, schrieb Jim Gish:


On 04/18/2013 08:49 AM, Ulf Zibis wrote:

Hi,

I'm wondering, that StringJoiner has some logic for pre/suffix, but 
nothing to loop the elements themselves :-(


To me, StringJoiner is a useless complicated box around 
StringBuilder, and imagine, someone needs thread-safety.
It also slows down performance, as it needs additional instances and 
additional class to be loaded (critical at VM startup).


Instead please add to StringBuilder and StringBuffer:
 append(CharSequence... elements);
 append(char delimiter, CharSequence... elements);
append(char delimiter, Iterable elements);
cut(int len);// removes len chars at the end of the sequence
optional:
append(CharSequence delimiter, CharSequence... elements);
append(CharSequence delimiter, Iterable 
elements);
I started off with something similar, but it was stripped out when 
Henry did the performance improvements.


Hm, I have no idea, how above suggestions should prevent performance 
improvements. Can you help me?


Given that most people feel that this is going to be put to 
heavy-weight usage, it doesn't seem to merit too much emphasis on 
performance or complicating the implementation at this point.


Your implementation has
1 class with 7 methods
2 additional methods in String
To cover the same functionality, above approach basically only needs 2 
additional methods in StringBuilder, has better performance, so what 
is complicated on that?


@Martin: What is your opinion?

Thanks,

-Ulf


For performance reasons, append should always append the trailing 
delimeter, which could be cut at the end.


It's questionable, if class string needs a static (=no relation to 
an existing string in contrast to non-static split()) join method, 
as it seduces to

"[" + String.join(...) + "]"
which needs some effort from javac side to optimize to a single 
StringBuilder task.
IMO we better had StringBuilder.join(...), so javac could easily 
optimize to:
new StringBuilder().append('[').append(',', 
someStrings).cut(1).append(']').toString()


-Ulf


Am 18.04.2013 00:07, schrieb Martin Buchholz:
I'm still wondering about whether a joiner utility should support a 
prefix

and suffix.  The obvious uses for this are collection class toString
methods, but we already know that we can and should implement those 
with a
single precise char[] construction, so should not use StringJoiner, 
or at

least not this StringJoiner implementation.  And if we're just talking
about pure convenience, it's hard to beat

"[" + String.join(...) + "]"


On Wed, Apr 17, 2013 at 2:49 PM, Jim Gish  wrote:


Here's an update: http://cr.openjdk.java.net/~**
jgish/Bugs-5015163-7172553/< 

http://cr.openjdk.java.net/%**7Ejgish/Bugs-5015163-7172553/ 


Jim




--
Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
Oracle Java Platform Group | Core Libraries Team
35 Network Drive
Burlington, MA 01803
jim.g...@oracle.com

Re: RFR: String.join(), StringJoiner additions

2013-04-19 Thread Henry Jen

Sorry I am not receiving emails from core-lib-dev earlier.

What I did was to remove things we considered not absolutely required, which 
left us only a StringJoiner class and two String.join methods.

StringJoiner was intended to be used with Stream as a collector, thus prefix, 
and suffix is desired like Collection.toString().

Cheers,
Henry


On Apr 19, 2013, at 9:54 AM, Jim Gish  wrote:

> Hi Henry,  Can you please comment on the simplifications you did ?
> 
> Thanks,
>Jim
> 
> On 04/18/2013 07:38 PM, Ulf Zibis wrote:
>> Am 18.04.2013 19:37, schrieb Jim Gish:
>>> 
>>> On 04/18/2013 08:49 AM, Ulf Zibis wrote:
 Hi,
 
 I'm wondering, that StringJoiner has some logic for pre/suffix, but 
 nothing to loop the elements themselves :-(
 
 To me, StringJoiner is a useless complicated box around StringBuilder, and 
 imagine, someone needs thread-safety.
 It also slows down performance, as it needs additional instances and 
 additional class to be loaded (critical at VM startup).
 
 Instead please add to StringBuilder and StringBuffer:
 append(CharSequence... elements);
 append(char delimiter, CharSequence... elements);
 append(char delimiter, Iterable elements);
 cut(int len);// removes len chars at the end of the sequence
 optional:
 append(CharSequence delimiter, CharSequence... elements);
 append(CharSequence delimiter, Iterable elements);
>>> I started off with something similar, but it was stripped out when Henry 
>>> did the performance improvements.
>> 
>> Hm, I have no idea, how above suggestions should prevent performance 
>> improvements. Can you help me?
>> 
>>> Given that most people feel that this is going to be put to heavy-weight 
>>> usage, it doesn't seem to merit too much emphasis on performance or 
>>> complicating the implementation at this point.
>> 
>> Your implementation has
>> 1 class with 7 methods
>> 2 additional methods in String
>> To cover the same functionality, above approach basically only needs 2 
>> additional methods in StringBuilder, has better performance, so what is 
>> complicated on that?
>> 
>> @Martin: What is your opinion?
>> 
>> Thanks,
>> 
>> -Ulf
>> 
>> 
 For performance reasons, append should always append the trailing 
 delimeter, which could be cut at the end.
 
 It's questionable, if class string needs a static (=no relation to an 
 existing string in contrast to non-static split()) join method, as it 
 seduces to
"[" + String.join(...) + "]"
 which needs some effort from javac side to optimize to a single 
 StringBuilder task.
 IMO we better had StringBuilder.join(...), so javac could easily optimize 
 to:
new StringBuilder().append('[').append(',', 
 someStrings).cut(1).append(']').toString()
 
 -Ulf
 
 
 Am 18.04.2013 00:07, schrieb Martin Buchholz:
> I'm still wondering about whether a joiner utility should support a prefix
> and suffix.  The obvious uses for this are collection class toString
> methods, but we already know that we can and should implement those with a
> single precise char[] construction, so should not use StringJoiner, or at
> least not this StringJoiner implementation.  And if we're just talking
> about pure convenience, it's hard to beat
> 
> "[" + String.join(...) + "]"
> 
> 
> On Wed, Apr 17, 2013 at 2:49 PM, Jim Gish  wrote:
> 
>> Here's an update: http://cr.openjdk.java.net/~**
>> jgish/Bugs-5015163-7172553/<
>>  
>> http://cr.openjdk.java.net/%**7Ejgish/Bugs-5015163-7172553/
>>  
>> Jim
>> 
> 
> -- 
> Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
> Oracle Java Platform Group | Core Libraries Team
> 35 Network Drive
> Burlington, MA 01803
> jim.g...@oracle.com
>

review request for 8010416: Provide a way for DriverManager.deregisterDriver to notify the JDBC driver that it has been deregistered.

2013-04-19 Thread Lance Andersen - Oracle

Hi,

We have been asked  by a few JDBC driver vendors  to allow a JDBC driver to be 
notified when/if it was deregistered via DriverManager.deregisterDriver if 
desired.


The webrev can be found at http://cr.openjdk.java.net/~lancea/8010416/webrev.01



Best
Lance



Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com

Re: [OpenJDK 2D-Dev] sun.java2D.Pisces renderer Performance and Memory enhancements

2013-04-19 Thread Richard Bair

> 
> Also, this is with the Java version, right?  
> 
> Yes, my patch is pure java given as webrev:
> http://jmmc.fr/~bourgesl/share/java2d-pisces/webrev-1/
>  
> We got a decent 2x speedup in FX by porting the version of Open Pisces that 
> you started with to C code (all except on Linux where we couldn't find the 
> right gcc options to make it faster than Hotspot).  So, we have yet to 
> investigate a native version in the JDK which might provide even more gains…

Oleg did more analysis on this and it appears the reason hotspot on Linux was 
faster than the C version was because on Linux it is -server compiler (c2) 
whereas on Windows / Mac it is client compiler (c1). Possibly using -server on 
windows / mac would also have hotspot beating the C version, although that 
hasn't been tested.

Richard

Re: RFR : 8010953: Add primitive summary statistics utils

2013-04-19 Thread Tim Peierls

On Mon, Apr 15, 2013 at 7:30 PM, Mike Duigou  wrote:

> Another integration review in the JSR-335 libraries series. These three
> classes provide a utility for conveniently finding count, sum, min,  max
> and average of ints, longs or doubles. They can be used with existing code
> but will most likely be used with the Collectors utilities or directly with
> primitive or boxed streams.
>
> http://cr.openjdk.java.net/~mduigou/JDK-8010953/1/webrev/


Why the extra toString() calls on the result of String.format(...)?

--tim

Re: [OpenJDK 2D-Dev] sun.java2D.Pisces renderer Performance and Memory enhancements

2013-04-19 Thread Andrea Aime

On Tue, Apr 9, 2013 at 3:02 PM, Laurent Bourgès
wrote:

> Dear Java2D members,
>
> Could someone review the following webrev concerning Java2D Pisces to
> enhance its performance and reduce its memory footprint (RendererContext
> stored in thread local or concurrent queue):
> http://jmmc.fr/~bourgesl/share/java2d-pisces/webrev-1/
>
> FYI I fixed file headers in this patch and signed my OCA 3 weeks ago.
>
> Remaining work:
> - cleanup (comments ...)
> - statistics to perform auto-tuning
> - cache / memory cleanup (SoftReference ?): use hints or System properties
> to adapt it to use cases
> - another problem: fix clipping performance in Dasher / Stroker for
> segments out of bounds
>
> Could somebody support me ? ie help me working on these tasks or just to
> discuss on Pisces algorithm / implementation ?
>

Hi,
I would like to express my support for this patch.
Given that micro-benchmarks have already been run, I took the patch for a
spin in a large, real world benchmark instead,
the OSGeo WMS Shootout 2010 benchmark, for which you can see the results
here:
http://www.slideshare.net/gatewaygeomatics.com/wms-performance-shootout-2010

The presentation is long, but suffice it to say all Java based
implementations took quite the beating due to the
poor scalability of Ductus with antialiased rendering of vector data (for
an executive summary just look
at slide 27 and slide 66, where GeoServer, Oracle MapViewer and
Constellation SDI were the
Java based ones)

I took the same tests and run them again on my machine (different hardware
than the tests, don't try to compare
the absolute values), using Oracle JDK 1.7.0_17, OpenJDK 8 (a checkout a
couple of weeks old) and the
same, but with Laurent's patches applied.
Here are the results, throughput (in maps generated per second) with the
load generator (JMeter) going
up from one client to 64 concurrent clients:

   *Threads* *JDK 1.7.0_17* *OpenJDK 8, vanilla* *OpenJDK 8 + pisces
renderer improvements* *Pisces renderer performance gain, %*  1 13,97 12,43
13,03 4,75%  2 22,08 20,60 20,77 0,81%  4 34,36 33,15 33,36 0,62%  8 39,39
40,51 41,71 2,96%  16 40,61 44,57 46,98 5,39%  32 41,41 44,73 48,16 7,66%
64 37,09 42,19 45,28 7,32%

Well, first of all, congratulations to the JDK developers, don't know what
changed in JDK 8, but
GeoServer seems to like it quite a bit :-).
That said, Laurent's patch also gives a visible boost, especially when
several concurrent clients are
asking for the maps.

Mind, as I said, this is no micro-benchmark, it is a real application
loading doing a lot of I/O
(from the operating system file cache), other processing before the data
reaches the rendering
pipeline, and then has to PNG encode the output BufferedImage (PNG encoding
being rather
expensive), so getting this speedup from just a change in the rendering
pipeline is significant.

Long story short... personally I'd be very happy if this patch was going to
be integrated in Java 8 :-)

Cheers
Andrea

-- 
==
GeoServer training in Milan, 6th & 7th June 2013!  Visit
http://geoserver.geo-solutions.it for more information.
==

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39  339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

---

hg: jdk8/tl/jdk: 2 new changesets

2013-04-19 Thread yong . huang

Changeset: 414384c3b3eb
Author:yhuang
Date:  2013-04-16 22:28 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/414384c3b3eb

8011977: ISO 4217 Amendment Number 155
Reviewed-by: naoto

! src/share/classes/java/util/CurrencyData.properties
! test/java/util/Currency/tablea1.txt

Changeset: 779ba708fee3
Author:yhuang
Date:  2013-04-17 01:04 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/779ba708fee3

Merge

- src/share/native/java/lang/ResourceBundle.c

Re: RFR: String.join(), StringJoiner additions

2013-04-19 Thread Jim Gish


Martin,

I've updated the toString() method as you suggested. 
http://cr.openjdk.java.net/~jgish/Bugs-5015163-7172553/ 



Thanks,
   Jim
On 04/18/2013 05:02 PM, Martin Buchholz wrote:




On Thu, Apr 18, 2013 at 10:34 AM, Jim Gish > wrote:


That was a nice idea, but you don't want to change the value when
you do toString().  Otherwise, if you subsequently add a new
element, you're hosed because you've already added on the suffix.


You can cheaply save the current length, append the suffix, call 
toString, and reset the length back to the old value to avoid the 
overhead of String "+".


--
Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
Oracle Java Platform Group | Core Libraries Team
35 Network Drive
Burlington, MA 01803
jim.g...@oracle.com

hg: jdk8/tl/jaxp: 8010495: Update JAXP NetBeans project - add support for generating javadoc

2013-04-19 Thread daniel . fuchs

Changeset: 1c2079d11a79
Author:dfuchs
Date:  2013-04-19 17:22 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jaxp/rev/1c2079d11a79

8010495: Update JAXP NetBeans project - add support for generating javadoc
Summary: Make it possible to use NetBeans to edit the jaxp sources and to 
generate a preview of the associated javadoc.
Reviewed-by: joehw, alanb

! build.xml
! nbproject/project.xml

Re: review request: 8011620 adding free form netbeans project for jdbc to jdk/make/netbeans

2013-04-19 Thread Lance Andersen - Oracle

still looking for a committer to bless this webrev so I can put it back 

thank you in advance

best
Lance
On Apr 9, 2013, at 11:58 AM, Lance Andersen - Oracle wrote:

> Thank you ulf,  I made the change in my workspace so that it will be 
> accommodated as part of the putback
> 
> Best
> Lance
> On Apr 9, 2013, at 11:26 AM, Ulf Zibis wrote:
> 
>> Hi,
>> 
>> there is a little indentation error in build.xml in line 42.
>> 
>> -Ulf
>> 
>> Am 09.04.2013 16:55, schrieb Lance Andersen - Oracle:
>>> Hi,
>>> 
>>> This is a request to review adding a netbeans freeform project to 
>>> jdk/make/netbeans for jdbc
>>> 
>>> As part of this change,  I also modified common/shared.xml to properly look 
>>> for the jtreg report.html in the html directory and so the javadoc was 
>>> using version 1.8
>>> 
>>> The web rev is here http://cr.openjdk.java.net/~lancea/8011620/webrev.00/
>>> 
>>> Best
>>> lance
>>> 
>>> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>> Oracle Java Engineering
>>> 1 Network Drive
>>> Burlington, MA 01803
>>> lance.ander...@oracle.com
>>> 
>>> 
>> 
> 
> 
> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering 
> 1 Network Drive 
> Burlington, MA 01803
> lance.ander...@oracle.com
> 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com

hg: jdk8/tl/langtools: 8012661: remove langtools Makefile-classic

2013-04-19 Thread jonathan . gibbons

Changeset: d59730bd3162
Author:jjg
Date:  2013-04-19 11:10 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/d59730bd3162

8012661: remove langtools Makefile-classic
Reviewed-by: erikj, tbell

- make/Makefile-classic

hg: jdk8/tl/jdk: 8008670: Initial java.util.stream putback -- internal API classes

2013-04-19 Thread mike . duigou

Changeset: 6139f8fb0137
Author:mduigou
Date:  2013-04-16 22:50 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6139f8fb0137

8008670: Initial java.util.stream putback -- internal API classes
Reviewed-by: mduigou, dholmes
Contributed-by: Brian Goetz , Doug Lea 
, Paul Sandoz 

+ src/share/classes/java/util/stream/AbstractShortCircuitTask.java
+ src/share/classes/java/util/stream/AbstractTask.java
+ src/share/classes/java/util/stream/FindOps.java
+ src/share/classes/java/util/stream/ForEachOps.java
+ src/share/classes/java/util/stream/MatchOps.java
+ src/share/classes/java/util/stream/Node.java
+ src/share/classes/java/util/stream/PipelineHelper.java
+ src/share/classes/java/util/stream/Sink.java
+ src/share/classes/java/util/stream/StreamOpFlag.java
+ src/share/classes/java/util/stream/StreamShape.java
+ src/share/classes/java/util/stream/TerminalOp.java
+ src/share/classes/java/util/stream/TerminalSink.java
+ src/share/classes/java/util/stream/Tripwire.java

Re: RFR: 8010939 Deadlock in LogManager

2013-04-19 Thread Jim Gish


Hi Mandy,

I've incorporated your changes, run JPRT, and posted an updated webrev: 
http://cr.openjdk.java.net/~jgish/Bug8010939-LogManager-Deadlock.1/ 



Thanks,
Jim

On 04/12/2013 06:51 PM, Mandy Chung wrote:

Hi Jim,

Thanks for fixing this.

On 4/12/2013 1:11 PM, Jim Gish wrote:
Please review: 
http://cr.openjdk.java.net/~jgish/Bug8010939-LogManager-Deadlock/




The fix looks okay.  I would recommend to move the call to 
manager.drainLoggerRefQueueBounded() back to LogManager.addLogger 
which was originally intended and make the two separate synchronized 
methods called at the entry point as it's described in the comments of 
the drainLoggerRefQueueBounded.


You added a comment listing the bug ID.   Our convention does not 
reference the bug numbers in the source unless it's absolute critical 
information to capture to help cross-referencing.  I wouldn't want to 
see the bug numbers in the source that are fixed in the past 18 years :)


Good that you have a test to reproduce the deadlock.   A few comments:
The thread should be daemon threads so that the test will terminate 
right away if there is a deadlock.
L99: is this needed to reproduce the deadlock?  Logger.getLogger has 
already called LogManager.addLogger to register the logger.

L111: what exception is expected to be thrown and ignored by the test?
L137,144 you can call ThreadMXBean.isSynchronizerUsageSupported() to 
test if monitoring of ownable synchronizer is supported instead of 
catch UOE.

L154: you may just throw an exception and terminate the test.

Better to rename your test to some informative name that will give 
some idea what it does.  It should have @summary to give a summary of 
what it tests.  There are several deadlock tests in 
test/java/util/logging.  I like your framework and probably good to 
consolidate these deadlock tests but this is something for the future 
clean up - just to mention it.  The LoggingDeadlocks*.java tests have 
the comments how the deadlock occurs that I find useful. It'll be good 
to add similar information in your new test.  Some formatting nit: L97 
a space after "i=0;" and an extra space is many places (e.g. L69-71, 
L128, 141, 156, etc: a space not needed in front of the first 
parameter after "(",   a space of the last parameter before ")" not 
needed - L127, 134, 152, etc.


thanks
Mandy


--
Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
Oracle Java Platform Group | Core Libraries Team
35 Network Drive
Burlington, MA 01803
jim.g...@oracle.com

RFR: 8005051: optimized defaults for Iterator.forEachRemaining

Please review the addition of optimized defaults for Iterator's 
forEachRemaining to ArrayList, LinkedList, Vector and 
CopyOnWriteArrayList. The unit test has a performance comparison test 
(disabled by default) that measures the difference between this method 
and hasNext()/next(). Significant improvements were measured by 
overriding the default forEachRemaining by these classes (others, not so 
much).


http://cr.openjdk.java.net/~akhil/8005051.1/webrev/

Thanks

Re: RFR: 8001647: In-place methods on Collection/List


Updated with feedback so far -

http://cr.openjdk.java.net/~akhil/8001647.9/webrev/

On 04/18/2013 11:49 AM, Akhil Arora wrote:

Looks like the stars are aligning on getting on this into TL... the
refreshed webrev is -

http://cr.openjdk.java.net/~akhil/8001647.8/webrev/

Please review

Thanks

On 12/10/2012 09:31 PM, Akhil Arora wrote:

http://cr.openjdk.java.net/~akhil/8001647.3/webrev/

- now with synchronized and unmodifiable wrappers in Collections.java
for the default methods being added

On 12/10/2012 01:48 PM, Akhil Arora wrote:

Updated with yours and Alan's comments -

http://cr.openjdk.java.net/~akhil/8001647.2/webrev/

- removed null check for removeSet
- cache this.size in removeAll, replaceAll
  (for ArrayList, Vector and CopyOnWriteArrayList)
- calculate removeCount instead of BitCount.cardinality()
- removed unnecessary @library from test support classes

Re: RFR: 8001647: In-place methods on Collection/List

On 04/18/2013 12:36 PM, Mike Duigou wrote:

Hi Akhil;

List.sort::

- @since tag is in a strange location.

- The (optional) on IAE is in a strange position and not linked like the others.

fixed both

AbstractList::

- Should we consider adding overrides for default methods here even if our
impls wouldn't use them? We could at least add modCount checking.

I tried, but could not do anything here more than what the default is
doing, so left it alone

Tests::

- Lots of enhancement here since my last review. Good Job!

- Wrong GPL license. Tests don't get Classpath exemption.

fixed

- In Map.Defaults (around line 539 in
http://cr.openjdk.java.net/~mduigou/JDK-8010122/6/webrev/test/java/util/Map/Defaults.java.html)
I found it useful to generate an implementation of the base interface to
directly test the default methods implementations. You may want to add
something similar to CollectionExtensionMethodsTest.

the default methods are exercised (and tested) by the classes that do
not provide optimized defaults, so i think we do have coverage for the
pure default methods

- You may want to include a Collections.newSetFromMap in DataProvider.

done

- I am now preferring the Iterator return from DataProvider though I haven't
made an on-demand provider yet. You could also mark your DataProvider as ", parallel =
true"

added parallel

On Apr 18 2013, at 11:49 , Akhil Arora wrote:

Looks like the stars are aligning on getting on this into TL... the refreshed
webrev is -

http://cr.openjdk.java.net/~akhil/8001647.8/webrev/

Please review

Thanks

On 12/10/2012 09:31 PM, Akhil Arora wrote:

http://cr.openjdk.java.net/~akhil/8001647.3/webrev/

- now with synchronized and unmodifiable wrappers in Collections.java
for the default methods being added

On 12/10/2012 01:48 PM, Akhil Arora wrote:

Updated with yours and Alan's comments -

http://cr.openjdk.java.net/~akhil/8001647.2/webrev/

- removed null check for removeSet
- cache this.size in removeAll, replaceAll
(for ArrayList, Vector and CopyOnWriteArrayList)
- calculate removeCount instead of BitCount.cardinality()
- removed unnecessary @library from test support classes

Re: RFR: 8001647: In-place methods on Collection/List


On 04/19/2013 04:15 AM, Alan Bateman wrote:

On 18/04/2013 19:49, Akhil Arora wrote:

Looks like the stars are aligning on getting on this into TL... the
refreshed webrev is -

http://cr.openjdk.java.net/~akhil/8001647.8/webrev/


A minor comment on Collection.removeIf is "that satisifies the given
predicate" might be better than "which matches the provided predicate".
Also for completeness, you could say "RuntimeExceptions and Errors
thrown by the predicate are propagated ...".


fixed both


In List.replaceAll then @throws NullPointerException is listed twice,
which is okay, but might be better to combine them. A typo in the second
NPE description "if the an element".


combined and fixed NPE


In the implementation then the only thing that puzzled me is checking
the modification count in legacy Vector, that seems unnecessary.

I see Mike has looked through the tests in detail. One additional
comment is just location and trying to keep it consistent with the
current organization if possible.


moved the tests to java/util/Collection


-Alan.

Re: RFR: 8001647: In-place methods on Collection/List