date:20130610


On 10/06/2013 07:26, Dan Xu wrote:


Hi Alan,

Thank you for the clarification. I have updated the test and "if" 
format at webrev, http://cr.openjdk.java.net/~dxu/8013827/webrev.02/. 
Please review it.

Thanks for changing it, it looks good now.

-Alan

hg: jdk8/tl/jdk: 8015798: Rename IntStream.longs/doubles and LongStream.doubles to asXxxStream

2013-06-10 Thread paul . sandoz

Changeset: 7322e8ad7c01
Author:psandoz
Date:  2013-06-10 12:20 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7322e8ad7c01

8015798: Rename IntStream.longs/doubles and LongStream.doubles to asXxxStream
Reviewed-by: alanb

! src/share/classes/java/util/stream/IntPipeline.java
! src/share/classes/java/util/stream/IntStream.java
! src/share/classes/java/util/stream/LongPipeline.java
! src/share/classes/java/util/stream/LongStream.java
! test/java/util/stream/boottest/java/util/stream/SpinedBufferTest.java
! 
test/java/util/stream/test/org/openjdk/tests/java/util/stream/DoublePrimitiveOpsTests.java
! 
test/java/util/stream/test/org/openjdk/tests/java/util/stream/ExplodeOpTest.java
! 
test/java/util/stream/test/org/openjdk/tests/java/util/stream/ForEachOpTest.java
! test/java/util/stream/test/org/openjdk/tests/java/util/stream/MapOpTest.java
! test/java/util/stream/test/org/openjdk/tests/java/util/stream/MatchOpTest.java
! test/java/util/stream/test/org/openjdk/tests/java/util/stream/MinMaxTest.java
! 
test/java/util/stream/test/org/openjdk/tests/java/util/stream/PrimitiveSumTest.java
! 
test/java/util/stream/test/org/openjdk/tests/java/util/stream/StreamBuilderTest.java
! 
test/java/util/stream/test/org/openjdk/tests/java/util/stream/StreamSpliteratorTest.java

Re: Proposal: Comparables.isCommutativelyComparable

On 08.06.2013, at 23:51, Aleksey Shipilev  wrote:

> Hm, sounds interesting.
> 
> What's about yet another "factory"-like default in Comparator:
> 
> Comparator Comparator.make(Class klass)
>  - returns synthetic comparator using C.compareTo iff C implements 
> Comparable
>  - returns null otherwise

Err... "make" actually suggests we better do the zero-return Comparator if 
there is no Comparable on the class. Which fits some of the things you want 
below.

> 
> That will naively instantiate the Comparator, but there is the possibility to 
> reuse e.g. ClassValue to lazily associate the default comparator with the 
> class.
> 
> -Aleksey.
> 
> On 08.06.2013, at 19:50, Doug Lea  wrote:
> 
>> 
>> Suppose you have two objects, x and y, both of the same
>> type (x.getClass() == y.getClass()) and both known
>> to be instanceof Comparable.
>> 
>> Can you compare them? That is, can you safely call
>> (Comparable)x).compareTo(y);
>> 
>> Almost always, the answer is yes, because almost all
>> Comparable classes are of form "class C implements Comparable"
>> 
>> But there's nothing about interface Comparable that forces
>> this to be true. It would be weird but legal if both
>> are of form, say, "class C implements Comparable".
>> Which means that you cannot compare them to each other.
>> 
>> There's currently no direct way to check if you have a normal
>> commutative (bidirectionally applicable) pair of comparables.
>> 
>> It can be checked using reflection though, using mechanics
>> that are both messy and  slower than they might be if
>> implemented as a JDK utility method.
>> 
>> There a few possible API choices for how to do this.
>> 
>> The most basic is
>> 
>> boolean isCommutativelyComparable(Class c);
>> 
>> returning true if c represents a class form C implements Comparable.
>> (It would be OK although a little annoying to use if it restricted
>> argument type to Class>.)
>> 
>> A more general one is:
>> 
>> Class commutativelyComparableClassFor(Object x)
>> 
>> that returns a class C such that
>>  (x instanceof C) and (C implements Comparable)
>> or null if there is no such class. The return type could
>> be strengthened to Class>
>> 
>> This allows use of the result to match up objects of
>> subclasses of commutatively comparable classes.
>> 
>> The most handy form is:
>> 
>> int compareIfCommutativelyComparable(Comparable x, Comparable y);
>> 
>> that returns compareTo result if both of same commutatively
>> comparable type, else 0.
>> 
>> Maybe this one is *too* handy though. It's not hard to
>> get the same effect manually given the above methods.
>> 
>> Does anyone have any thoughts about introducing at
>> least the first, and possibly the others into the
>> java.util.Comparables utility class? If so, there would
>> be a few immediate usages within JDK.
>> 
>> I'm surprised that this hasn't come up for consideration
>> before. Or maybe I haven't noticed previous discussions?
>> 
>> -Doug
>> 
>> 
>>

Re: RFR 8015978: Incorrect transformation of XPath expression "string(-0)"

2013-06-10 Thread Aleksej Efimov


Hi Joe,
We can replace the "Double.isNaN(d) || Double.isInfinite(d)" with 
"!Double.isFinite(d)" - I agree that this one check looks better, but we 
still need to do the -0.0 -> 0.0 conversion to solve the reported 
problem. And as I understand (might be wrong) modification of this check 
won't help us to achieve this goal, we still need to do the conversion:


+//Convert -0.0 to +0.0 other values remains the same
+d = d + 0.0;
+

Regards,
-Aleksej

On 06/09/2013 10:23 PM, Joe Darcy wrote:

Hello Aleksej,

Looking at the code, I have another suggestion. If this code can run 
exclusively on JDK 8 or later, replace


 955 if (Double.isNaN(d) || Double.isInfinite(d))
 956 return(Double.toString(d));

with

 955 if (!Double.isFinite(d))
 956 return(Double.toString(d));

Cheers,

-Joe

On 6/9/2013 11:18 AM, Aleksej Efimov wrote:

Joe,

I definitely like it:
1. Its a one-line change - perfect size.
2. Its fastest one from discussed previously.
3. -0.0 -> 0.0 has tests.
4. And it solves our problem.

As a result of all props the next version of webrev: 
http://cr.openjdk.java.net/~coffeys/webrev.8015978.v2/ 



Thanks
-Aleksej

On 06/07/2013 11:11 PM, huizhe wang wrote:

Nice. One-line change, I guess Aleksej would love it :-)

On 6/7/2013 10:19 AM, Joe Darcy wrote:
I'll do you one better; you can turn a negative zero into a 
positive zero leaving other values unchanged like this:


d = d + 0.0;

In IEEE 754 under the round-to-nearest-even rounding mode required 
by Java

-0.0 + 0.0 => (+)0.0

This trick is used in various places in Java's numerical libraries, 
is required behavior by our specifications, and even has some tests 
for it :-)


-Joe

On 6/7/2013 8:43 AM, David Chase wrote:
Wouldn't be more efficient to do the following, assuming that the 
full Java compilation chain respects the trickiness of 0 vs -0:


if (d == 0.0) {
 d=0.0 // Jam -0 == +0 to +0, per 
http://www.w3.org/TR/xpath/#function-string


}

Division's plenty more expensive than assigning a constant, 
especially on platforms that lack hardware FP division.


David

On 2013-06-07, at 2:03 AM, huizhe wang  
wrote:



Hi Aleksej,

According to XPath spec, both positive and negative zero are 
converted to the string 0, so it seems doesn't matter. But if you 
want to detect the negative zero, you may do the following:

if (d == 0.0 && 1/d < 0.0) {
 d=0.0
}

Recognizing that (-0.0 == 0.0), and (1/(-0.0) == -Infinity).

-Joe

RFR [7181748] TEST_BUG: java/lang/ThreadGroup/Suspend test fails intermittently

2013-06-10 Thread Ivan Gerasimov


Hello everyone!

The test of ThreadGroup.Suspend() was reported to fail on rare occasions.
It can happen on a busy machine that 1 second delay would not be enough 
for the second thread to start.

Then the first thread would suspend only itself and the test would fail.
Earlier, another test was updated for similar reasons [1], [2].

The proposed test can still report false *positive* results if the 
second thread has had no chance to execute during one second after it 
had started.

To avoid them there must be a way do distinguish suspended threads.

WEBREV: http://cr.openjdk.java.net/~igerasim/7181748/webrev.0/
BUG: http://bugs.sun.com/view_bug.do?bug_id=7181748


[1] http://bugs.sun.com/view_bug.do?bug_id=7084033
[2] http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/184578f3e8b9

Sincerely yours,
Ivan Gerasimov

hg: jdk8/tl/langtools: 8013576: Add stat support to LambdaToMethod

2013-06-10 Thread maurizio . cimadamore

Changeset: 3582b62dccb2
Author:mcimadamore
Date:  2013-06-10 15:57 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/3582b62dccb2

8013576: Add stat support to LambdaToMethod
Summary: LambdaToMethod should emit info to help diagnose/test lambda 
metafactory problems
Reviewed-by: jjg, vromero

! src/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
! src/share/classes/com/sun/tools/javac/resources/compiler.properties
+ test/tools/javac/diags/examples/LambdaStat.java
+ test/tools/javac/diags/examples/MrefStat.java
+ test/tools/javac/diags/examples/MrefStat.java.rej
+ test/tools/javac/diags/examples/MrefStat1.java
+ test/tools/javac/diags/examples/MrefStat1.java.rej
+ test/tools/javac/lambda/TestLambdaToMethodStats.java

Re: Request for review: 8016046 (process) Strict validation of input should be security manager case only [win]

2013-06-10 Thread Alexey Utkin


Here is the changed version:
http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016046/webrev.01/

On 6/7/2013 8:02 PM, Alan Bateman wrote:

On 07/06/2013 10:02, Alexey Utkin wrote:

Hi,
Please review the fix.

Bug description:
http://bugs.sun.com/view_bug.do?bug_id=8016046
https://jbs.oracle.com/bugs/browse/JDK-8016046

The suggested fix:
http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016046/webrev.00/

Summary:
In absence of the Security Manager the verification procedure for
the command-line was restored as before the JDK-8012453 fix. That
suggests the ability of inline input/output redirection, piping,
simultaneous launching of several programs by single command, lost
spaces and etc.
The extended verification procedure is activated in presence of the
Security Manager or installing to "false" the
"jdk.lang.Process.allowAmbiguousCommands" Java property.

Given 15 years of sloppy usage of Runtime.exec on Windows then it was
always going to be difficult to switch to tighter checking by default.
So I think the solution is right and keeps existing code working. I
also agree that it's useful to have a property to opt-in to get
property checking of the input.

So the changes looks good to me. A minor comment on the test but in
checkOut then it could use Files.notExists. It would be useful to
expand the @bug to include the new bug ID too.

Done.

Regards,
-uta

Re: Request for review: 8016046 (process) Strict validation of input should be security manager case only [win]


On 10/06/2013 15:05, Alexey Utkin wrote:

Here is the changed version:
http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016046/webrev.01/

This looks good to me.

-Alan

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

2013-06-10 Thread Dan Xu



On 06/03/2013 03:23 AM, Alan Bateman wrote:

On 02/06/2013 03:16, Dan Xu wrote:

:

As for the SpecialTempFile testcase, I wonder whether these 
regression tests may happen to be used to test earlier JDK versions. 
In that case, the thread pool will help the test framework find the 
test failure easily. Otherwise, I agree it adds extra overhead into 
this test.
My concern is mostly the timeout as we've had so many problems with 
tests failing intermittently when the machine is very busy. In this 
case, I could imagine this failing when there are tests running 
concurrently in a pool VMs and at the same time competing with AV 
software on Windows that is sucking the life out of the system.


In general, it's not always possible to write a test that behaves well 
when run an unpatched-JDK. Deadlocks, crashes, and hangs are just some 
examples where a test might timeout or jtreg needs to spin up a new VM 
to continue the test execution. So while important for one-off 
verification then it's probably not worth spending too much effort 
trying to get it to behaves well on an JDK that doesn't have the fix.


-Alan.

Hi Alan,

Thank you for the clarification. I have updated the test and "if" format 
at webrev, http://cr.openjdk.java.net/~dxu/8013827/webrev.02/. Please 
review it.


-Dan

Re: 8016218: Warnings building corba repo due to missing hashCode methods

2013-06-10 Thread Seán Coffey

Thanks for taking care of that Alan. Those changes look fine to me. Hope 
the testing works out ok.


regards,
Sean.

On 10/06/13 12:11, Alan Bateman wrote:


The warnings from the build of the corba repository on missing 
hashCode methods have been annoying me for some time. I was hoping 
they would annoy others too but looks like I blinked first :-)


Here's the webrev with the changes that I'd like to push to fix the 
issues:


http://cr.openjdk.java.net/~alanb/8016218/webrev/

Note that there are 3 real issues but they are obvious. I'm running 
the CORBA tests to make sure that it doesn't break anything. Note that 
the bulk of the warnings were coming from ParserTable but they turn 
out not to be interesting because they are only used in the testing 
framework.


-Alan.

Re: 8016218: Warnings building corba repo due to missing hashCode methods

2013-06-10 Thread Daniel Fuchs


On 6/10/13 1:11 PM, Alan Bateman wrote:


The warnings from the build of the corba repository on missing hashCode
methods have been annoying me for some time. I was hoping they would
annoy others too but looks like I blinked first :-)

Here's the webrev with the changes that I'd like to push to fix the issues:

http://cr.openjdk.java.net/~alanb/8016218/webrev/

Note that there are 3 real issues but they are obvious. I'm running the
CORBA tests to make sure that it doesn't break anything. Note that the
bulk of the warnings were coming from ParserTable but they turn out not
to be interesting because they are only used in the testing framework.

-Alan.


Hi Alan,

Your changes look good to me.

In ParserTable.java, I wonder whether it would be worth
returning a different constant for each different class.

best regards,

-- daniel

Re: RFR 8015492: Remove DoubleStream.range methods


On 03/06/2013 16:55, Paul Sandoz wrote:

Hi,

The following patch updates DoubleStream to remove the range methods:

   
http://cr.openjdk.java.net/~psandoz/tl/JDK-8015492-rm-DoubleStream.range/webrev/

CCC is in progress.

It's best if we let numerical libraries integrate with the Stream API since the 
library writers are in a better position to design something that meets their 
use-cases.

Paul.

This looks fine to me.

-Alan.

8016251: Balanced spliterator for SpinedBuffer

2013-06-10 Thread Paul Sandoz

Hi,

This patch contains the following changes:

1) implements balanced splits for the spliterator of SpinedBuffer, which is 
contributed by Peter Levart; and

2) syncs lambda's version SpinedBuffer with tl, hence why there are some other 
changes they may not be entirely obvious until other code follows later on but 
will make that process easier e.g. the renaming of asXxxArray to 
asPrimitiveArray or the method SpinedBuffer.OfPrimitive.newArray changing from 
protected to public.

  http://cr.openjdk.java.net/~psandoz/tl/JDK-8016251-spinedBuffer-splitr/webrev/

Paul.

Re: Proposal: Comparables.isCommutativelyComparable

2013-06-10 Thread Doug Lea


On 06/08/13 21:52, Martin Buchholz wrote:

On Sat, Jun 8, 2013 at 8:50 AM, Doug Lea mailto:d...@cs.oswego.edu>> wrote:


Suppose you have two objects, x and y, both of the same
type (x.getClass() == y.getClass()) and both known
to be instanceof Comparable.

Can you compare them? That is, can you safely call
   (Comparable)x).compareTo(y);


Probably others tried and gave up, because you cannot actually get a reliable
answer - you can only get YES NO DUNNO, because some of the generic type
information you need may have been erased.


Right, including the case where the class itself was declared with
raw types. I should have stated that the method spec should say
that a true return means YES, a false return means "not validated
under best effort".

-Doug

Re: RFR (S) CR 7177472: JSR292: MethodType interning penalizes scalability

On 06/09/2013 12:17 AM, Peter Levart wrote:
> In case the loop retries, there's no need to construct another WeakEntry...
> 
> T interned;
>  WeakEntry e = new WeakEntry<>(elem, stale);
> do {
> expungeStaleElements();
> WeakEntry exist = map.putIfAbsent(e, e);
> interned = (exist == null)? elem: exist.get();
> } while (interned == null);
> return interned;

That's right, thanks!

The update is here:
  http://cr.openjdk.java.net/~shade/7177472/webrev.02/

Testing:
  - Linux x86_64 builds OK
  - Linux x86_64 java/lang/invoke/ jtreg passes OK
  - The microbenchmark scores in the original note are still the same

-Aleksey.

Re: RFR [7181748] TEST_BUG: java/lang/ThreadGroup/Suspend test fails intermittently

2013-06-10 Thread Chris Hegarty


I'm not sure I ever saw this test fail, but it does look like it has issues.

I would much prefer to see a j.u.c.Latch/Phaser used to synchronize 
across these threads.


-Chris.

On 10/06/2013 13:51, Ivan Gerasimov wrote:

Hello everyone!

The test of ThreadGroup.Suspend() was reported to fail on rare occasions.
It can happen on a busy machine that 1 second delay would not be enough
for the second thread to start.
Then the first thread would suspend only itself and the test would fail.
Earlier, another test was updated for similar reasons [1], [2].

The proposed test can still report false *positive* results if the
second thread has had no chance to execute during one second after it
had started.
To avoid them there must be a way do distinguish suspended threads.

WEBREV: http://cr.openjdk.java.net/~igerasim/7181748/webrev.0/
BUG: http://bugs.sun.com/view_bug.do?bug_id=7181748


[1] http://bugs.sun.com/view_bug.do?bug_id=7084033
[2] http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/184578f3e8b9

Sincerely yours,
Ivan Gerasimov

RFR (jaxp): 8016153 : Property http://javax.xml.XMLConstants/property/accessExternalDTD is not recognized.


Hi,

This patch is to continue the support of plugging in 3rd party parser 
impls (such as Xerces) that do not support JAXP 1.5. The fix is to catch 
SAXNotRecognizedException, print out a warning, and then continue as 
usual. The warning is for a developer in case he actually sets the new 
access* properties on the TransformerFactory but has a 3rd party parser 
on the classpath that does not support the new properties.


webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/

I intend to backport the change to 7u.  The change will be identical.

Thanks,
Joe

8016217: More javadoc warnings



About 8 months ago I tried an early build of doclint [1] and used it to 
fix up a bunch issues at the time [2]. It's been awhile, so I decided to 
try out the latest version to see how it has progressed. All I can say 
is "Yikes".  The good news is that they reported against the original 
source and that makes it easy when compared to tools that validate the 
generated html.


I decided to fix up a few issues, mostly syntax (escaping of > and < in 
particular) and a few reference issues that were missed the last time 
(or are new). There are thousands of other issues for anyone that wants 
to jump in.


I've put the webrev with the changes here:

http://cr.openjdk.java.net/~alanb/8016217/webrev/

In total this fixes ~500 issues, although 270 of those were coming from 
java.sql.DatabaseMetaData due to the number of un-escaped usages of  
"=>". In many cases, the changes are simply to use {@code ..} or replace 
 with {@code ...}. It is tempting to just do a global replace on 
existing  usages (would fixing up content that is escaped 
of course).


I've run specdiff on the before & after to check that I didn't mess 
anything up. One obvious difference is that code examples that use 
generics now have the type parameters going through to the generated 
javadoc.


The webrev touches many areas but as the changes are trivial, I don't 
need a reviewer from every area.


-Alan.

[1] http://openjdk.java.net/jeps/172
[2] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/39cbe256c3d1

Proposal: Comparables.isCommutativelyComparable

2013-06-10 Thread Paul Benedict

I like the idea except for the overly-elongated name. I think
isCommunicative would suffice.

Paul

RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement

Hi,

This is the follow-up on the issue Doug identified:
  http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017798.html

I had reworked the patch, webrev is here:
  http://cr.openjdk.java.net/~shade/8016236/webrev.01/

Notable differences from Doug's version are:
  - handle non-generic cases as well
  - reuse ReflectionData to cache the interfaces and generic signatures
  - code style (chained ternary operators blown up)
  - fixes the race along the way
  http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6398355

Testing:
  - Linux x86_64/release: build OK
  - Linux x86_64/release: java/lang/reflect regression tests OK
  - Microbenchmarks show whooping increase in performance, see below

(If there are no stylistic and other comments, I would like to do the
JPRT submit and/or additional testing [which?]).

The benchmark is here (you need JMH to build and run):
  http://cr.openjdk.java.net/~shade/8016236/classbench.zip

Before:
> Benchmark   Mode ThrCnt  Sec 
> Mean   Mean errorUnits
> o.b.ClassBench.generic_getGenericInterfaces avgt   1  51  
> 326.8351.220  nsec/op
> o.b.ClassBench.generic_getGenericSuperclass avgt   1  51  
> 308.7932.828  nsec/op
> o.b.ClassBench.generic_getTypeParametersavgt   1  51  
> 312.6271.637  nsec/op
> o.b.ClassBench.raw_getGenericInterfaces avgt   1  51  
> 216.9838.902  nsec/op
> o.b.ClassBench.raw_getGenericSuperclass avgt   1  51   
> 59.9330.183  nsec/op
> o.b.ClassBench.raw_getTypeParametersavgt   1  51   
> 65.4690.284  nsec/op

Doug's version:
> Benchmark   Mode ThrCnt  Sec 
> Mean   Mean errorUnits
> o.b.ClassBench.generic_getGenericInterfaces avgt   1  51   
> 15.1060.271  nsec/op
> o.b.ClassBench.generic_getGenericSuperclass avgt   1  51
> 5.3040.024  nsec/op
> o.b.ClassBench.generic_getTypeParametersavgt   1  51   
> 16.7390.045  nsec/op
> o.b.ClassBench.raw_getGenericInterfaces avgt   1  51  
> 213.8251.346  nsec/op
> o.b.ClassBench.raw_getGenericSuperclass avgt   1  51   
> 61.6510.394  nsec/op
> o.b.ClassBench.raw_getTypeParametersavgt   1  51   
> 64.3400.522  nsec/op

After:
> Benchmark   Mode ThrCnt  Sec 
> Mean   Mean errorUnits
> o.b.ClassBench.generic_getGenericInterfaces avgt   1  51   
> 14.9850.104  nsec/op
> o.b.ClassBench.generic_getGenericSuperclass avgt   1  51
> 5.3000.008  nsec/op
> o.b.ClassBench.generic_getTypeParametersavgt   1  51   
> 16.8740.175  nsec/op
> o.b.ClassBench.raw_getGenericInterfaces avgt   1  51   
> 23.9410.177  nsec/op
> o.b.ClassBench.raw_getGenericSuperclass avgt   1  51
> 8.5830.044  nsec/op
> o.b.ClassBench.raw_getTypeParametersavgt   1  51   
> 12.4000.045  nsec/op

Thanks,
Aleksey.

Re: Request for review: 8016046 (process) Strict validation of input should be security manager case only [win]

2013-06-10 Thread Alexey Utkin


Here is the changed version:
http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016046/webrev.01/

On 6/7/2013 8:02 PM, Alan Bateman wrote:

On 07/06/2013 10:02, Alexey Utkin wrote:

Hi,
Please review the fix.

Bug description:
http://bugs.sun.com/view_bug.do?bug_id=8016046
https://jbs.oracle.com/bugs/browse/JDK-8016046

The suggested fix:
http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016046/webrev.00/

Summary:
In absence of the Security Manager the verification procedure for 
the command-line was restored as before the JDK-8012453 fix. That 
suggests the ability of inline input/output redirection, piping, 
simultaneous launching of several programs by single command, lost 
spaces and etc.
The extended verification procedure is activated in presence of the 
Security Manager or installing to "false" the 
"jdk.lang.Process.allowAmbiguousCommands" Java property.
Given 15 years of sloppy usage of Runtime.exec on Windows then it was 
always going to be difficult to switch to tighter checking by default. 
So I think the solution is right and keeps existing code working. I 
also agree that it's useful to have a property to opt-in to get 
property checking of the input.


So the changes looks good to me. A minor comment on the test but in 
checkOut then it could use Files.notExists. It would be useful to 
expand the @bug to include the new bug ID too.

Done.

Regards,
-uta

Re: RFR 8015978: Incorrect transformation of XPath expression "string(-0)"

2013-06-10 Thread Joe Darcy


Hello Aleksej,

Looking at the code, I have another suggestion. If this code can run 
exclusively on JDK 8 or later, replace


 955 if (Double.isNaN(d) || Double.isInfinite(d))
 956 return(Double.toString(d));

with

 955 if (!Double.isFinite(d))
 956 return(Double.toString(d));

Cheers,

-Joe

On 6/9/2013 11:18 AM, Aleksej Efimov wrote:

Joe,

I definitely like it:
1. Its a one-line change - perfect size.
2. Its fastest one from discussed previously.
3. -0.0 -> 0.0 has tests.
4. And it solves our problem.

As a result of all props the next version of webrev: 
http://cr.openjdk.java.net/~coffeys/webrev.8015978.v2/ 



Thanks
-Aleksej

On 06/07/2013 11:11 PM, huizhe wang wrote:

Nice. One-line change, I guess Aleksej would love it :-)

On 6/7/2013 10:19 AM, Joe Darcy wrote:
I'll do you one better; you can turn a negative zero into a positive 
zero leaving other values unchanged like this:


d = d + 0.0;

In IEEE 754 under the round-to-nearest-even rounding mode required 
by Java

-0.0 + 0.0 => (+)0.0

This trick is used in various places in Java's numerical libraries, 
is required behavior by our specifications, and even has some tests 
for it :-)


-Joe

On 6/7/2013 8:43 AM, David Chase wrote:
Wouldn't be more efficient to do the following, assuming that the 
full Java compilation chain respects the trickiness of 0 vs -0:


if (d == 0.0) {
 d=0.0 // Jam -0 == +0 to +0, per 
http://www.w3.org/TR/xpath/#function-string


}

Division's plenty more expensive than assigning a constant, 
especially on platforms that lack hardware FP division.


David

On 2013-06-07, at 2:03 AM, huizhe wang  wrote:


Hi Aleksej,

According to XPath spec, both positive and negative zero are 
converted to the string 0, so it seems doesn't matter. But if you 
want to detect the negative zero, you may do the following:

if (d == 0.0 && 1/d < 0.0) {
 d=0.0
}

Recognizing that (-0.0 == 0.0), and (1/(-0.0) == -Infinity).

-Joe

Re: 8016218: Warnings building corba repo due to missing hashCode methods

2013-06-10 Thread Chris Hegarty


All look good to me.

These, hashCode/equals, warning have been annoying me for a while now 
too, but I have yet to blink!


-Chris.

On 10/06/2013 12:11, Alan Bateman wrote:


The warnings from the build of the corba repository on missing hashCode
methods have been annoying me for some time. I was hoping they would
annoy others too but looks like I blinked first :-)

Here's the webrev with the changes that I'd like to push to fix the issues:

http://cr.openjdk.java.net/~alanb/8016218/webrev/

Note that there are 3 real issues but they are obvious. I'm running the
CORBA tests to make sure that it doesn't break anything. Note that the
bulk of the warnings were coming from ParserTable but they turn out not
to be interesting because they are only used in the testing framework.

-Alan.

8016218: Warnings building corba repo due to missing hashCode methods



The warnings from the build of the corba repository on missing hashCode 
methods have been annoying me for some time. I was hoping they would 
annoy others too but looks like I blinked first :-)


Here's the webrev with the changes that I'd like to push to fix the issues:

http://cr.openjdk.java.net/~alanb/8016218/webrev/

Note that there are 3 real issues but they are obvious. I'm running the 
CORBA tests to make sure that it doesn't break anything. Note that the 
bulk of the warnings were coming from ParserTable but they turn out not 
to be interesting because they are only used in the testing framework.


-Alan.

Re: RFR 8015978: Incorrect transformation of XPath expression "string(-0)"

2013-06-10 Thread Aleksej Efimov


Joe,

I definitely like it:
1. Its a one-line change - perfect size.
2. Its fastest one from discussed previously.
3. -0.0 -> 0.0 has tests.
4. And it solves our problem.

As a result of all props the next version of webrev: 
http://cr.openjdk.java.net/~coffeys/webrev.8015978.v2/ 



Thanks
-Aleksej

On 06/07/2013 11:11 PM, huizhe wang wrote:

Nice. One-line change, I guess Aleksej would love it :-)

On 6/7/2013 10:19 AM, Joe Darcy wrote:
I'll do you one better; you can turn a negative zero into a positive 
zero leaving other values unchanged like this:


d = d + 0.0;

In IEEE 754 under the round-to-nearest-even rounding mode required by 
Java

-0.0 + 0.0 => (+)0.0

This trick is used in various places in Java's numerical libraries, 
is required behavior by our specifications, and even has some tests 
for it :-)


-Joe

On 6/7/2013 8:43 AM, David Chase wrote:
Wouldn't be more efficient to do the following, assuming that the 
full Java compilation chain respects the trickiness of 0 vs -0:


if (d == 0.0) {
 d=0.0 // Jam -0 == +0 to +0, per 
http://www.w3.org/TR/xpath/#function-string


}

Division's plenty more expensive than assigning a constant, 
especially on platforms that lack hardware FP division.


David

On 2013-06-07, at 2:03 AM, huizhe wang  wrote:


Hi Aleksej,

According to XPath spec, both positive and negative zero are 
converted to the string 0, so it seems doesn't matter. But if you 
want to detect the negative zero, you may do the following:

if (d == 0.0 && 1/d < 0.0) {
 d=0.0
}

Recognizing that (-0.0 == 0.0), and (1/(-0.0) == -Infinity).

-Joe

Re: 8016217: More javadoc warnings

2013-06-10 Thread Chris Hegarty


I eyeballed the patch file. Looks fine to me.

It is really nice to have a tool to that operates on the actual source.

-Chris.

On 10/06/2013 11:31, Alan Bateman wrote:


About 8 months ago I tried an early build of doclint [1] and used it to
fix up a bunch issues at the time [2]. It's been awhile, so I decided to
try out the latest version to see how it has progressed. All I can say
is "Yikes". The good news is that they reported against the original
source and that makes it easy when compared to tools that validate the
generated html.

I decided to fix up a few issues, mostly syntax (escaping of > and < in
particular) and a few reference issues that were missed the last time
(or are new). There are thousands of other issues for anyone that wants
to jump in.

I've put the webrev with the changes here:

http://cr.openjdk.java.net/~alanb/8016217/webrev/

In total this fixes ~500 issues, although 270 of those were coming from
java.sql.DatabaseMetaData due to the number of un-escaped usages of
"=>". In many cases, the changes are simply to use {@code ..} or replace
 with {@code ...}. It is tempting to just do a global replace on
existing  usages (would fixing up content that is escaped
of course).

I've run specdiff on the before & after to check that I didn't mess
anything up. One obvious difference is that code examples that use
generics now have the type parameters going through to the generated
javadoc.

The webrev touches many areas but as the changes are trivial, I don't
need a reviewer from every area.

-Alan.

[1] http://openjdk.java.net/jeps/172
[2] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/39cbe256c3d1

RFR (jaxp): 8016153 : Property http://javax.xml.XMLConstants/property/accessExternalDTD is not recognized.


Hi,

This patch is to continue the support of plugging in 3rd party parser 
impls (such as Xerces) that do not support JAXP 1.5. The fix is to catch 
SAXNotRecognizedException, print out a warning, and then continue as 
usual. The warning is for the developer in case he actually sets the new 
access* properties on the TransformerFactory but has a 3rd party parser 
on the classpath that does not support the new properties.


webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/

I intend to backport the change to 7u.  The change will be identical.

Thanks,
Joe

Re: 8016217: More javadoc warnings

2013-06-10 Thread Lance Andersen - Oracle

This looks good.  Thank you for taking this on Alan.

Best
Lance
On Jun 10, 2013, at 6:31 AM, Alan Bateman wrote:

> 
> About 8 months ago I tried an early build of doclint [1] and used it to fix 
> up a bunch issues at the time [2]. It's been awhile, so I decided to try out 
> the latest version to see how it has progressed. All I can say is "Yikes".  
> The good news is that they reported against the original source and that 
> makes it easy when compared to tools that validate the generated html.
> 
> I decided to fix up a few issues, mostly syntax (escaping of > and < in 
> particular) and a few reference issues that were missed the last time (or are 
> new). There are thousands of other issues for anyone that wants to jump in.
> 
> I've put the webrev with the changes here:
> 
> http://cr.openjdk.java.net/~alanb/8016217/webrev/
> 
> In total this fixes ~500 issues, although 270 of those were coming from 
> java.sql.DatabaseMetaData due to the number of un-escaped usages of  "=>". In 
> many cases, the changes are simply to use {@code ..} or replace  with 
> {@code ...}. It is tempting to just do a global replace on existing 
>  usages (would fixing up content that is escaped of course).
> 
> I've run specdiff on the before & after to check that I didn't mess anything 
> up. One obvious difference is that code examples that use generics now have 
> the type parameters going through to the generated javadoc.
> 
> The webrev touches many areas but as the changes are trivial, I don't need a 
> reviewer from every area.
> 
> -Alan.
> 
> [1] http://openjdk.java.net/jeps/172
> [2] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/39cbe256c3d1


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: RFR 8015978: Incorrect transformation of XPath expression "string(-0)"

2013-06-10 Thread Joe Darcy


Hi Aleksej,

On 6/10/2013 8:02 AM, Aleksej Efimov wrote:

Hi Joe,
We can replace the "Double.isNaN(d) || Double.isInfinite(d)" with 
"!Double.isFinite(d)" - I agree that this one check looks better, but 
we still need to do the -0.0 -> 0.0 conversion to solve the reported 
problem. And as I understand (might be wrong) modification of this 
check won't help us to achieve this goal, we still need to do the 
conversion:

+//Convert -0.0 to +0.0 other values remains the same
+d = d + 0.0;
+


Right; changing the set of Double.isFoo methods called earlier doesn't 
change the need for the (d + 0.0) expression. I just noticed the double 
isFoo calls when looking at the code and saw an opportunity to use the 
new method.


Cheers,

-Joe



Regards,
-Aleksej

On 06/09/2013 10:23 PM, Joe Darcy wrote:

Hello Aleksej,

Looking at the code, I have another suggestion. If this code can run 
exclusively on JDK 8 or later, replace


 955 if (Double.isNaN(d) || Double.isInfinite(d))
 956 return(Double.toString(d));

with

 955 if (!Double.isFinite(d))
 956 return(Double.toString(d));

Cheers,

-Joe

On 6/9/2013 11:18 AM, Aleksej Efimov wrote:

Joe,

I definitely like it:
1. Its a one-line change - perfect size.
2. Its fastest one from discussed previously.
3. -0.0 -> 0.0 has tests.
4. And it solves our problem.

As a result of all props the next version of webrev: 
http://cr.openjdk.java.net/~coffeys/webrev.8015978.v2/ 



Thanks
-Aleksej

On 06/07/2013 11:11 PM, huizhe wang wrote:

Nice. One-line change, I guess Aleksej would love it :-)

On 6/7/2013 10:19 AM, Joe Darcy wrote:
I'll do you one better; you can turn a negative zero into a 
positive zero leaving other values unchanged like this:


d = d + 0.0;

In IEEE 754 under the round-to-nearest-even rounding mode required 
by Java

-0.0 + 0.0 => (+)0.0

This trick is used in various places in Java's numerical 
libraries, is required behavior by our specifications, and even 
has some tests for it :-)


-Joe

On 6/7/2013 8:43 AM, David Chase wrote:
Wouldn't be more efficient to do the following, assuming that the 
full Java compilation chain respects the trickiness of 0 vs -0:


if (d == 0.0) {
 d=0.0 // Jam -0 == +0 to +0, per 
http://www.w3.org/TR/xpath/#function-string


}

Division's plenty more expensive than assigning a constant, 
especially on platforms that lack hardware FP division.


David

On 2013-06-07, at 2:03 AM, huizhe wang  
wrote:



Hi Aleksej,

According to XPath spec, both positive and negative zero are 
converted to the string 0, so it seems doesn't matter. But if 
you want to detect the negative zero, you may do the following:

if (d == 0.0 && 1/d < 0.0) {
 d=0.0
}

Recognizing that (-0.0 == 0.0), and (1/(-0.0) == -Infinity).

-Joe

Re: 8016218: Warnings building corba repo due to missing hashCode methods


On 10/06/2013 16:42, Daniel Fuchs wrote:

:

In ParserTable.java, I wonder whether it would be worth
returning a different constant for each different class.
I could although I don't think they are used (and probably could be 
deleted).


-Alan.

Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement

2013-06-10 Thread Paul Sandoz


On Jun 10, 2013, at 5:53 PM, Aleksey Shipilev  
wrote:

> Hi,
> 
> This is the follow-up on the issue Doug identified:
>  http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017798.html
> 
> I had reworked the patch, webrev is here:
>  http://cr.openjdk.java.net/~shade/8016236/webrev.01/
> 
> Notable differences from Doug's version are:
>  - handle non-generic cases as well

I was wondering about that case too.


>  - reuse ReflectionData to cache the interfaces and generic signatures

Any guess on the size impact due to those new fields in ReflectionData?


>  - code style (chained ternary operators blown up)
>  - fixes the race along the way
>  http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6398355
> 
> Testing:
>  - Linux x86_64/release: build OK
>  - Linux x86_64/release: java/lang/reflect regression tests OK
>  - Microbenchmarks show whooping increase in performance, see below
> 

Nice improvements.

Paul.


> (If there are no stylistic and other comments, I would like to do the
> JPRT submit and/or additional testing [which?]).
> 
> The benchmark is here (you need JMH to build and run):
>  http://cr.openjdk.java.net/~shade/8016236/classbench.zip
> 
> Before:
>> Benchmark   Mode ThrCnt  Sec 
>> Mean   Mean errorUnits
>> o.b.ClassBench.generic_getGenericInterfaces avgt   1  51  
>> 326.8351.220  nsec/op
>> o.b.ClassBench.generic_getGenericSuperclass avgt   1  51  
>> 308.7932.828  nsec/op
>> o.b.ClassBench.generic_getTypeParametersavgt   1  51  
>> 312.6271.637  nsec/op
>> o.b.ClassBench.raw_getGenericInterfaces avgt   1  51  
>> 216.9838.902  nsec/op
>> o.b.ClassBench.raw_getGenericSuperclass avgt   1  51   
>> 59.9330.183  nsec/op
>> o.b.ClassBench.raw_getTypeParametersavgt   1  51   
>> 65.4690.284  nsec/op
> 
> Doug's version:
>> Benchmark   Mode ThrCnt  Sec 
>> Mean   Mean errorUnits
>> o.b.ClassBench.generic_getGenericInterfaces avgt   1  51   
>> 15.1060.271  nsec/op
>> o.b.ClassBench.generic_getGenericSuperclass avgt   1  51
>> 5.3040.024  nsec/op
>> o.b.ClassBench.generic_getTypeParametersavgt   1  51   
>> 16.7390.045  nsec/op
>> o.b.ClassBench.raw_getGenericInterfaces avgt   1  51  
>> 213.8251.346  nsec/op
>> o.b.ClassBench.raw_getGenericSuperclass avgt   1  51   
>> 61.6510.394  nsec/op
>> o.b.ClassBench.raw_getTypeParametersavgt   1  51   
>> 64.3400.522  nsec/op
> 
> After:
>> Benchmark   Mode ThrCnt  Sec 
>> Mean   Mean errorUnits
>> o.b.ClassBench.generic_getGenericInterfaces avgt   1  51   
>> 14.9850.104  nsec/op
>> o.b.ClassBench.generic_getGenericSuperclass avgt   1  51
>> 5.3000.008  nsec/op
>> o.b.ClassBench.generic_getTypeParametersavgt   1  51   
>> 16.8740.175  nsec/op
>> o.b.ClassBench.raw_getGenericInterfaces avgt   1  51   
>> 23.9410.177  nsec/op
>> o.b.ClassBench.raw_getGenericSuperclass avgt   1  51
>> 8.5830.044  nsec/op
>> o.b.ClassBench.raw_getTypeParametersavgt   1  51   
>> 12.4000.045  nsec/op
> 
> Thanks,
> Aleksey.

hg: jdk8/tl/corba: 8016218: Warnings building corba repo due to missing hashCode methods

2013-06-10 Thread alan . bateman

Changeset: 5845df371e25
Author:alanb
Date:  2013-06-10 17:15 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/corba/rev/5845df371e25

8016218: Warnings building corba repo due to missing hashCode methods
Reviewed-by: chegar, coffeys, dfuchs

! src/share/classes/com/sun/corba/se/impl/javax/rmi/CORBA/StubDelegateImpl.java
! src/share/classes/com/sun/corba/se/impl/orb/ParserTable.java
! src/share/classes/com/sun/corba/se/impl/orbutil/RepIdDelegator.java
! src/share/classes/sun/rmi/rmic/iiop/CompoundType.java

Re: RFR (S) CR 7177472: JSR292: MethodType interning penalizes scalability



On 06/08/2013 10:25 PM, Peter Levart wrote:


On 06/08/2013 10:17 PM, Peter Levart wrote:


On 06/08/2013 12:21 PM, Remi Forax wrote:

On 06/07/2013 12:01 PM, Aleksey Shipilev wrote:
(posting this to hotspot-dev@ and cc-ing core-libs-dev@, as 
Christian T.

suggested offline)

Hi guys,


Hi Aleksey,



The fix for scalability problem is here:
http://cr.openjdk.java.net/~shade/7177472/webrev.00/


in add, the do/while does something a little weird, if putIfAbsent 
returns null,
interned is equals to elem, there is no need to do a e.get() in that 
case,

I would prefer a code like this:

T interned;
do {
expungeStaleElements();
WeakEntry e = new WeakEntry<>(elem, stale);
WeakEntry exist = map.putIfAbsent(e, e);
interned = (exist == null)? elem: exist.get();
} while (interned == null);
return interned;


Hi Remi, Aleksey,

In case the loop retries, there's no need to construct another 
WeakEntry...


T interned;
 WeakEntry e = new WeakEntry<>(elem, stale);
do {
expungeStaleElements();
WeakEntry exist = map.putIfAbsent(e, e);
interned = (exist == null)? elem: exist.get();
} while (interned == null);
return interned;

Regards, Peter


Aleksey,

Also, in a retry-loop you might be spin-waiting for the 
ReferenceHandler thread to transfer the cleared WeakEntry to the 
ReferenceQueue. You might want to do a map.replace(e, exist, e) in 
case exist != null and exist.get() == null to make it independent of 
ReferenceHandler thread's processing. In this case only a single 
out-of-loop call to expungeStaleElements() would be enough.


Scrap that. when exist.get() == null, exist.equals(e) == false, so 
putIfAbsent should succeed.




Peter





The cast to WeakEntry in expungeStaleElements is not needed.

In WeakEntry.equals, the null check is not needed because null 
instanceof WeakEntry returns false,
and you don't need to cast obj.get() to a WeakEntry given you 
only to call equals on entry.get().

So the code should be:


   public boolean equals(Object obj) {
 if (!(obj instanceof WeakEntry)) {
   return false;
 }
   Object that = ((WeakEntry)obj).get();
 Object mine = get();
 return (that == null || mine == null)? this == obj: 
mine.equals(that);

 }


Otherwise, it looks good.

Rémi



Testing:
   - Linux x86_64 builds OK
   - Linux x86_64 java/lang/invoke/ jtreg passes OK

Since this is a proof-of-concept patch at this point, I kept the 
testing

minimal. Let me know what other testing you think is needed before the
integration.

Thanks,
Aleksey.

*** Rationale and details for the issue follow: ***

This issue was in limbo for an year. The performance issue with
MethodType.methodType factory leads to abysmal scalability on the
targeted tests. While this can arguably demonstrated on larger
workloads, the targeted microbenchmarks are showcasing it nicely. This
issue was blocking the JSR292 API performance testing, because on 
large

enough test server, you will inevitably be bitten by this.

If you run the JMH-enabled [1] microbenchmark:
http://openjdk.java.net/projects/code-tools/jmh/
http://cr.openjdk.java.net/~shade/7177472/jsr292bench.tar.gz

...on the current jdk8/jdk8, 2x2 i5 2.0 Ghz, Linux x86_64, you will 
see
the scalability is going down. ("testCached" makes sure the 
reference to

MethodType is always strongly reachable, "testNew" makes sure the
reference is mostly non-reachable, prompting cache re-fill, "testOOB"
does not make any precautions about that, and also does not suffer 
from

the static overheads entailed by the reference management in first two
tests).

1 thread:
MethodTypeBench.testCached 78 +-  1 nsec/op
MethodTypeBench.testNew   120 +-  1 nsec/op
MethodTypeBench.testOOB41 +-  1 nsec/op

4 threads:
MethodTypeBench.testCached495 +- 40 nsec/op
MethodTypeBench.testNew   611 +- 37 nsec/op
MethodTypeBench.testOOB   377 +- 10 nsec/op

In fact, on larger machines it may be as bad as tens/hundreds of
microseconds to look up the MethodType. With the patch applied, on the
same 2x2 machine:

1 thread:
MethodTypeBench.testCached 92 +-  1 nsec/op
MethodTypeBench.testNew   101 +-  1 nsec/op
MethodTypeBench.testOOB49 +-  1 nsec/op

4 threads:
MethodTypeBench.testCached129 +-  1 nsec/op
MethodTypeBench.testNew   142 +- 10 nsec/op
MethodTypeBench.testOOB89 +-  3 nsec/op


...which means the scalability is back: the average time to look up 
the

MethodType is not growing spectacularly when the thread count go up.
Again, the effect is astonishing on large machines.

Notice that we took some hit in single-threaded performance, and 
that is

due to two reasons: a) WeakEntry allocation in get(); b) CHM. With the
upcoming CHMv8, this starts to look b

Re: RFR (jaxp): 8016153 : Property http://javax.xml.XMLConstants/property/accessExternalDTD is not recognized.

2013-06-10 Thread Lance Andersen - Oracle

looks fine joe

-lance
On Jun 8, 2013, at 4:11 PM, huizhe wang wrote:

> Hi,
> 
> This patch is to continue the support of plugging in 3rd party parser impls 
> (such as Xerces) that do not support JAXP 1.5. The fix is to catch 
> SAXNotRecognizedException, print out a warning, and then continue as usual. 
> The warning is for the developer in case he actually sets the new access* 
> properties on the TransformerFactory but has a 3rd party parser on the 
> classpath that does not support the new properties.
> 
> webrev:
> http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/
> 
> I intend to backport the change to 7u.  The change will be identical.
> 
> Thanks,
> Joe


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: Proposal: Comparables.isCommutativelyComparable

Hm, sounds interesting.

What's about yet another "factory"-like default in Comparator:

Comparator Comparator.make(Class klass)
  - returns synthetic comparator using C.compareTo iff C implements 
Comparable
  - returns null otherwise

That will naively instantiate the Comparator, but there is the possibility to 
reuse e.g. ClassValue to lazily associate the default comparator with the class.

-Aleksey.

On 08.06.2013, at 19:50, Doug Lea  wrote:

> 
> Suppose you have two objects, x and y, both of the same
> type (x.getClass() == y.getClass()) and both known
> to be instanceof Comparable.
> 
> Can you compare them? That is, can you safely call
>  (Comparable)x).compareTo(y);
> 
> Almost always, the answer is yes, because almost all
> Comparable classes are of form "class C implements Comparable"
> 
> But there's nothing about interface Comparable that forces
> this to be true. It would be weird but legal if both
> are of form, say, "class C implements Comparable".
> Which means that you cannot compare them to each other.
> 
> There's currently no direct way to check if you have a normal
> commutative (bidirectionally applicable) pair of comparables.
> 
> It can be checked using reflection though, using mechanics
> that are both messy and  slower than they might be if
> implemented as a JDK utility method.
> 
> There a few possible API choices for how to do this.
> 
> The most basic is
> 
>  boolean isCommutativelyComparable(Class c);
> 
> returning true if c represents a class form C implements Comparable.
> (It would be OK although a little annoying to use if it restricted
> argument type to Class>.)
> 
> A more general one is:
> 
>  Class commutativelyComparableClassFor(Object x)
> 
> that returns a class C such that
>   (x instanceof C) and (C implements Comparable)
> or null if there is no such class. The return type could
> be strengthened to Class>
> 
> This allows use of the result to match up objects of
> subclasses of commutatively comparable classes.
> 
> The most handy form is:
> 
>  int compareIfCommutativelyComparable(Comparable x, Comparable y);
> 
> that returns compareTo result if both of same commutatively
> comparable type, else 0.
> 
> Maybe this one is *too* handy though. It's not hard to
> get the same effect manually given the above methods.
> 
> Does anyone have any thoughts about introducing at
> least the first, and possibly the others into the
> java.util.Comparables utility class? If so, there would
> be a few immediate usages within JDK.
> 
> I'm surprised that this hasn't come up for consideration
> before. Or maybe I haven't noticed previous discussions?
> 
> -Doug
> 
> 
>

Re: RFR (S) CR 7177472: JSR292: MethodType interning penalizes scalability



On 06/08/2013 10:17 PM, Peter Levart wrote:


On 06/08/2013 12:21 PM, Remi Forax wrote:

On 06/07/2013 12:01 PM, Aleksey Shipilev wrote:
(posting this to hotspot-dev@ and cc-ing core-libs-dev@, as 
Christian T.

suggested offline)

Hi guys,


Hi Aleksey,



The fix for scalability problem is here:
http://cr.openjdk.java.net/~shade/7177472/webrev.00/


in add, the do/while does something a little weird, if putIfAbsent 
returns null,
interned is equals to elem, there is no need to do a e.get() in that 
case,

I would prefer a code like this:

T interned;
do {
expungeStaleElements();
WeakEntry e = new WeakEntry<>(elem, stale);
WeakEntry exist = map.putIfAbsent(e, e);
interned = (exist == null)? elem: exist.get();
} while (interned == null);
return interned;


Hi Remi, Aleksey,

In case the loop retries, there's no need to construct another 
WeakEntry...


T interned;
 WeakEntry e = new WeakEntry<>(elem, stale);
do {
expungeStaleElements();
WeakEntry exist = map.putIfAbsent(e, e);
interned = (exist == null)? elem: exist.get();
} while (interned == null);
return interned;

Regards, Peter


Aleksey,

Also, in a retry-loop you might be spin-waiting for the ReferenceHandler 
thread to transfer the cleared WeakEntry to the ReferenceQueue. You 
might want to do a map.replace(e, exist, e) in case exist != null and 
exist.get() == null to make it independent of ReferenceHandler thread's 
processing. In this case only a single out-of-loop call to 
expungeStaleElements() would be enough.


Peter





The cast to WeakEntry in expungeStaleElements is not needed.

In WeakEntry.equals, the null check is not needed because null 
instanceof WeakEntry returns false,
and you don't need to cast obj.get() to a WeakEntry given you only 
to call equals on entry.get().

So the code should be:


   public boolean equals(Object obj) {
 if (!(obj instanceof WeakEntry)) {
   return false;
 }
   Object that = ((WeakEntry)obj).get();
 Object mine = get();
 return (that == null || mine == null)? this == obj: 
mine.equals(that);

 }


Otherwise, it looks good.

Rémi



Testing:
   - Linux x86_64 builds OK
   - Linux x86_64 java/lang/invoke/ jtreg passes OK

Since this is a proof-of-concept patch at this point, I kept the 
testing

minimal. Let me know what other testing you think is needed before the
integration.

Thanks,
Aleksey.

*** Rationale and details for the issue follow: ***

This issue was in limbo for an year. The performance issue with
MethodType.methodType factory leads to abysmal scalability on the
targeted tests. While this can arguably demonstrated on larger
workloads, the targeted microbenchmarks are showcasing it nicely. This
issue was blocking the JSR292 API performance testing, because on large
enough test server, you will inevitably be bitten by this.

If you run the JMH-enabled [1] microbenchmark:
http://openjdk.java.net/projects/code-tools/jmh/
http://cr.openjdk.java.net/~shade/7177472/jsr292bench.tar.gz

...on the current jdk8/jdk8, 2x2 i5 2.0 Ghz, Linux x86_64, you will see
the scalability is going down. ("testCached" makes sure the 
reference to

MethodType is always strongly reachable, "testNew" makes sure the
reference is mostly non-reachable, prompting cache re-fill, "testOOB"
does not make any precautions about that, and also does not suffer from
the static overheads entailed by the reference management in first two
tests).

1 thread:
MethodTypeBench.testCached 78 +-  1 nsec/op
MethodTypeBench.testNew   120 +-  1 nsec/op
MethodTypeBench.testOOB41 +-  1 nsec/op

4 threads:
MethodTypeBench.testCached495 +- 40 nsec/op
MethodTypeBench.testNew   611 +- 37 nsec/op
MethodTypeBench.testOOB   377 +- 10 nsec/op

In fact, on larger machines it may be as bad as tens/hundreds of
microseconds to look up the MethodType. With the patch applied, on the
same 2x2 machine:

1 thread:
MethodTypeBench.testCached 92 +-  1 nsec/op
MethodTypeBench.testNew   101 +-  1 nsec/op
MethodTypeBench.testOOB49 +-  1 nsec/op

4 threads:
MethodTypeBench.testCached129 +-  1 nsec/op
MethodTypeBench.testNew   142 +- 10 nsec/op
MethodTypeBench.testOOB89 +-  3 nsec/op


...which means the scalability is back: the average time to look up the
MethodType is not growing spectacularly when the thread count go up.
Again, the effect is astonishing on large machines.

Notice that we took some hit in single-threaded performance, and 
that is

due to two reasons: a) WeakEntry allocation in get(); b) CHM. With the
upcoming CHMv8, this starts to look better:

1 thread:
MethodTypeBench.testCached 87 +-  1 nsec/op
MethodTypeBench.testNew95 +-  1 nsec/op
MethodTypeBench.testOOB52 +-  1

Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement

Thanks for taking a look, Paul!
Can I count your review as official?

On 06/10/2013 08:44 PM, Paul Sandoz wrote:
>>  - reuse ReflectionData to cache the interfaces and generic signatures
> 
> Any guess on the size impact due to those new fields in ReflectionData?

Seems to be additional 16 bytes (boolean+oop+oop+alignment, see below);
again, since the ReflectionData is lazily-constructed, this is not the
issue for most of the classes.

 Before:

Running 64-bit HotSpot VM.
Using compressed references with 3-bit shift.
Objects are 8 bytes aligned.

java.lang.Class.ReflectionData
 offset  size  type description
  012   (assumed to be the object header)
 12 4   int ReflectionData.redefinedCount
 16 4   Field[] ReflectionData.declaredFields
 20 4   Field[] ReflectionData.publicFields
 24 4  Method[] ReflectionData.declaredMethods
 28 4  Method[] ReflectionData.publicMethods
 32 4 Constructor[] ReflectionData.declaredConstructors
 36 4 Constructor[] ReflectionData.publicConstructors
 40 4   Field[] ReflectionData.declaredPublicFields
 44 4  Method[] ReflectionData.declaredPublicMethods
 48 (object boundary, size estimate)

 After:

Running 64-bit HotSpot VM.
Using compressed references with 3-bit shift.
Objects are 16 bytes aligned.

java.lang.Class.ReflectionData
 offset  size  type description
  012   (assumed to be the object header)
 12 4   int ReflectionData.redefinedCount
 16 1   boolean ReflectionData.genericSignatureResolved
 17 3   (alignment/padding gap)
 20 4   Field[] ReflectionData.declaredFields
 24 4   Field[] ReflectionData.publicFields
 28 4  Method[] ReflectionData.declaredMethods
 32 4  Method[] ReflectionData.publicMethods
 36 4 Constructor[] ReflectionData.declaredConstructors
 40 4 Constructor[] ReflectionData.publicConstructors
 44 4   Field[] ReflectionData.declaredPublicFields
 48 4  Method[] ReflectionData.declaredPublicMethods
 52 4   Class[] ReflectionData.interfaces
 56 4String ReflectionData.genericSignature
 60 4   (loss due to the next object alignment)
 64 (object boundary, size estimate)

RE: 8016217: More javadoc warnings

2013-06-10 Thread Iris Clark

Hi, Alan.

The changes look good to me.

One thing caught my eye.  In java.util.Calendar, around lines 2681 and later 
around 2702 you removed the ""
 
--- old/src/share/classes/java/util/Calendar.java   2013-06-10 
10:37:52.585879060 +0100
+++ new/src/share/classes/java/util/Calendar.java   2013-06-10 
10:37:52.297879063 +0100
@@ -2681,9 +2681,9 @@
  * Returns whether this Calendar represents a time
  * before the time represented by the specified
  * Object. This method is equivalent to:
- * 
+ * {@code
  * compareTo(when) < 0
- * 
+ * }
  * if and only if when is a Calendar
  * instance. Otherwise, the method returns false.
  *

But you left them in java.io.Console, javax.security.sasl.SasClient, etc.

--- old/src/share/classes/java/io/Console.java  2013-06-10 10:37:17.361879868 
+0100
+++ new/src/share/classes/java/io/Console.java  2013-06-10 10:37:17.093879874 
+0100
@@ -75,7 +75,7 @@
  * manually zero the returned character array after processing to minimize the
  * lifetime of sensitive data in memory.
  *
- * 
+ * {@code
  * Console cons;
  * char[] passwd;
  * if ((cons = System.console()) != null &&
@@ -83,7 +83,7 @@
  * ...
  * java.util.Arrays.fill(passwd, ' ');
  * }
- * 
+ * }
  *
  * @author  Xueming Shen
  * @since   1.6

Was the removal intentional?

Thanks,
iris

-Original Message-
From: Alan Bateman 
Sent: Monday, June 10, 2013 3:32 AM
To: core-libs-dev
Subject: 8016217: More javadoc warnings


About 8 months ago I tried an early build of doclint [1] and used it to fix up 
a bunch issues at the time [2]. It's been awhile, so I decided to try out the 
latest version to see how it has progressed. All I can say is "Yikes".  The 
good news is that they reported against the original source and that makes it 
easy when compared to tools that validate the generated html.

I decided to fix up a few issues, mostly syntax (escaping of > and < in
particular) and a few reference issues that were missed the last time (or are 
new). There are thousands of other issues for anyone that wants to jump in.

I've put the webrev with the changes here:

http://cr.openjdk.java.net/~alanb/8016217/webrev/

In total this fixes ~500 issues, although 270 of those were coming from 
java.sql.DatabaseMetaData due to the number of un-escaped usages of "=>". In 
many cases, the changes are simply to use {@code ..} or replace  with 
{@code ...}. It is tempting to just do a global replace on existing 
 usages (would fixing up content that is escaped of course).

I've run specdiff on the before & after to check that I didn't mess anything 
up. One obvious difference is that code examples that use generics now have the 
type parameters going through to the generated javadoc.

The webrev touches many areas but as the changes are trivial, I don't need a 
reviewer from every area.

-Alan.

[1] http://openjdk.java.net/jeps/172
[2] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/39cbe256c3d1

Re: RFR (S) CR 7177472: JSR292: MethodType interning penalizes scalability



On 06/08/2013 12:21 PM, Remi Forax wrote:

On 06/07/2013 12:01 PM, Aleksey Shipilev wrote:

(posting this to hotspot-dev@ and cc-ing core-libs-dev@, as Christian T.
suggested offline)

Hi guys,


Hi Aleksey,



The fix for scalability problem is here:
   http://cr.openjdk.java.net/~shade/7177472/webrev.00/


in add, the do/while does something a little weird, if putIfAbsent 
returns null,
interned is equals to elem, there is no need to do a e.get() in that 
case,

I would prefer a code like this:

T interned;
do {
expungeStaleElements();
WeakEntry e = new WeakEntry<>(elem, stale);
WeakEntry exist = map.putIfAbsent(e, e);
interned = (exist == null)? elem: exist.get();
} while (interned == null);
return interned;


Hi Remi, Aleksey,

In case the loop retries, there's no need to construct another WeakEntry...

T interned;
 WeakEntry e = new WeakEntry<>(elem, stale);
do {
expungeStaleElements();
WeakEntry exist = map.putIfAbsent(e, e);
interned = (exist == null)? elem: exist.get();
} while (interned == null);
return interned;

Regards, Peter



The cast to WeakEntry in expungeStaleElements is not needed.

In WeakEntry.equals, the null check is not needed because null 
instanceof WeakEntry returns false,
and you don't need to cast obj.get() to a WeakEntry given you only 
to call equals on entry.get().

So the code should be:


   public boolean equals(Object obj) {
 if (!(obj instanceof WeakEntry)) {
   return false;
 }
   Object that = ((WeakEntry)obj).get();
 Object mine = get();
 return (that == null || mine == null)? this == obj: 
mine.equals(that);

 }


Otherwise, it looks good.

Rémi



Testing:
   - Linux x86_64 builds OK
   - Linux x86_64 java/lang/invoke/ jtreg passes OK

Since this is a proof-of-concept patch at this point, I kept the testing
minimal. Let me know what other testing you think is needed before the
integration.

Thanks,
Aleksey.

*** Rationale and details for the issue follow: ***

This issue was in limbo for an year. The performance issue with
MethodType.methodType factory leads to abysmal scalability on the
targeted tests. While this can arguably demonstrated on larger
workloads, the targeted microbenchmarks are showcasing it nicely. This
issue was blocking the JSR292 API performance testing, because on large
enough test server, you will inevitably be bitten by this.

If you run the JMH-enabled [1] microbenchmark:
  http://openjdk.java.net/projects/code-tools/jmh/
  http://cr.openjdk.java.net/~shade/7177472/jsr292bench.tar.gz

...on the current jdk8/jdk8, 2x2 i5 2.0 Ghz, Linux x86_64, you will see
the scalability is going down. ("testCached" makes sure the reference to
MethodType is always strongly reachable, "testNew" makes sure the
reference is mostly non-reachable, prompting cache re-fill, "testOOB"
does not make any precautions about that, and also does not suffer from
the static overheads entailed by the reference management in first two
tests).

1 thread:
MethodTypeBench.testCached 78 +-  1 nsec/op
MethodTypeBench.testNew   120 +-  1 nsec/op
MethodTypeBench.testOOB41 +-  1 nsec/op

4 threads:
MethodTypeBench.testCached495 +- 40 nsec/op
MethodTypeBench.testNew   611 +- 37 nsec/op
MethodTypeBench.testOOB   377 +- 10 nsec/op

In fact, on larger machines it may be as bad as tens/hundreds of
microseconds to look up the MethodType. With the patch applied, on the
same 2x2 machine:

1 thread:
MethodTypeBench.testCached 92 +-  1 nsec/op
MethodTypeBench.testNew   101 +-  1 nsec/op
MethodTypeBench.testOOB49 +-  1 nsec/op

4 threads:
MethodTypeBench.testCached129 +-  1 nsec/op
MethodTypeBench.testNew   142 +- 10 nsec/op
MethodTypeBench.testOOB89 +-  3 nsec/op


...which means the scalability is back: the average time to look up the
MethodType is not growing spectacularly when the thread count go up.
Again, the effect is astonishing on large machines.

Notice that we took some hit in single-threaded performance, and that is
due to two reasons: a) WeakEntry allocation in get(); b) CHM. With the
upcoming CHMv8, this starts to look better:

1 thread:
MethodTypeBench.testCached 87 +-  1 nsec/op
MethodTypeBench.testNew95 +-  1 nsec/op
MethodTypeBench.testOOB52 +-  1 nsec/op

4 threads:
MethodTypeBench.testCached121 +-  2 nsec/op
MethodTypeBench.testNew   141 +-  1 nsec/op
MethodTypeBench.testOOB93 +-  5 nsec/op

...which seems to indicate this change is future-proof.

Re: 8016217: More javadoc warnings


On 10/06/2013 18:29, Iris Clark wrote:

Hi, Alan.

The changes look good to me.

One thing caught my eye.  In java.util.Calendar, around lines 2681 and later around 2702 you 
removed the ""

--- old/src/share/classes/java/util/Calendar.java   2013-06-10 
10:37:52.585879060 +0100
+++ new/src/share/classes/java/util/Calendar.java   2013-06-10 
10:37:52.297879063 +0100
@@ -2681,9 +2681,9 @@
   * Returns whether thisCalendar  represents a time
   * before the time represented by the specified
   *Object. This method is equivalent to:
- *
+ *{@code
   * compareTo(when)<  0
- *
+ * }
   * if and only ifwhen  is aCalendar
   * instance. Otherwise, the method returnsfalse.
   *

But you left them in java.io.Console, javax.security.sasl.SasClient, etc.

The  isn't really needed here (and in several other places). 
I did experimental with removing a few of them, I guess I just didn't 
revert these two (but it's completely harmless). Thanks for paying close 
attention.


-Alan

Re: 8016217: More javadoc warnings

I took a pass through the patch and nothing leaped out at me either. I wondered 
about the use of   to prevent an expression from breaking across lines but 
decided it's OK.

On Jun 10 2013, at 03:31 , Alan Bateman wrote:

> 
> About 8 months ago I tried an early build of doclint [1] and used it to fix 
> up a bunch issues at the time [2]. It's been awhile, so I decided to try out 
> the latest version to see how it has progressed. All I can say is "Yikes".  
> The good news is that they reported against the original source and that 
> makes it easy when compared to tools that validate the generated html.
> 
> I decided to fix up a few issues, mostly syntax (escaping of > and < in 
> particular) and a few reference issues that were missed the last time (or are 
> new). There are thousands of other issues for anyone that wants to jump in.
> 
> I've put the webrev with the changes here:
> 
> http://cr.openjdk.java.net/~alanb/8016217/webrev/
> 
> In total this fixes ~500 issues, although 270 of those were coming from 
> java.sql.DatabaseMetaData due to the number of un-escaped usages of  "=>". In 
> many cases, the changes are simply to use {@code ..} or replace  with 
> {@code ...}. It is tempting to just do a global replace on existing 
>  usages (would fixing up content that is escaped of course).

Yes, it's very tempting to do a global pass of:

([^<]*) -> {@code $1}
([^<]*) -> {@code $1}
(.*) -> $1

For backporting reasons it would be nice to do this simultaneously for the 
7u-dev repo as well. Perhaps at the time of the JDK 9 repo split?

With the changes in definitions of i, em, b, strong elements in HTML 5 
(http://html5doctor.com/i-b-em-strong-element/) it's no longer important to 
convert  tags to  tags.

> I've run specdiff on the before & after to check that I didn't mess anything 
> up. One obvious difference is that code examples that use generics now have 
> the type parameters going through to the generated javadoc.
> 
> The webrev touches many areas but as the changes are trivial, I don't need a 
> reviewer from every area.
> 
> -Alan.
> 
> [1] http://openjdk.java.net/jeps/172
> [2] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/39cbe256c3d1

Dismal performance of String.intern()

2013-06-10 Thread Steven Schlansker

Hi core-libs-dev,

While doing performance profiling of my application, I discovered that nearly 
50% of the time deserializing JSON was spent within String.intern().  I 
understand that in general interning Strings is not the best approach for 
things, but I think I have a decent use case -- the value of a certain field is 
one of a very limited number of valid values (that are not known at compile 
time, so I cannot use an Enum), and is repeated many millions of times in the 
JSON stream.

I discovered that replacing String.intern() with a ConcurrentHashMap improved 
performance by almost an order of magnitude.

I'm not the only person that discovered this and was surprised: 
http://stackoverflow.com/questions/10624232/performance-penalty-of-string-intern

I've been excited about starting to contribute to OpenJDK, so I am thinking 
that this might be a fun project for me to take on and then contribute back.  
But I figured I should check in on the list before spending a lot of time 
tracking this down.  I have a couple of preparatory questions:

* Has this bottleneck been examined thoroughly before?  Am I wishing too hard 
for performance here?

* String.intern() is a native method currently.  My understanding is that there 
is a nontrivial penalty to invoking native methods (at least via JNI, not sure 
if this is also true for "built ins"?).  I assume the reason that this is 
native is so the Java intern is the same as C++-invoked interns from within the 
JVM itself.  Is this an actual requirement, or could String.intern be replaced 
with Java code?

* If the interning itself must be handled by a symbol table in C++ land as it 
is today, would a "second level cache" in Java land that invokes a native 
"intern0" method be acceptable, so that there is a low-penalty "fast path"?  If 
so, this would involve a nonzero memory cost, although I assume that a few 
thousand references inside of a Map is an OK price to pay for a (for example) 
5x speedup.

* I assume the String class itself is loaded at a very sensitive time during VM 
initialization.  Having String initialization trigger (for example) 
ConcurrentHashMap class initialization may cause problems or circularities.  If 
this is the case, would triggering such a load lazily on the first intern() 
call be "late enough" as to not cause problems?

I'm sure that if I get anywhere with this I will have more questions, but this 
should get me started. Thank you for any advice / insight you may be able to 
provide!

Steven

hg: jdk8/tl/jdk: 8013827: File.createTempFile hangs with temp file starting with 'com1.4'; ...

2013-06-10 Thread dan . xu

Changeset: 4a66dd1d7eea
Author:dxu
Date:  2013-06-10 11:06 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4a66dd1d7eea

8013827: File.createTempFile hangs with temp file starting with 'com1.4'
8011950: java.io.File.createTempFile enters infinite loop when passed invalid 
data
Reviewed-by: alanb

! src/share/classes/java/io/File.java
! src/windows/native/java/io/WinNTFileSystem_md.c
! test/java/io/File/CreateNewFile.java
! test/java/io/File/NulFile.java
+ test/java/io/File/createTempFile/SpecialTempFile.java

Re: 8016218: Warnings building corba repo due to missing hashCode methods

StubDelegateImpl.java::

- Could just call Objects.hashCode(ior);

RepIdDelegator::

- Could also call Objects.hashCode(delegate) as it extends nothing the 
super.equals(delegate) is calling Object.equals

Mike

On Jun 10 2013, at 04:11 , Alan Bateman wrote:

> 
> The warnings from the build of the corba repository on missing hashCode 
> methods have been annoying me for some time. I was hoping they would annoy 
> others too but looks like I blinked first :-)
> 
> Here's the webrev with the changes that I'd like to push to fix the issues:
> 
> http://cr.openjdk.java.net/~alanb/8016218/webrev/
> 
> Note that there are 3 real issues but they are obvious. I'm running the CORBA 
> tests to make sure that it doesn't break anything. Note that the bulk of the 
> warnings were coming from ParserTable but they turn out not to be interesting 
> because they are only used in the testing framework.
> 
> -Alan.

Re: RFR (jaxp): 8016153 : Property http://javax.xml.XMLConstants/property/accessExternalDTD is not recognized.

Thanks Lance!

As Daniel noted, I added warnings to these additional property settings,
but there are a couple of other places in the original JAXP 1.5 impl
that did not have warning. I've added the warning message now (in
ValidatorHandlerImpl.java and XMLReaderManager.java). Also, in
ValidatorHandlerImpl.java, there was a redundant setting at line 677
that would invalidate a 3rd party impl. This was a porting error when
the patch was ported from jaxp standalone[1] to JDK. It is now removed.

webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/

[1] jaxp standalone source
https://java.net/projects/jaxp-sources/sources/svn/content/trunk/xml-xerces/java/src/com/sun/org/apache/xerces/internal/jaxp/validation/ValidatorHandlerImpl.java?rev=3134

Thanks,
Joe

On 6/10/2013 9:44 AM, Lance Andersen - Oracle wrote:

looks fine joe

-lance
On Jun 8, 2013, at 4:11 PM, huizhe wang wrote:

Hi,

This patch is to continue the support of plugging in 3rd party parser impls
(such as Xerces) that do not support JAXP 1.5. The fix is to catch
SAXNotRecognizedException, print out a warning, and then continue as usual. The
warning is for the developer in case he actually sets the new access*
properties on the TransformerFactory but has a 3rd party parser on the
classpath that does not support the new properties.

webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/

I intend to backport the change to 7u. The change will be identical.

Thanks,
Joe

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: RFR (jaxp): 8016153 : Property http://javax.xml.XMLConstants/property/accessExternalDTD is not recognized.

2013-06-10 Thread Lance Andersen - Oracle

looks OK
On Jun 10, 2013, at 3:26 PM, huizhe wang wrote:

> Thanks Lance!
> 
> As Daniel noted, I added warnings to these additional property settings, but 
> there are a couple of other places in the original JAXP 1.5 impl that did not 
> have warning. I've added the warning message now (in 
> ValidatorHandlerImpl.java and XMLReaderManager.java). Also, in 
> ValidatorHandlerImpl.java, there was a redundant setting at line 677 that 
> would invalidate a 3rd party impl. This was a porting error when the patch 
> was ported from jaxp standalone[1] to JDK.  It is now removed.
> 
> webrev:
> http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/
> 
> [1] jaxp standalone source
> https://java.net/projects/jaxp-sources/sources/svn/content/trunk/xml-xerces/java/src/com/sun/org/apache/xerces/internal/jaxp/validation/ValidatorHandlerImpl.java?rev=3134
> 
> Thanks,
> Joe
> 
> On 6/10/2013 9:44 AM, Lance Andersen - Oracle wrote:
>> looks fine joe
>> 
>> -lance
>> On Jun 8, 2013, at 4:11 PM, huizhe wang wrote:
>> 
>>> Hi,
>>> 
>>> This patch is to continue the support of plugging in 3rd party parser impls 
>>> (such as Xerces) that do not support JAXP 1.5. The fix is to catch 
>>> SAXNotRecognizedException, print out a warning, and then continue as usual. 
>>> The warning is for the developer in case he actually sets the new access* 
>>> properties on the TransformerFactory but has a 3rd party parser on the 
>>> classpath that does not support the new properties.
>>> 
>>> webrev:
>>> http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/
>>> 
>>> I intend to backport the change to 7u.  The change will be identical.
>>> 
>>> Thanks,
>>> Joe
>> 
>> 
>> 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

Re: 8016218: Warnings building corba repo due to missing hashCode methods

2013-06-10 Thread Daniel Fuchs


On 6/10/13 9:04 PM, Mike Duigou wrote:

StubDelegateImpl.java::

- Could just call Objects.hashCode(ior);

RepIdDelegator::

- Could also call Objects.hashCode(delegate) as it extends nothing the 
super.equals(delegate) is calling Object.equals

Depends on whether you intend to backport the patch to jdk7.
I suspect that CORBA is like JAXP - in that it needs to be compiled with the
bootstrap JDK (if I remember well it's compiled before JAXP).

Otherwise - I'd agree with mike :-)

-- daniel



Mike

On Jun 10 2013, at 04:11 , Alan Bateman wrote:


The warnings from the build of the corba repository on missing hashCode methods 
have been annoying me for some time. I was hoping they would annoy others too 
but looks like I blinked first :-)

Here's the webrev with the changes that I'd like to push to fix the issues:

http://cr.openjdk.java.net/~alanb/8016218/webrev/

Note that there are 3 real issues but they are obvious. I'm running the CORBA 
tests to make sure that it doesn't break anything. Note that the bulk of the 
warnings were coming from ParserTable but they turn out not to be interesting 
because they are only used in the testing framework.

-Alan.

Re: RFR (jaxp): 8016153 : Property http://javax.xml.XMLConstants/property/accessExternalDTD is not recognized.

Thanks!

On 6/10/2013 12:28 PM, Lance Andersen - Oracle wrote:

looks OK
On Jun 10, 2013, at 3:26 PM, huizhe wang wrote:

Thanks Lance!

As Daniel noted, I added warnings to these additional property
settings, but there are a couple of other places in the original JAXP
1.5 impl that did not have warning. I've added the warning message
now (in ValidatorHandlerImpl.java and XMLReaderManager.java). Also,
in ValidatorHandlerImpl.java, there was a redundant setting at line
677 that would invalidate a 3rd party impl. This was a porting error
when the patch was ported from jaxp standalone[1] to JDK. It is now
removed.

webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/

[1] jaxp standalone source
https://java.net/projects/jaxp-sources/sources/svn/content/trunk/xml-xerces/java/src/com/sun/org/apache/xerces/internal/jaxp/validation/ValidatorHandlerImpl.java?rev=3134

Thanks,
Joe

On 6/10/2013 9:44 AM, Lance Andersen - Oracle wrote:

looks fine joe

-lance
On Jun 8, 2013, at 4:11 PM, huizhe wang wrote:

Hi,

webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/

I intend to backport the change to 7u. The change will be identical.

Thanks,
Joe

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

Re: RFR (jaxp): 8016153 : Property http://javax.xml.XMLConstants/property/accessExternalDTD is not recognized.

2013-06-10 Thread Daniel Fuchs

Hi Joe,

Looks good.
I wonder whether using java.util.logging would be more (or less)
appropriate than

System.err.println()?

best regards,

-- daniel

On 6/10/13 9:26 PM, huizhe wang wrote:

Thanks Lance!

As Daniel noted, I added warnings to these additional property
settings, but there are a couple of other places in the original JAXP
1.5 impl that did not have warning. I've added the warning message now
(in ValidatorHandlerImpl.java and XMLReaderManager.java). Also, in
ValidatorHandlerImpl.java, there was a redundant setting at line 677
that would invalidate a 3rd party impl. This was a porting error when
the patch was ported from jaxp standalone[1] to JDK. It is now removed.

webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/

[1] jaxp standalone source
https://java.net/projects/jaxp-sources/sources/svn/content/trunk/xml-xerces/java/src/com/sun/org/apache/xerces/internal/jaxp/validation/ValidatorHandlerImpl.java?rev=3134

Thanks,
Joe

On 6/10/2013 9:44 AM, Lance Andersen - Oracle wrote:

looks fine joe

-lance
On Jun 8, 2013, at 4:11 PM, huizhe wang wrote:

Hi,

This patch is to continue the support of plugging in 3rd party
parser impls (such as Xerces) that do not support JAXP 1.5. The fix
is to catch SAXNotRecognizedException, print out a warning, and then
continue as usual. The warning is for the developer in case he
actually sets the new access* properties on the TransformerFactory
but has a 3rd party parser on the classpath that does not support
the new properties.

webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/

I intend to backport the change to 7u. The change will be identical.

Thanks,
Joe

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: 8016218: Warnings building corba repo due to missing hashCode methods


On 10/06/2013 20:59, Daniel Fuchs wrote:

On 6/10/13 9:04 PM, Mike Duigou wrote:

StubDelegateImpl.java::

- Could just call Objects.hashCode(ior);

RepIdDelegator::

- Could also call Objects.hashCode(delegate) as it extends nothing 
the super.equals(delegate) is calling Object.equals

Depends on whether you intend to backport the patch to jdk7.
I suspect that CORBA is like JAXP - in that it needs to be compiled 
with the

bootstrap JDK (if I remember well it's compiled before JAXP).
The corba repository is compiled with the boot JDK so it's not currently 
possible to use new language features or libraries. Hopefully in time 
this wart in the build can be sorted out, maybe by compiling it at the 
same time (there are circularly dependencies due to support for the IIOP 
transport in a few areas, plus other CORBA interop). So you are right 
that Objects couldn't be used if back-ported (although I wasn't planning 
to do that).


-Alan.

Re: RFR (jaxp): 8016153 : Property http://javax.xml.XMLConstants/property/accessExternalDTD is not recognized.

On 6/10/2013 1:04 PM, Daniel Fuchs wrote:

Hi Joe,

Looks good.
I wonder whether using java.util.logging would be more (or less)
appropriate than

System.err.println()?

It would make sense. The current error/warning report system came from
the original source. System.err is consistent with that of the current
system.

Best,
Joe

best regards,

-- daniel

On 6/10/13 9:26 PM, huizhe wang wrote:

Thanks Lance!

As Daniel noted, I added warnings to these additional property
settings, but there are a couple of other places in the original JAXP
1.5 impl that did not have warning. I've added the warning message
now (in ValidatorHandlerImpl.java and XMLReaderManager.java). Also,
in ValidatorHandlerImpl.java, there was a redundant setting at line
677 that would invalidate a 3rd party impl. This was a porting error
when the patch was ported from jaxp standalone[1] to JDK. It is now
removed.

webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/

[1] jaxp standalone source
https://java.net/projects/jaxp-sources/sources/svn/content/trunk/xml-xerces/java/src/com/sun/org/apache/xerces/internal/jaxp/validation/ValidatorHandlerImpl.java?rev=3134

Thanks,
Joe

On 6/10/2013 9:44 AM, Lance Andersen - Oracle wrote:

looks fine joe

-lance
On Jun 8, 2013, at 4:11 PM, huizhe wang wrote:

Hi,

This patch is to continue the support of plugging in 3rd party
parser impls (such as Xerces) that do not support JAXP 1.5. The fix
is to catch SAXNotRecognizedException, print out a warning, and
then continue as usual. The warning is for the developer in case he
actually sets the new access* properties on the TransformerFactory
but has a 3rd party parser on the classpath that does not support
the new properties.

webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8016153/webrev/

I intend to backport the change to 7u. The change will be identical.

Thanks,
Joe

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: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement


Hi Aleksey,

When 'sun.reflect.noCaches=true' system property is defined, 
reflectionData() returns null. In that case, the code should cope 
without using ReflectionData (no caching). See other uses of 
reflectionData() (for example: in privateGetDeclaredFields())...


I should note that ReflectionData is invalidated when the class is 
redefined. I don't know if generic signature is one of those things that 
can change with class redefinition, but invalidation is just one purpose 
of ReflectionData and the other is caching (via SoftReference), so 
generic signature can be added to it for that purpose. If 
'genericSignature' is cached on ReflectionData then it would be 
consistent to also cache the derived 'genericInfo', what do you think?


I don't think 3 fields are needed (genericSignatureResolved, 
genericSignature & genericInfo). I think one boolean flag (say : 
genericInfoPresent) and genericInfo is all that is needed. If there was 
a singleton ClassRepository NONE instance, then event the boolean flag 
wouldn't be needed.


Regards, Peter


On 06/10/2013 05:53 PM, Aleksey Shipilev wrote:

Hi,

This is the follow-up on the issue Doug identified:
   http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017798.html

I had reworked the patch, webrev is here:
   http://cr.openjdk.java.net/~shade/8016236/webrev.01/

Notable differences from Doug's version are:
   - handle non-generic cases as well
   - reuse ReflectionData to cache the interfaces and generic signatures
   - code style (chained ternary operators blown up)
   - fixes the race along the way
   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6398355

Testing:
   - Linux x86_64/release: build OK
   - Linux x86_64/release: java/lang/reflect regression tests OK
   - Microbenchmarks show whooping increase in performance, see below

(If there are no stylistic and other comments, I would like to do the
JPRT submit and/or additional testing [which?]).

The benchmark is here (you need JMH to build and run):
   http://cr.openjdk.java.net/~shade/8016236/classbench.zip

Before:

Benchmark   Mode ThrCnt  Sec 
Mean   Mean errorUnits
o.b.ClassBench.generic_getGenericInterfaces avgt   1  51  
326.8351.220  nsec/op
o.b.ClassBench.generic_getGenericSuperclass avgt   1  51  
308.7932.828  nsec/op
o.b.ClassBench.generic_getTypeParametersavgt   1  51  
312.6271.637  nsec/op
o.b.ClassBench.raw_getGenericInterfaces avgt   1  51  
216.9838.902  nsec/op
o.b.ClassBench.raw_getGenericSuperclass avgt   1  51   
59.9330.183  nsec/op
o.b.ClassBench.raw_getTypeParametersavgt   1  51   
65.4690.284  nsec/op

Doug's version:

Benchmark   Mode ThrCnt  Sec 
Mean   Mean errorUnits
o.b.ClassBench.generic_getGenericInterfaces avgt   1  51   
15.1060.271  nsec/op
o.b.ClassBench.generic_getGenericSuperclass avgt   1  51
5.3040.024  nsec/op
o.b.ClassBench.generic_getTypeParametersavgt   1  51   
16.7390.045  nsec/op
o.b.ClassBench.raw_getGenericInterfaces avgt   1  51  
213.8251.346  nsec/op
o.b.ClassBench.raw_getGenericSuperclass avgt   1  51   
61.6510.394  nsec/op
o.b.ClassBench.raw_getTypeParametersavgt   1  51   
64.3400.522  nsec/op

After:

Benchmark   Mode ThrCnt  Sec 
Mean   Mean errorUnits
o.b.ClassBench.generic_getGenericInterfaces avgt   1  51   
14.9850.104  nsec/op
o.b.ClassBench.generic_getGenericSuperclass avgt   1  51
5.3000.008  nsec/op
o.b.ClassBench.generic_getTypeParametersavgt   1  51   
16.8740.175  nsec/op
o.b.ClassBench.raw_getGenericInterfaces avgt   1  51   
23.9410.177  nsec/op
o.b.ClassBench.raw_getGenericSuperclass avgt   1  51
8.5830.044  nsec/op
o.b.ClassBench.raw_getTypeParametersavgt   1  51   
12.4000.045  nsec/op

Thanks,
Aleksey.

Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement


Hi Aleksey,

While you're at it adding getInterfaces() cache, why not also 
getSuperclass()? Is it maybe already intrinsified?


Regards, Peter

On 06/10/2013 05:53 PM, Aleksey Shipilev wrote:

Hi,

This is the follow-up on the issue Doug identified:
   http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017798.html

I had reworked the patch, webrev is here:
   http://cr.openjdk.java.net/~shade/8016236/webrev.01/

Notable differences from Doug's version are:
   - handle non-generic cases as well
   - reuse ReflectionData to cache the interfaces and generic signatures
   - code style (chained ternary operators blown up)
   - fixes the race along the way
   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6398355

Testing:
   - Linux x86_64/release: build OK
   - Linux x86_64/release: java/lang/reflect regression tests OK
   - Microbenchmarks show whooping increase in performance, see below

(If there are no stylistic and other comments, I would like to do the
JPRT submit and/or additional testing [which?]).

The benchmark is here (you need JMH to build and run):
   http://cr.openjdk.java.net/~shade/8016236/classbench.zip

Before:

Benchmark   Mode ThrCnt  Sec 
Mean   Mean errorUnits
o.b.ClassBench.generic_getGenericInterfaces avgt   1  51  
326.8351.220  nsec/op
o.b.ClassBench.generic_getGenericSuperclass avgt   1  51  
308.7932.828  nsec/op
o.b.ClassBench.generic_getTypeParametersavgt   1  51  
312.6271.637  nsec/op
o.b.ClassBench.raw_getGenericInterfaces avgt   1  51  
216.9838.902  nsec/op
o.b.ClassBench.raw_getGenericSuperclass avgt   1  51   
59.9330.183  nsec/op
o.b.ClassBench.raw_getTypeParametersavgt   1  51   
65.4690.284  nsec/op

Doug's version:

Benchmark   Mode ThrCnt  Sec 
Mean   Mean errorUnits
o.b.ClassBench.generic_getGenericInterfaces avgt   1  51   
15.1060.271  nsec/op
o.b.ClassBench.generic_getGenericSuperclass avgt   1  51
5.3040.024  nsec/op
o.b.ClassBench.generic_getTypeParametersavgt   1  51   
16.7390.045  nsec/op
o.b.ClassBench.raw_getGenericInterfaces avgt   1  51  
213.8251.346  nsec/op
o.b.ClassBench.raw_getGenericSuperclass avgt   1  51   
61.6510.394  nsec/op
o.b.ClassBench.raw_getTypeParametersavgt   1  51   
64.3400.522  nsec/op

After:

Benchmark   Mode ThrCnt  Sec 
Mean   Mean errorUnits
o.b.ClassBench.generic_getGenericInterfaces avgt   1  51   
14.9850.104  nsec/op
o.b.ClassBench.generic_getGenericSuperclass avgt   1  51
5.3000.008  nsec/op
o.b.ClassBench.generic_getTypeParametersavgt   1  51   
16.8740.175  nsec/op
o.b.ClassBench.raw_getGenericInterfaces avgt   1  51   
23.9410.177  nsec/op
o.b.ClassBench.raw_getGenericSuperclass avgt   1  51
8.5830.044  nsec/op
o.b.ClassBench.raw_getTypeParametersavgt   1  51   
12.4000.045  nsec/op

Thanks,
Aleksey.

hg: jdk8/tl/jaxp: 8016153: Property http://javax.xml.XMLConstants/property/accessExternalDTD is not recognized.

2013-06-10 Thread huizhe . wang

Changeset: 5c84d4a878f1
Author:joehw
Date:  2013-06-10 14:42 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jaxp/rev/5c84d4a878f1

8016153: Property http://javax.xml.XMLConstants/property/accessExternalDTD is 
not recognized.
Reviewed-by: lancea, dfuchs

! src/com/sun/org/apache/xalan/internal/xsltc/compiler/Parser.java
! src/com/sun/org/apache/xalan/internal/xsltc/trax/Util.java
! 
src/com/sun/org/apache/xerces/internal/jaxp/validation/ValidatorHandlerImpl.java
! src/com/sun/org/apache/xml/internal/utils/XMLReaderManager.java

Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement



On 06/10/2013 10:57 PM, Peter Levart wrote:
I don't think 3 fields are needed (genericSignatureResolved, 
genericSignature & genericInfo). I think one boolean flag (say : 
genericInfoPresent) and genericInfo is all that is needed. If there 
was a singleton ClassRepository NONE instance, then even the boolean 
flag wouldn't be needed.


Hi Aleksey,

Here's what I meant by above (no genericSignature caching is needed and 
no additional fields):



private static class LazyHolder {
static final ClassRepository NULL_CLASS_REPOSITORY = 
ClassRepository.make("Ljava/lang/Object;", null);

}

// accessor for generic info repository
private ClassRepository getGenericInfo() {
ClassRepository genericInfo = this.genericInfo;
// lazily initialize repository if necessary
if (genericInfo == null) {
String signature = getGenericSignature();
// create and cache generic info repository
this.genericInfo = genericInfo = (signature == null)
? LazyHolder.NULL_CLASS_REPOSITORY
: ClassRepository.make(signature, getFactory());
}
//return cached repository
return genericInfo == LazyHolder.NULL_CLASS_REPOSITORY
   ? null
   : genericInfo;
}



Regards, Peter

Re: RFR (S) CR 7177472: JSR292: MethodType interning penalizes scalability

2013-06-10 Thread Christian Thalinger


On Jun 10, 2013, at 8:47 AM, Aleksey Shipilev  
wrote:

> On 06/09/2013 12:17 AM, Peter Levart wrote:
>> In case the loop retries, there's no need to construct another WeakEntry...
>> 
>>T interned;
>> WeakEntry e = new WeakEntry<>(elem, stale);
>>do {
>>expungeStaleElements();
>>WeakEntry exist = map.putIfAbsent(e, e);
>>interned = (exist == null)? elem: exist.get();
>>} while (interned == null);
>>return interned;
> 
> That's right, thanks!
> 
> The update is here:
>  http://cr.openjdk.java.net/~shade/7177472/webrev.02/
> 
> Testing:
>  - Linux x86_64 builds OK
>  - Linux x86_64 java/lang/invoke/ jtreg passes OK
>  - The microbenchmark scores in the original note are still the same

This looks good to me.  One thing I mentioned in the bug report is that V8's 
RegExp shows some MethodType related methods in a -Xprof run:

 Compiled + native Method 
 11.9% 2455 + 0 java.util.regex.Pattern$GroupTail.match 
 11.5% 2382 + 0 java.util.regex.Pattern$Start.match 
 10.2% 2110 + 0 java.util.regex.Pattern$5.isSatisfiedBy 
  8.5% 1765 + 0 java.util.regex.Pattern$Branch.match 
  7.1% 1461 + 0 java.lang.invoke.MethodType$WeakInternSet.get 
  3.4% 709 + 0 java.util.regex.Pattern$CharProperty.match 
  2.9% 597 + 1 java.lang.invoke.MethodType.makeImpl 

Could you run RegExp with Nashorn again with and without your changes?

-- Chris

> 
> -Aleksey.
>

Re: Dismal performance of String.intern()

2013-06-10 Thread Andrew Haley

On 06/10/2013 07:06 PM, Steven Schlansker wrote:
> 
> I discovered that replacing String.intern() with a ConcurrentHashMap
> improved performance by almost an order of magnitude.
> 
> I'm not the only person that discovered this and was surprised:
> http://stackoverflow.com/questions/10624232/performance-penalty-of-string-intern
> 
> I've been excited about starting to contribute to OpenJDK, so I am
> thinking that this might be a fun project for me to take on and then
> contribute back.  But I figured I should check in on the list before
> spending a lot of time tracking this down.  I have a couple of
> preparatory questions:
> 
> * Has this bottleneck been examined thoroughly before?  Am I wishing
> too hard for performance here?
> 
> * String.intern() is a native method currently.  My understanding is
> that there is a nontrivial penalty to invoking native methods (at
> least via JNI, not sure if this is also true for "built ins"?). 

That's not really true.  For simple JNI methods I've measured the
overhead in nanoseconds.

> I assume the reason that this is native is so the Java intern is the
> same as C++-invoked interns from within the JVM itself.  Is this an
> actual requirement, or could String.intern be replaced with Java
> code?

I don't think that replacing intern() with Java code is a great place
to begin.  Surely your first plan should be to improve the performance
of the C++ version.  Maybe the only problem is that it needs a better
hash table.  Rather than get into complex re-engineering of VM
startup, this might be an easy fix.

Andrew.

Re: Dismal performance of String.intern()

2013-06-10 Thread Steven Schlansker


On Jun 10, 2013, at 3:55 PM, Andrew Haley  wrote:

> On 06/10/2013 07:06 PM, Steven Schlansker wrote:
>> 
>> I discovered that replacing String.intern() with a ConcurrentHashMap
>> improved performance by almost an order of magnitude.
>> 
>> I'm not the only person that discovered this and was surprised:
>> http://stackoverflow.com/questions/10624232/performance-penalty-of-string-intern
>> 
>> I've been excited about starting to contribute to OpenJDK, so I am
>> thinking that this might be a fun project for me to take on and then
>> contribute back.  But I figured I should check in on the list before
>> spending a lot of time tracking this down.  I have a couple of
>> preparatory questions:
>> 
>> * Has this bottleneck been examined thoroughly before?  Am I wishing
>> too hard for performance here?
>> 
>> * String.intern() is a native method currently.  My understanding is
>> that there is a nontrivial penalty to invoking native methods (at
>> least via JNI, not sure if this is also true for "built ins"?). 
> 
> That's not really true.  For simple JNI methods I've measured the
> overhead in nanoseconds.
> 
>> I assume the reason that this is native is so the Java intern is the
>> same as C++-invoked interns from within the JVM itself.  Is this an
>> actual requirement, or could String.intern be replaced with Java
>> code?
> 
> I don't think that replacing intern() with Java code is a great place
> to begin.  Surely your first plan should be to improve the performance
> of the C++ version.  Maybe the only problem is that it needs a better
> hash table.  Rather than get into complex re-engineering of VM
> startup, this might be an easy fix.

OK, sounds reasonable.  I only mentioned moving it to Java because that's what 
I know and I already have shown it to be fast enough.  I'll see if I can hack 
the C++ code into faster shape preferentially.

Re: Dismal performance of String.intern()

2013-06-10 Thread Steven Schlansker


On Jun 10, 2013, at 3:55 PM, Andrew Haley  wrote:

> On 06/10/2013 07:06 PM, Steven Schlansker wrote:
>> 
>> I discovered that replacing String.intern() with a ConcurrentHashMap
>> improved performance by almost an order of magnitude.
>> 
>> I'm not the only person that discovered this and was surprised:
>> http://stackoverflow.com/questions/10624232/performance-penalty-of-string-intern
>> 
>> I've been excited about starting to contribute to OpenJDK, so I am
>> thinking that this might be a fun project for me to take on and then
>> contribute back.  But I figured I should check in on the list before
>> spending a lot of time tracking this down.  I have a couple of
>> preparatory questions:
>> 
>> * Has this bottleneck been examined thoroughly before?  Am I wishing
>> too hard for performance here?
>> 
>> * String.intern() is a native method currently.  My understanding is
>> that there is a nontrivial penalty to invoking native methods (at
>> least via JNI, not sure if this is also true for "built ins"?). 
> 
> That's not really true.  For simple JNI methods I've measured the
> overhead in nanoseconds.
> 

Sorry to double-reply, but I thought it worthwhile to note that the time to 
intern strings via ConcurrentHashMap is measured in the low double-digit 
nanoseconds on my machine, so this may indeed end up being nontrivial.  But I 
will consider it trivial unless I can measure it to be large.

>> I assume the reason that this is native is so the Java intern is the
>> same as C++-invoked interns from within the JVM itself.  Is this an
>> actual requirement, or could String.intern be replaced with Java
>> code?
> 
> I don't think that replacing intern() with Java code is a great place
> to begin.  Surely your first plan should be to improve the performance
> of the C++ version.  Maybe the only problem is that it needs a better
> hash table.  Rather than get into complex re-engineering of VM
> startup, this might be an easy fix.
> 
> Andrew.

Re: RFR : 7129185 : (M) Add Collections.{checked|empty|unmodifiable}Navigable{Map|Set}

I've done some further updates based upon feedback. I believe this is now 
"done" and ready for final review.

http://cr.openjdk.java.net/~mduigou/JDK-7129185/3/webrev/

I did find one inconsistency in the implementations SortedSet.headSet and 
SortedSet.tailSet methods.

Mike

On Jun 7 2013, at 10:58 , Mike Duigou wrote:

> Hello all;
> 
> I've incorporated feedback from previous rounds and expect to finalize this 
> addition soon.
> 
> http://cr.openjdk.java.net/~mduigou/JDK-7129185/2/webrev/
> 
> Any review feedback or suggestions of additional tests welcome.
> 
> Thanks,
> 
> Mike
> 
>

Re: RFR : 7129185 : (M) Add Collections.{checked|empty|unmodifiable}Navigable{Map|Set}

On Jun 10 2013, at 16:47 , Louis Wasserman wrote:

> Is there any reason that Collections.unmodifiableSortedSet() shouldn't just 
> delegate to Collections.unmodifiableNavigableSet(), and similarly for the 
> Maps?

Some of the implementations are pre-8 and almost certainly have serialized 
instances sitting on disk somewhere.

For the classes not introduced in Java 8 I opted to leave things exactly as 
they were since we won't ever be able to get rid of the existing impls. Also, 
if we added a fancy readResolve to EmptyMap that substituted a an 
EmptyNavigableMap for EmptySortedMap we would have a problem when we tried to 
serialized that as we would have forgotten what type it originally was and 
would serialize an EmptyNavigableMap rather than an EmptyMap instance. This 
couldn't be read by Java 7 or earlier.

A clean paper implementation of Collections would only require one Map and one 
Set implementing the most specialized interface but unfortunately we have to 
provide serialization compatibility.

Mike

> On Mon, Jun 10, 2013 at 4:36 PM, Mike Duigou  wrote:
> I've done some further updates based upon feedback. I believe this is now 
> "done" and ready for final review.
> 
> http://cr.openjdk.java.net/~mduigou/JDK-7129185/3/webrev/
> 
> I did find one inconsistency in the implementations SortedSet.headSet and 
> SortedSet.tailSet methods.
> 
> Mike
> 
> 
> On Jun 7 2013, at 10:58 , Mike Duigou wrote:
> 
> > Hello all;
> >
> > I've incorporated feedback from previous rounds and expect to finalize this 
> > addition soon.
> >
> > http://cr.openjdk.java.net/~mduigou/JDK-7129185/2/webrev/
> >
> > Any review feedback or suggestions of additional tests welcome.
> >
> > Thanks,
> >
> > Mike
> >
> >
> 
> 
> 
> 
> -- 
> Louis Wasserman

Review request JDK-8015912: jdeps support to output in dot file format

2013-06-10 Thread Mandy Chung

This adds jdeps --format=dot option to print the output in dot-style 
format that can be taken to generate a dependency graph.


Webrev at:
http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8015912/webrev.00/

This also extends --verbose option to take an optional argument ("class" 
and "package") to replace the --verbose-level option and cleans up 
PlatformClassPath to use java.nio.file.


thanks
Mandy

RFR (jaxp): 8016133 : Regression: diff. behavior with user-defined SAXParser