hg: jdk8/tl/jdk: 8029063: test/com/sun/jmx/snmp/NoInfoLeakTest.java does not compile with OpenJDK builds

[shanliang . jiang]

Changeset: c11553506228
Author:sjiang
Date:  2013-12-03 08:53 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c11553506228

8029063: test/com/sun/jmx/snmp/NoInfoLeakTest.java does not compile with 
OpenJDK builds
Reviewed-by: alanb, dfuchs

- test/com/sun/jmx/snmp/NoInfoLeakTest.java

Re: 8029281: Synchronization issues in Logger and LogManager

[Peter Levart]

Hi Daniel,

1st sorry for the delay. I promised looking at the patch, but was then 
distracted by other things. I think that synchronization in LogManager 
is correct now. The fact that Mandy thinks so is also reassuring.


I wonder if calling LoggerWeakRef.dispose() from 
LoggerContext.addLocalLoger() is needed now that you check the 
LogNode.loggerRef before clearing in LoggerWeakRef.dispose():


if (n.loggerRef == this) {
n.loggerRef = null;  // clear LogNode's weak 
ref to us

}


The LoggerContext.addLocalLoger() will already overwrite the 
namedLoggers Hashtable entry for the logger's name with new LoggerWeakRef:


namedLoggers.put(name, ref);

...and it will already overwrite the LogNode.loggerRef with new 
LoggerWeakRef:


// Find the new node and its parent.
LogNode node = getNode(name);
node.loggerRef = ref;

So the only effect of calling LoggerWeakRef.dispose() from 
LoggerContext.addLocalLoger() is a more prompt unlinking of 
LoggerWeakRef child, which has already been cleared but not yet 
disposed(), from parent logger. This will eventually happen in a call to 
LogManager.drainLoggerRefQueueBounded(), called from public 
LogManager.addLogger(), which is the only entry point to 
LoggerContext.addLocalLoger(). But it's not wrong as it is now when you 
also ensure more prompt unlinking of cleared LoggerWeakRef and code is 
more uniform and less fragile then it would be if you created and called 
a hypothetical disposeFromParent() which only took care of unlinking 
from parent logger...


Regards, Peter


On 11/29/2013 12:41 PM, Daniel Fuchs wrote:

Hi,

Here is a new revision that includes Peter's findings.
Thanks again for that Peter!



It has passed all jdk_core tests and running the new tests
in a loop hasn't triggered any of the issues that appeared
before the fix.

-- daniel

On 11/28/13 3:26 PM, Daniel Fuchs wrote:

On 11/27/13 10:34 PM, Peter Levart wrote:

Hi Daniel,

I have started looking at the LogManager change first, and here's the
1st race I found...


Hi Peter,

Thanks a lot for your feedback! see below...


  The new method LoggerWeakRef.dispose() can be called from 3 places:

- LoggerContext.addLocalLogger
- LoggerContext.findLogger
- LogManager.drainLoggerRefQueueBounded which is called from public
method LogManager.addLogger

The 1st and 2nd are guarded by LoggerContext.this lock, but the 3rd is
unguarded.

What can happen is the following:

Thread1:
 LogManager.addLogger(someLogger)
 LogManager.drainLoggerRefQueueBounded() // finds some enqueued
LoggerWeakRef for name == 'X.Y' and...
 LoggerWeakRef.dispose() // this is 1st call to dispose,
so...
 synchronized (LoggerWeakRef.this) {
disposed = true;
 }

---context switch---

Thread2:
 LogManager.addLogger(logger{name=='X.Y'})
 LogManager.drainLoggerRefQueueBounded()  // no enqueued
refs...
 LoggerContext.addLocalLogger(logger{name=='X.Y'}, true)
 LoggerWeakRef ref = namedLoggers.get(name); // returns
a non-null ref ('X.Y' still points to a cleared weak ref)
 if (ref.get() == null) {
 ref.dispose(); // this is a 2nd call to this ref,
so it returns quickly
 }
 // ... namedLoggers gets new entry, etc...
 LogNode node = getNode(name); // get node for 'X.Y'
*node.loggerRef = ref;*

---context switch---

Thread1: (still in LoggerWeakRef.dispose())
 if (node != null) {
*node.loggerRef = null;*  // clear LogNode's weak 
ref to

us *!!! NOT QUITE !!!*


... so Thread1 erroneously clears the reference from Node to new
LoggerWeakRef that was just added by Thread2 ...


Damn. I believe that you're right! I overlooked the fact that LogNode is
reused.

Locking on LogNode.context is indeed a good way of solving the issue.
The only method that we will call from within the lock is
LogNode.context.rmoveLoggerRef(name this) - and that already requires
a lock on LogNode.context so it would be safe.

I have also double checked the places where LogNode.loggerRef is set,
and all those places (except in dispose() so far) are already from
within a lock on LoggerContext.

I think I would prefer however to keep the 'disposed' flag - and to
check whether LogNode.loggerRef == this before clearing it.

Given that - as I said - LogNode.loggerRef is always modified
from within a lock on LoggerContext I think the following would
work:

void dispose() {
 synchronized(this) {
 if (disposed) return;
 disposed = true;
 }

 final LogNode n = node;
 if (n != null) {
 synchronized (n.context) {
 n.context.removeLoggerRef(name, this);
 name = null;
 if (n.loggerRef == this)

[7u]: org.omg.PortableServer.AdapterActivator.java compile fails on French platform

[Francis ANDRE]

Hi

Found a surprising issue when building the jdk7u on a French platform with this 
compile error


Z:\JDK\JDK7U-~2\build\windows-i586\corba\gensrc\org\omg\PortableServer\AdapterActivator.java:8: 
error: unmappable character for encoding ascii

* lundi 2 d?cembre 2013 19 h 22 CET
   ^

In effect, the header generated is

/**
* org/omg/PortableServer/AdapterActivator.java .
* Generated by the IDL-to-Java compiler (portable), version "3.2"
* from ../../../../src/share/classes/org/omg/PortableServer/poa.idl
* lundi 2 décembre 2013 19 h 22 CET
*/
 which contains the "é" character in the french month of december.

As the compiling command request an ASCII encoding, it fails as "é" is not 
ASCII.

make[5]: Entering directory 
`/cygdrive/Z/JDK/jdk7u-dev/corba/make/org/omg/PortableServer'
if [ -s 
Z:/JDK/JDK7U-~2/build/windows-i586/corba/tmp/org/org.omg.PortableServer/.classes.list 
] ; then \
  /usr/bin/cat 
Z:/JDK/JDK7U-~2/build/windows-i586/corba/tmp/org/org.omg.PortableServer/.classes.list; 
\
  C:/Progra~1/Java/jdk1.6.0_35/bin/java -XX:-PrintVMOptions 
-XX:+UnlockDiagnosticVMOptions -XX:-LogVMOutput -client -Xmx512m -Xms512m 
-XX:PermSize=32m -XX:MaxPermSize=160m 
"-Xbootclasspath/p:Z:/JDK/JDK7U-~2/build/WINDOW~1/LANGTO~1/dist/bootstrap/lib/javac.jar" 
-jar Z:/JDK/JDK7U-~2/build/WINDOW~1/LANGTO~1/dist/bootstrap/lib/javac.jar 
-XDignore.symbol.file=true -source 7 -target 7 *-encoding ascii *-classpath 
C:/Progra~1/Java/jdk1.6.0_35/lib/tools.jar  -Xprefer:source -sourcepath 
"Z:/JDK/JDK7U-~2/build/windows-i586/corba/gensrc;../../../../src/windows/classes;../../../../src/share/classes" 
-d Z:/JDK/JDK7U-~2/build/windows-i586/corba/classes 
@Z:/JDK/JDK7U-~2/build/windows-i586/corba/tmp/org/org.omg.PortableServer/.classes.list; 
\

fi


I would suggest to fix this issue by removing the "-encoding ascii" parameter 
since pao.idl should contains only ascii characters and is not subject to i18n 
problem (except for the idl-to-java translation time in the header comment).


Francis

RFR for JDK-6963118 Intermittent test failure: test/java/nio/channels/Selector/Wakeup.java fail intermittently (win)

[srikalyan chandrashekar]

Hi all, I am working on bug JDK-6963118 
 .

Root Cause:
- Sensitive timing dependency between events in Main and Sleeper threads 
are causes for test failure.


Suggested Fix:
  1) Main thread should wait for more than 1sec(made it 3sec) and check 
more often than 50ms(made it 1ms) intervals , sleeper thread may be 
still waiting for interrupt/wakeup hence main thread waiting for just 
1sec to flag a failure is premature . The reason is especially on 
windows high priority virus scanners etc run(we faced it when simulating 
failures) and kept the system busy.

  2) The test is essentially a sequence of 2 events
  a)Firing up wakeups/interrupts followed by a
  b)Check
 Check the sleeper.entries value and yield the main thread as required 
so that the above 2 events step in tandem.


The webrev is hosted at 
http://cr.openjdk.java.net/~cl/host_for_kal/6963118-Wakeup/ .

Please let me know if you have any comments or suggestions.

--

--
Thanks
kalyan

答复: 答复: RFR for JDK-7190106 RMI benchmark fails intermittently because of use of fixed port

[Tristan Yan]

Thank you Stuart, next time I will keep the last version.
I'm appreciated that you can sponsor this for me.
Thank you very much.
Tristan.

-邮件原件-
发件人: Stuart Marks 
发送时间: Tuesday, December 03, 2013 9:11 AM
收件人: Tristan Yan
抄送: core-libs-dev@openjdk.java.net
主题: Re: 答复: RFR for JDK-7190106 RMI benchmark fails intermittently because of 
use of fixed port

Hi Tristan,

Sorry (again^2) for the delay. Holiday weekend here in the U.S.

The latest webrev looks fine. Did you need a sponsor for this? I can push it 
for you.

I guess we also need an official Reviewer before I can push it.

(One additional point for the future. If there is more than one round of 
review, it's useful to post the new webrev alongside the old one, instead of 
overwriting it, so that old links are still valid. It's not likely to happen in 
this case, but if someone were to read the email archives and follow the link 
to an earlier webrev, they'd have no idea what I was talking about.)

s'marks

On 11/25/13 9:12 PM, Tristan Yan wrote:
> Hi Stuart
> Thanks for your code review. I updated the webrev according your suggestion.
> Could you review it again?
>
> http://cr.openjdk.java.net/~ewang/tristan/JDK-7190106/webrev.00/
> Tristan
>
> On 11/26/2013 10:36 AM, Stuart Marks wrote:
>> Hi Tristan,
>>
>> Finally getting back to this. Again, sorry for the delay.
>>
>> The changes look much better now. I've listed a bunch of items below, 
>> but they're all comparatively minor, even nitpicky. But there are a 
>> few things that should be cleaned up before we integrate this.
>>
>> Items listed below.
>>
>>
>> ** bench/serial/Main.java
>>
>>
>>  - The description should simply be "The Serialization benchmark 
>> test". This test has nothing to do with RMI, even though it's under 
>> the java/rmi hierarchy in the filesystem.
>>
>>  - parseArgs() uses strings-in-switch! Good, but put "break" on its 
>> own line instead of following the close-brace of the catch clause. 
>> (rmi/Main.java doesn't have this issue.)
>>
>>
>> ** bench/rmi/Main.java
>>
>>
>>  - Imports of java.util.logging.Level and Logger are unused?
>>
>>  - Missing "-server" line from usage().
>>
>>  - Interesting use of enum. Note by convention an enum is like a 
>> class and names are in mixed case, thus use OutputFormat instead of 
>> OUTPUT_FORMAT. Also note that the enum doesn't buy much, at least in 
>> terms of lines of code, since the enum declaration and enum instance 
>> overrides add about as much code as the case statement that got 
>> removed from setupReporter(). It does buy a bit of type-safety, though, so 
>> might as well leave it in.
>>
>>  - Enum instance HTML should be on a new line, like XML.
>>
>>  - Reflection code can be chained instead of declaring several 
>> locals. This is mainly a matter of taste, but to my eye it's cleaner. 
>> The main advantage is avoiding the need to come up with names for 
>> intermediate locals. For example:
>>
>>port = (int) Class.forName("TestLibrary")
>>  .getMethod("getUnusedRandomPort")
>>  .invoke(null);
>>
>>  - Catch clause at 389 can be of ReflectiveOperationException. This 
>> covers everything except IllegalArgumentException, which is 
>> unchecked, so you don't need to catch it.
>>
>> (Not sure why Method.invoke is declared to throw 
>> IllegalArgumentException, since generally only checked exceptions are 
>> declared in the throws clause.)
>>
>>  - Line 416, no need to mention RMISecurityManager in a comment, just 
>> make the change to use SecurityManager.
>>
>>  - It's kind of surprising that TEST_SRC_PATH appends the file 
>> separator to the test.src property. At line 241 test.src has to be 
>> fetched again to use it without the file separator.
>>
>>  - Line 234, instead of the java.home property, use the test.jdk property.
>> This will use the JDK under test instead of the JDK that's running 
>> jtreg. In practice it's unclear whether this makes any actual 
>> difference today, but it's good to try to keep this separation. Also, 
>> use file separators here instead of appending "/bin/java".
>>
>> (Hmmm. I observe that the RMI testlibrary invokes JVM subprocesses 
>> using
>> java.home.)
>>
>>
>> Thanks,
>>
>>
>> s'marks
>>
>>
>> On 11/20/13 1:49 PM, Stuart Marks wrote:
>>> Hi, sorry about the delay, I'm still backlogged from traveling. I'll 
>>> get to this soon.
>>>
>>> s'marks
>>>
>>> On 11/19/13 6:27 PM, Tristan Yan wrote:
 Hi Stuart
 Did you get chance to review it again.
 Let me know if you have any new comments or suggestions.
 Thanks
 Tristan

 -邮件原件-
 发件人: Tristan Yan
 发送时间: Thursday, November 14, 2013 11:09 PM
 收件人: Stuart Marks
 抄送: core-libs-dev@openjdk.java.net
 主题: 答复: RFR for JDK-7190106 RMI benchmark fails intermittently 
 because of use of fixed port

 Thank you Stuart
 It took me a little time to correct the code. sorry be late. This 
 is new webrev for the code c

Re: 答复: RFR for JDK-7190106 RMI benchmark fails intermittently because of use of fixed port

[Stuart Marks]

Hi Tristan,

Sorry (again^2) for the delay. Holiday weekend here in the U.S.

The latest webrev looks fine. Did you need a sponsor for this? I can push it for 
you.


I guess we also need an official Reviewer before I can push it.

(One additional point for the future. If there is more than one round of review, 
it's useful to post the new webrev alongside the old one, instead of overwriting 
it, so that old links are still valid. It's not likely to happen in this case, 
but if someone were to read the email archives and follow the link to an earlier 
webrev, they'd have no idea what I was talking about.)


s'marks

On 11/25/13 9:12 PM, Tristan Yan wrote:

Hi Stuart
Thanks for your code review. I updated the webrev according your suggestion.
Could you review it again?

http://cr.openjdk.java.net/~ewang/tristan/JDK-7190106/webrev.00/
Tristan

On 11/26/2013 10:36 AM, Stuart Marks wrote:

Hi Tristan,

Finally getting back to this. Again, sorry for the delay.

The changes look much better now. I've listed a bunch of items below, but
they're all comparatively minor, even nitpicky. But there are a few things
that should be cleaned up before we integrate this.

Items listed below.


** bench/serial/Main.java


 - The description should simply be "The Serialization benchmark test". This
test has nothing to do with RMI, even though it's under the java/rmi hierarchy
in the filesystem.

 - parseArgs() uses strings-in-switch! Good, but put "break" on its own line
instead of following the close-brace of the catch clause. (rmi/Main.java
doesn't have this issue.)


** bench/rmi/Main.java


 - Imports of java.util.logging.Level and Logger are unused?

 - Missing "-server" line from usage().

 - Interesting use of enum. Note by convention an enum is like a class and
names are in mixed case, thus use OutputFormat instead of OUTPUT_FORMAT. Also
note that the enum doesn't buy much, at least in terms of lines of code, since
the enum declaration and enum instance overrides add about as much code as the
case statement that got removed from setupReporter(). It does buy a bit of
type-safety, though, so might as well leave it in.

 - Enum instance HTML should be on a new line, like XML.

 - Reflection code can be chained instead of declaring several locals. This is
mainly a matter of taste, but to my eye it's cleaner. The main advantage is
avoiding the need to come up with names for intermediate locals. For example:

   port = (int) Class.forName("TestLibrary")
 .getMethod("getUnusedRandomPort")
 .invoke(null);

 - Catch clause at 389 can be of ReflectiveOperationException. This covers
everything except IllegalArgumentException, which is unchecked, so you don't
need to catch it.

(Not sure why Method.invoke is declared to throw IllegalArgumentException,
since generally only checked exceptions are declared in the throws clause.)

 - Line 416, no need to mention RMISecurityManager in a comment, just make the
change to use SecurityManager.

 - It's kind of surprising that TEST_SRC_PATH appends the file separator to
the test.src property. At line 241 test.src has to be fetched again to use it
without the file separator.

 - Line 234, instead of the java.home property, use the test.jdk property.
This will use the JDK under test instead of the JDK that's running jtreg. In
practice it's unclear whether this makes any actual difference today, but it's
good to try to keep this separation. Also, use file separators here instead of
appending "/bin/java".

(Hmmm. I observe that the RMI testlibrary invokes JVM subprocesses using
java.home.)


Thanks,


s'marks


On 11/20/13 1:49 PM, Stuart Marks wrote:

Hi, sorry about the delay, I'm still backlogged from traveling. I'll get to this
soon.

s'marks

On 11/19/13 6:27 PM, Tristan Yan wrote:

Hi Stuart
Did you get chance to review it again.
Let me know if you have any new comments or suggestions.
Thanks
Tristan

-邮件原件-
发件人: Tristan Yan
发送时间: Thursday, November 14, 2013 11:09 PM
收件人: Stuart Marks
抄送: core-libs-dev@openjdk.java.net
主题: 答复: RFR for JDK-7190106 RMI benchmark fails intermittently because of
use of fixed port

Thank you Stuart
It took me a little time to correct the code. sorry be late. This is new
webrev for the code change. Please help to review it again.

Description:
1. Convert shell script test to Java program test.
2. Using random server port by reusing Darryl Mocek's
work(TestLibrary.getUnusedRandomPort) to replace fixed server port.
3. Because TestLibrary doesn't have package. Here is using reflection to call
TestLibrary.getUnusedRandomPort. This is going to change when TestLibrary
moves to named package.
4. Using java Process class to start client process. Client and server are
sharing IO.
5. Also convert other shell script test runSerialBench.sh to java program test
also.
6. ProblemList has been changed to get back
java/rmi/reliability/benchmark/runRmiBench.sh test.

http://cr.openjdk.java.net/~pzhang/Tristan/7190106/

Re: 8029281: Synchronization issues in Logger and LogManager

[Mandy Chung]

On 11/29/2013 3:41 AM, Daniel Fuchs wrote:

Hi,

Here is a new revision that includes Peter's findings.
Thanks again for that Peter!





LogManager.java
   L156: make props volatile is good.

   I think the reason why LoggerWeakRef.node and parentRef don't need 
to be volatile because of the synchronized block to check and set 
disposed flag that ensures that only one thread will be doing the node 
and parentRef cleanup.  I agree that the two lines should be done with 
synchronized on the LoggerContext.


 node.context.removeLogger(name);
 node.loggerRef = null;  // clear LogNode's weak ref to us

I don't see anything obvious wrong with this patch.   Have you 
considered having the dispose() method simply synchronizes on 
node.context (probably need to get the LoggerContext when instantiating 
LoggerWeakRef and break dispose method into two - one for the node and 
the other for the parentRef)?I'm not proposing to do this in JDK 8 
since we are ramping down and get a low risk fix in.   It may be 
something to revisit for jdk 9 to a cleaner version.


Logger.java
   The assert this.loggerBundle == NO_RESOURCE_BUNDLE; in the Logger 
constructors doesn't seem to be needed.


Having an immutable LoggerBundle class is good to ensure the 
resourceBundleName and resourceBundle are consistent.


   L1930: is lb.userBundle and lb.resourceBundleName null when getting 
to this line?  Might be better to rename setupResourceInfo to 
setResourceBundleName (that creates a LoggerBundle with a null RB). On 
the other hand,  setResourceBundle will instantiate a LoggerBundle with 
non-null rbname and RB.  It wasn't obvious to me what does what.


2137 final String rbName = getResourceBundleName();
2138 if (!SYSTEM_LOGGER_RB_NAME.equals(rbName)
2139 && 
!SYSTEM_LOGGER_RB_NAME.equals(b.getBaseBundleName())) {
2140 return LoggerBundle.get(rbName, b);
2141 } else {
2142 return SYSTEM_BUNDLE;
2143 }

To get to the above line, either lb.userBundle is null or 
getResourceBundle() isoverridden.  L2126 already checks that this logger 
does not associate with the system rbname.  It seems to me that the 
special case here is when the Logger subclass overrides 
getResourceBundleName() to returnSYSTEM_LOGGER_RB_NAME or override 
getResourceBundle to return the "sun.util.logging.resources.logging" RB. 
I tink that we might just simplify the above and just return 
LoggerBundle.get(rbName, b) and no need to worry about those overridden 
case which does no use to such a Logger subclass with unmatched rbname 
and RB.  Same comment applied to L2157-2165.


thanks
Mandy

Re: RFR: MethodHandles.Lookup. Wrong access verification

[John Rose]

I appreciate the report and the proposed fixes.  We are looking at this 
seriously.

— John

P.S.  The root issue may be an accidental inheritance of private methods.   I 
have been thinking for a while that, in the name of compatibility, the JVMS 
allows an "invokevirtual" invocation to reach a private method, but that this 
is unfortunate, since "invokespecial" already supplies a more specific way to 
get to the method.  Likewise, an "invokeinterface" instruction can reach a 
virtual method of Object, while "invokevirtual" gives a more specific way.  
There are a number of odd pathways through the linkResolver, to support these 
extra combinations, that I would like to regularize or remove.  I think we 
should investigate tightening up the JVM linkage rules to avoid the extra 
corner cases.  That will probably have the effect of removing surprise effects 
like this one, as we continue to extend the JVM linkage rules.


On Dec 2, 2013, at 1:42 PM, Sebastian Sickelmann  
wrote:

> Hi,
> 
> some days ago i had written[0] that i maybe found a bug in access 
> verification in MethodHandles.Lookup.
> 
> Now i produces webrev's for the two repos jdk and hotspot.
> 
> In the jdk webrev [1] I implemented some additional tests.
> 
> In the hotspot webrev [2] I tried to fix the wrong behavior. I am not sure if 
> it is the right place to fix this.
> It is more an proof-of-concept--like--reverse-engineering-fix that fixes the 
> symptom, and i do not fully
> understand all the things in the patched source. In fact i think that we can 
> move line 187 to 182 instead
> of chaning line 245 in methodHandles.cpp. But as i said i do not fully 
> understand init_method_MemberName
> and all those CallInfo::vtable_call / CallInfo::itable_call / 
> CallInfo::direct_call differences.
> 
> Hope my works helps solving this issue. And i hope it is an issue. And i hope 
> my fix isn't breaking anything else
> so that it was enough to test my fix against my new testcase in 
> MethodHandlesTest.testFindPrivate
> 
> Kind regards
> Sebastian
> 
> [0] 
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2013-November/010228.html
> [1] 
> https://dl.dropboxusercontent.com/u/43692695/oss-patches/openjdk8/MethodHandles.Lookup/webrev_jdk/index.html
> [2] 
> https://dl.dropboxusercontent.com/u/43692695/oss-patches/openjdk8/MethodHandles.Lookup/webrev_hotspot/index.html

Re: Review Request for 8029216: (jdeps) Provide a specific option to report JDK internal APIs

[Mandy Chung]

On 11/27/13 4:04 AM, Alan Bateman wrote:

On 26/11/2013 23:59, Mandy Chung wrote:
This is a simple patch that adds a new jdeps -jdkinternals option to 
make it easier for developers to find dependencies on the JDK 
internal APIs:

http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8029216/webrev.00/
This looks good (and I can't think of a better name for the option). 
Do you think this needs a test as otherwise this option will not be 
exercised?


Here is the updated webrev:
http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8029216/webrev.01/

I added the new test cases.  This also updates -v to include the from 
class in the o and the verbose mode prints more information.


Mandy

Re: Review Request for 8029216: (jdeps) Provide a specific option to report JDK internal APIs

[Mandy Chung]

On 11/27/13 4:04 AM, Alan Bateman wrote:

On 26/11/2013 23:59, Mandy Chung wrote:
This is a simple patch that adds a new jdeps -jdkinternals option to 
make it easier for developers to find dependencies on the JDK 
internal APIs:

http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8029216/webrev.00/
This looks good (and I can't think of a better name for the option). 
Do you think this needs a test as otherwise this option will not be 
exercised?


Here is the updated webrev:
http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8029216/webrev.01/

I added the new test cases.  This also updates -v to include the from 
class in the output and the verbose mode prints more information.


Mandy

RFR: MethodHandles.Lookup. Wrong access verification

[Sebastian Sickelmann]

Hi,

some days ago i had written[0] that i maybe found a bug in access
verification in MethodHandles.Lookup.

Now i produces webrev's for the two repos jdk and hotspot.

In the jdk webrev [1] I implemented some additional tests.

In the hotspot webrev [2] I tried to fix the wrong behavior. I am not
sure if it is the right place to fix this.
It is more an proof-of-concept--like--reverse-engineering*//*
-fix
that fixes the symptom, and i do not fully
understand all the things in the patched source. In fact i think that we
can move line 187 to 182 instead
of chaning line 245 in methodHandles.cpp. But as i said i do not fully
understand init_method_MemberName
and all those CallInfo::vtable_call / CallInfo::itable_call /
CallInfo::direct_call differences.

Hope my works helps solving this issue. And i hope it is an issue. And i
hope my fix isn't breaking anything else
so that it was enough to test my fix against my new testcase in
MethodHandlesTest.testFindPrivate

Kind regards
Sebastian

[0]
http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2013-November/010228.html
[1]
https://dl.dropboxusercontent.com/u/43692695/oss-patches/openjdk8/MethodHandles.Lookup/webrev_jdk/index.html
[2]
https://dl.dropboxusercontent.com/u/43692695/oss-patches/openjdk8/MethodHandles.Lookup/webrev_hotspot/index.html

Re: There needs to be support for java.time in java.text.MessageFormat

[Xueming Shen]

On 12/02/2013 12:46 PM, Stephen Colebourne wrote:

On 2 December 2013 19:47, Xueming Shen  wrote:

That said, it might be desired to add the support of the JSR310 date/time
for the DateFormat, but it will not make jdk8.

FWIW, I think this should have been fixed in jdk 8.
https://bugs.openjdk.java.net/browse/JDK-8016743

Does the spec allow it to be fixed in a jdk 8 update release?


DateFormat.format(Object obj, ...) has the "obj" specified as "must be a Number or a 
Date".
So it's going to be a spec change. Though it probably is not going to break any 
existing code
except the tck.

-Sherman



Stephen

Re: JDK 8 RFR 8022181: Tune algorithm crossover thresholds in BigInteger

[Joseph Darcy]

Hi Brian,

The new thresholds look reasonable, as is planned follow-up tuning in JDK 9.

Thanks,

-Joe

On 12/2/2013 12:54 PM, Brian Burkhalter wrote:

Hello,

Issue:  https://bugs.openjdk.java.net/browse/JDK-8022181
Webrev: http://cr.openjdk.java.net/~bpb/8022181/webrev/

Based on numerous micro-benchmark test runs on various operating systems and 
architectures I would like to propose changing the BigInteger algorithm 
crossover thresholds as indicated in the webrev from which I excerpt here for 
convenience:

-private static final int KARATSUBA_THRESHOLD = 50;
+private static final int KARATSUBA_THRESHOLD = 80;
  
-private static final int TOOM_COOK_THRESHOLD = 75;

+private static final int TOOM_COOK_THRESHOLD = 240;
  
-private static final int KARATSUBA_SQUARE_THRESHOLD = 90;

+private static final int KARATSUBA_SQUARE_THRESHOLD = 128;

-private static final int TOOM_COOK_SQUARE_THRESHOLD = 140;
+private static final int TOOM_COOK_SQUARE_THRESHOLD = 216;
  
-static final int BURNIKEL_ZIEGLER_THRESHOLD = 50;

+static final int BURNIKEL_ZIEGLER_THRESHOLD = 80;

-private static final int SCHOENHAGE_BASE_CONVERSION_THRESHOLD = 8;
+private static final int SCHOENHAGE_BASE_CONVERSION_THRESHOLD = 20;

Each threshold is approximately the smallest value for which no statistically 
significant performance regression was observed with respect to the baseline. 
For the Toom-Cook algorithms, the baselines are the respective Karatsuba 
algorithms; for the other algorithms, the baselines are the respective 
algorithms previously implemented in BigInteger. Also, these new suggested 
thresholds were benchmarked against a performance baseline defined by the 
previous values of the respective thresholds.

With respect to the potential objection that the proposed thresholds are too 
high, please note that the values are subject to further refinement in the 
future. This issue https://bugs.openjdk.java.net/browse/JDK-8029425 has been 
filed as a reminder to continue to refine these values in the JDK 9 time frame 
although such fine tuning could as well occur in JDK 8 updates.

Thanks,

Brian

hg: jdk8/tl/jdk: 8029417: JDBC 4.2 javadoc updates

[lance . andersen]

Changeset: bcf5fa5e9509
Author:lancea
Date:  2013-12-02 16:06 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bcf5fa5e9509

8029417: JDBC 4.2 javadoc updates
Reviewed-by: darcy

! src/share/classes/java/sql/CallableStatement.java
! src/share/classes/java/sql/DriverManager.java
! src/share/classes/java/sql/JDBCType.java
! src/share/classes/java/sql/PreparedStatement.java
! src/share/classes/java/sql/ResultSet.java
! src/share/classes/java/sql/SQLPermission.java

Re: RFR JDK-8028397: Undo the lenient MIME BASE64 decoder support change (JDK-8025003) and remove methods de/encode(buf, buf)

[Alan Bateman]

On 02/12/2013 19:25, Xueming Shen wrote:

Hi,

Based on the feedback so far here is the latest webrev, and the link 
to the bug.


http://cr.openjdk.java.net/~sherman/8028397/webrev
https://bugs.openjdk.java.net/browse/JDK-8028397

During the discussion it became clearthat methods 
decode/encode(ByteBuffer,

ByteBuffer) are insufficient to support the continuous de/encoding
operation request ("underflow" handling, in charset coder's term).
Given the assumed usage for these methods is for de/encoding the bytes
from io channels, I have been convinced it might be more useful/desired
to provide a pair of wrap methods instead for 
readable/writableByteChannel,

same as the pair we have now for in/output stream (the wrap methods
are under development, but will probably be for future releases). For
jdk8 the proposal is to remove the pair of de/encode(Buffer, ByteBuffer),
as showed in the webrev.

I think this is the right thing to do and the changes in the webrev look 
okay to me (and easy to review as it's just an undo of a recent change 
plus the removal of two methods).


-Alan.

Re: Review request 8029417: Minor javadoc cleanup for JDBC 4.2

[Joe Darcy]

Hi Lance,

Looks fine; cheers,

-Joe

On 12/02/2013 11:33 AM, Lance Andersen - Oracle wrote:

Hi all

This is a review request for some minor javadoc clarifications for JDBC 4.2 
based on some feedback that I received.

The webrev can be found http://cr.openjdk.java.net/~lancea/8029417/webrev.00

Best
Lance

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

JDK 8 RFR 8022181: Tune algorithm crossover thresholds in BigInteger

[Brian Burkhalter]

Hello,

Issue:  https://bugs.openjdk.java.net/browse/JDK-8022181
Webrev: http://cr.openjdk.java.net/~bpb/8022181/webrev/

Based on numerous micro-benchmark test runs on various operating systems and 
architectures I would like to propose changing the BigInteger algorithm 
crossover thresholds as indicated in the webrev from which I excerpt here for 
convenience:

-private static final int KARATSUBA_THRESHOLD = 50;
+private static final int KARATSUBA_THRESHOLD = 80;
 
-private static final int TOOM_COOK_THRESHOLD = 75;
+private static final int TOOM_COOK_THRESHOLD = 240;
 
-private static final int KARATSUBA_SQUARE_THRESHOLD = 90;
+private static final int KARATSUBA_SQUARE_THRESHOLD = 128;

-private static final int TOOM_COOK_SQUARE_THRESHOLD = 140;
+private static final int TOOM_COOK_SQUARE_THRESHOLD = 216;
 
-static final int BURNIKEL_ZIEGLER_THRESHOLD = 50;
+static final int BURNIKEL_ZIEGLER_THRESHOLD = 80;

-private static final int SCHOENHAGE_BASE_CONVERSION_THRESHOLD = 8;
+private static final int SCHOENHAGE_BASE_CONVERSION_THRESHOLD = 20;

Each threshold is approximately the smallest value for which no statistically 
significant performance regression was observed with respect to the baseline. 
For the Toom-Cook algorithms, the baselines are the respective Karatsuba 
algorithms; for the other algorithms, the baselines are the respective 
algorithms previously implemented in BigInteger. Also, these new suggested 
thresholds were benchmarked against a performance baseline defined by the 
previous values of the respective thresholds.

With respect to the potential objection that the proposed thresholds are too 
high, please note that the values are subject to further refinement in the 
future. This issue https://bugs.openjdk.java.net/browse/JDK-8029425 has been 
filed as a reminder to continue to refine these values in the JDK 9 time frame 
although such fine tuning could as well occur in JDK 8 updates.

Thanks,

Brian

Re: There needs to be support for java.time in java.text.MessageFormat

[Stephen Colebourne]

On 2 December 2013 19:47, Xueming Shen  wrote:
> That said, it might be desired to add the support of the JSR310 date/time
> for the DateFormat, but it will not make jdk8.

FWIW, I think this should have been fixed in jdk 8.
https://bugs.openjdk.java.net/browse/JDK-8016743

Does the spec allow it to be fixed in a jdk 8 update release?

Stephen

Re: RFR 8029164: Race condition in CompletableFuture.thenCompose with asynchronous task

[Mike Duigou]

Looks fine to me.

Mike

On Nov 27 2013, at 03:02 , Paul Sandoz  wrote:

> Hi,
> 
> https://bugs.openjdk.java.net/browse/JDK-8029164
> 
> http://cr.openjdk.java.net/~psandoz/tl/JDK-8029164-thenCompose-async/webrev/
> 
> This fixes an issue with CompletableFuture.thenCompose.  
> 
> If the composing future is already completed before the call to thenCompose 
> (or completes during some part of the call to thenCompose) and the composing 
> function returns a future that is not yet completed then there is race to set 
> the result, usually won by thenCompose which incorrectly sets the result to 
> null, due to an errant conditional statement.
> 
> Doug has reviewed and committed the fix to the 166 repo and we have double 
> checked there are no similar errors lurking in other areas of 
> CompletableFuture code.
> 
> Paul.

Re: There needs to be support for java.time in java.text.MessageFormat

[Nick Williams]

On Dec 2, 2013, at 1:47 PM, Xueming Shen wrote:

> On 12/01/2013 01:13 PM, Nick Williams wrote:
>> On Dec 1, 2013, at 12:52 PM, Xueming Shen wrote:
>> 
>>> On 12/1/13 10:29 AM, Nick Williams wrote:
 I filed these bugs back in June. I noticed today that they were migrated 
 to the JIRA instance:
 
 https://bugs.openjdk.java.net/browse/JDK-8016742
 https://bugs.openjdk.java.net/browse/JDK-8016743
 
 I filed the bugs, though they say someone else did. It's frustrating that 
 suddenly I can't even comment on bugs I created, but that's beside the 
 point of this email...
 
 I'm dismayed to see that 8016743 has been scheduled for being fixed in 
 JAVA 9! What the heck!? If I can't use the new Java 8 Date&  Time types in 
 Java's localization support, then what good are they!? I have to just 
 stick with java.util.Date for yet another two years because I can't 
 localize Java 8 Date&  Time types in my i18n message formats. That's not 
 the quality that I normally expect out of the Java team.>:-[
>>> java.time has its own formatting/parsing mechanism/support via 
>>> java.time.format package, in which
>>> it does have l10n/i18n support for print/parsing the new java.time 
>>> date/time types for various locales.
>>> Any particular reason you must use j.text.MessageFormat to print/parse the 
>>> java.time date/time types?
>> Yes: So that you can use message formats in ResourceBundles with java.time 
>> types. For example:
> 
> The idea here is to encourage the use of java.time.format for java.time 
> date/time
> types, not the Message/DateFormat for the new JSR310 date/time. You should be
> able to use DateTimeFormatterBuilder + ResourceBundles to obtain a 
> DateTimeFormatter
> for particular locale with customized localized message, for your JSR310 
> data/time.

Nope.

MessageFormat is a universal i18n formatting class for messages that contain 
number, currency, percentage, date, and time parameters. If I have a message 
like this:

You last logged in on {0,date,long} at {0,time,long}. You have logged in 
{1,number} times.

I should be able to pass this message to a single method/class, supply 
parameters/arguments, and have the message formatted correctly. With 
MessageFormat, I can. The problem is that Java 8 now offers more date and time 
types, and MessageFormat was not updated to support those new types. That would 
be like adding a new primitive to Java (let's call it "extralong") and not 
updating MessageFormat to support that primitive. I shouldn't have to parse the 
message myself, pull out the date and time parameters, pass those to a 
java.time.format.DateTimeFormatter to format those parameters, replace them in 
the message, then pass the rest of the message to the MessageFormat to handle 
other parameters.

> Yes, I guess the  tag should support the java.time.format.DTF, if
> the argument is a JSR310 date/time types.

This isn't just about that tag. That tag doesn't support OR not support 
java.text.DateFormat or java.time.format.DTF. It supports MessageFormats. 
That's it. MessageFormat is part of the SE's core i18n support. If java.time 
can't work with MessageFormats, it's going to be a problem. The solution is for 
MessageFormat to support java.time.DTF.

> That said, it might be desired to add the support of the JSR310 date/time
> for the DateFormat, but it will not make jdk8.
> 
> -Sherman
> 
>> [resource bundle file contents]
>> some.message.key=You last logged in on {1,date,long} at {2,time,long}.
>> [/resource bundle file contents]
>> 
>> And then, in a JSP (just one example use case):
>> 
>> 
>> 
>> 
>> 
>> 
>> Resource bundle messages _must_ follow the MessageFormat conventions. 
>> MessageFormat only supports java.text.DateFormat. DateFormat only supports 
>> java.util.Date. MessageSource needs to also support 
>> java.time.format.DateTimeFormatter, otherwise you can't use java.time types 
>> with resource bundle messages.
>> 
>> Nick
>> 
>>> -Sherman
>>> 
 8016742 is a slightly different story. It's higher priority than 8016743, 
 and although there is absolutely no update about it, it appears that MAYBE 
 it's scheduled for being fixed in Java 8? I have no idea what "tbd_major" 
 means. Note that a fix for 8016743 could potentially help fix 8016742.
 
 Is there anyone here that can shed some light on 8016742's status and why 
 the heck 8016743 isn't getting fixed until Java 9?
 
 If not, can someone point me to a more appropriate list that I can 
 escalate my frustrations on? These awesome new date&  time types are 
 useless if they aren't supported in Java's i18n/L10n and JAXB components.
 
 N
 
 On Jun 17, 2013, at 8:17 AM, Nick Williams wrote:
 
> On Jun 17, 2013, at 6:53 AM, Stephen Colebourne wrote:
> 
>> On 17 June 2013 12:40, Alan Bateman  wrote:
>>> On 17/06/2013 11:05, Nick Williams wrote:
 Thanks. I have filed two di

Re: RFR for JDK-6772009 Intermittent test failure: java/util/concurrent/locks/ReentrantLock/CancelledLockLoops.java test failed with 'Completed != 2'

[srikalyan]

Hi David, Thanks for the review, the new webrev is hosted at 
http://cr.openjdk.java.net/~cl/host_for_kal/6772009-CancelledLockLoop/ . 
Please see inline text.


On 11/20/13, 6:35 PM, David Holmes wrote:

On 21/11/2013 10:28 AM, Martin Buchholz wrote:

I again tried and failed to reproduce a failure.  Even if I go whole hog
and multiply TIMEOUT by 100 and divide ITERS by 100, the test 
continues to

PASS.  Is it just me?!


I think you are going the wrong way Martin - you want the timeout to 
be smaller than the time it takes to execute ITERS.


I don't think there's any reason to make result long.  It's not even 
used

except to inhibit hotspot optimizations.

+private volatile long result = 17;//Can get int overflow,so 
using long


Further the subsequent use of += is incorrect as it is not an atomic 
operation. Even if we don't care about the value, it looks bad.

Made the necessary changes for atomic update.


I'm not sure result must be updated if we get a BrokenBarrierException 
either. Probably harmless, but necessary?
I retained it in the fix for completeness in updating the numbers, 
please let me know if you still think otherwise.



need to fix spelling and spacing below.

+barrier.await();//If a BrokeBarrierException happens
here(due to


There are a number of style issues with spacing around comments.

Fixed the spelling error and styling issues.


And I don't think this change is sufficient to claim co-author status 
with Doug either ;-)

Removed the claim :)


The additional tracing may be useful but seems stylistically different 
from the rest of the code.
Retained the tracking to understand if it is again the timing issue 
which is the cause in an event of a failure, however i can remove it if 
you think it is not necessary (OR) include an alternate solution as you 
may want to suggest.


Overall I'm suspicious that the changed timeout actually fixes 
anything - normally we need to add longer timeouts not shorter ones. 
Does this fail on a range of machines or only specific ones? Have we 
verified that the clocks/timers are behaving properly on those systems?
Here the time out is not about waiting for threads to complete something 
but to "be interrupted" before being considered done, so we decreased 
the timeout. However we now chose to increase the number of iterations 
to 500 from 100(thanks to tristan for the suggestion) instead of 
decreasing the timeout as done earlier because the increasing iterations 
ensures the threads are busy for long time curtailing the need to touch 
the timeout.




Thanks,
David


--
Thanks
kalyan
Ph: (408)-585-8040







On Wed, Nov 20, 2013 at 11:52 AM, srikalyan <
srikalyan.chandrashe...@oracle.com> wrote:

  Hi Martin , apologies for the delay , was trying to get help for 
hosting

my webrev.  .  Please see inline text.


On 11/19/13, 10:35 PM, Martin Buchholz wrote:

Hi Kalyan,

  None of us can review your changes yet because you haven't given us a
URL of your webrev.

It is located here
http://cr.openjdk.java.net/~cl/host_for_srikalyan_6772009_CancelledLockLoops/ 




  I've tried to make the jsr166 copy of CancelledLockLoops fail by
adjusting ITERS and TIMEOUT radically up and down, but the test just 
keeps

on passing for me.  Hints appreciated.

Bump up the timeout to 500ms and you will see a failure (i can see it
consistently on my machine Linux 64bit,8GBRAM,dual cores, with JDK 1.8
latest any promoted build).



On Tue, Nov 19, 2013 at 6:39 PM, srikalyan chandrashekar <
srikalyan.chandrashe...@oracle.com> wrote:


Suggested Fix:
a) Decrease the timeout from 100 to 50ms which will ensure that 
the test

will pass even on faster machines



  This doesn't look like a permanent fix - it just makes the failing 
case

rarer.

Thats true , the other way is to make the main thread wait on TIMEOUT
after firing the interrupts instead of other way round, but that 
would be
over-optimization which probably is not desirable as well.  The 50 
ms was

arrived at empirically after running several 100 times on multiple
configurations and did not cause failures.

--
Thanks
kalyan
Ph: (408)-585-8040

Re: There needs to be support for java.time in java.text.MessageFormat

[Xueming Shen]

On 12/01/2013 01:13 PM, Nick Williams wrote:

On Dec 1, 2013, at 12:52 PM, Xueming Shen wrote:


On 12/1/13 10:29 AM, Nick Williams wrote:

I filed these bugs back in June. I noticed today that they were migrated to the 
JIRA instance:

https://bugs.openjdk.java.net/browse/JDK-8016742
https://bugs.openjdk.java.net/browse/JDK-8016743

I filed the bugs, though they say someone else did. It's frustrating that 
suddenly I can't even comment on bugs I created, but that's beside the point of 
this email...

I'm dismayed to see that 8016743 has been scheduled for being fixed in JAVA 9! What the 
heck!? If I can't use the new Java 8 Date&  Time types in Java's localization support, 
then what good are they!? I have to just stick with java.util.Date for yet another two 
years because I can't localize Java 8 Date&  Time types in my i18n message formats. 
That's not the quality that I normally expect out of the Java team.>:-[

java.time has its own formatting/parsing mechanism/support via java.time.format 
package, in which
it does have l10n/i18n support for print/parsing the new java.time date/time 
types for various locales.
Any particular reason you must use j.text.MessageFormat to print/parse the 
java.time date/time types?

Yes: So that you can use message formats in ResourceBundles with java.time 
types. For example:


The idea here is to encourage the use of java.time.format for java.time 
date/time
types, not the Message/DateFormat for the new JSR310 date/time. You should be
able to use DateTimeFormatterBuilder + ResourceBundles to obtain a 
DateTimeFormatter
for particular locale with customized localized message, for your JSR310 
data/time.

Yes, I guess the  tag should support the java.time.format.DTF, if
the argument is a JSR310 date/time types.

That said, it might be desired to add the support of the JSR310 date/time
for the DateFormat, but it will not make jdk8.

-Sherman


[resource bundle file contents]
some.message.key=You last logged in on {1,date,long} at {2,time,long}.
[/resource bundle file contents]

And then, in a JSP (just one example use case):


 
 


Resource bundle messages _must_ follow the MessageFormat conventions. 
MessageFormat only supports java.text.DateFormat. DateFormat only supports 
java.util.Date. MessageSource needs to also support 
java.time.format.DateTimeFormatter, otherwise you can't use java.time types 
with resource bundle messages.

Nick


-Sherman


8016742 is a slightly different story. It's higher priority than 8016743, and although 
there is absolutely no update about it, it appears that MAYBE it's scheduled for being 
fixed in Java 8? I have no idea what "tbd_major" means. Note that a fix for 
8016743 could potentially help fix 8016742.

Is there anyone here that can shed some light on 8016742's status and why the 
heck 8016743 isn't getting fixed until Java 9?

If not, can someone point me to a more appropriate list that I can escalate my 
frustrations on? These awesome new date&  time types are useless if they aren't 
supported in Java's i18n/L10n and JAXB components.

N

On Jun 17, 2013, at 8:17 AM, Nick Williams wrote:


On Jun 17, 2013, at 6:53 AM, Stephen Colebourne wrote:


On 17 June 2013 12:40, Alan Bateman  wrote:

On 17/06/2013 11:05, Nick Williams wrote:

Thanks. I have filed two different bugs for these two different problems.

Here they are:

8016743: java.text.MessageFormat does not support java.time.* types
8016742: JAXB does not support java.time.* types

Note that JAXB is maintained in an upstream project (they periodically do
source drops into OpenJDK). I don't know if support for java.time requires
API changes or not but just to mention that JAXB is tied to a standalone JSR
and I think they the requirement to be able to drop-in into jdk7 builds.

That could be a problem, but its really a process one. It shouldn't be
users that suffer as a result of issues like that (but there isn't
anything I can do about it other than wave my hands...)
Stephen

^^ What Stephen said. :-)

Re: RFR JDK-8028397: Undo the lenient MIME BASE64 decoder support change (JDK-8025003) and remove methods de/encode(buf, buf)

[Xueming Shen]

Hi,

Based on the feedback so far here is the latest webrev, and the link to the bug.

http://cr.openjdk.java.net/~sherman/8028397/webrev
https://bugs.openjdk.java.net/browse/JDK-8028397

During the discussion it became clearthat methods decode/encode(ByteBuffer,
ByteBuffer) are insufficient to support the continuous de/encoding
operation request ("underflow" handling, in charset coder's term).
Given the assumed usage for these methods is for de/encoding the bytes
from io channels, I have been convinced it might be more useful/desired
to provide a pair of wrap methods instead for readable/writableByteChannel,
same as the pair we have now for in/output stream (the wrap methods
are under development, but will probably be for future releases). For
jdk8 the proposal is to remove the pair of de/encode(Buffer, ByteBuffer),
as showed in the webrev.

Thanks!
-Sherman

On 11/14/2013 12:59 PM, Xueming Shen wrote:

Hi,

The consensus/conclusion from the discussion [1]  regarding the changes we
made earlier for JDK-8025003 [2] is to back out the existing "half-lenient" 
change
to leave the door open for a clean "lenient" support for future releases.

Here is the webrev for the undo (keep the mime encoder(...) rename change)

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

Thanks!
-Sherman

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-November/023048.html
[2] http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/e7bdb2fcc7bc

Review request 8029417: Minor javadoc cleanup for JDBC 4.2

[Lance Andersen - Oracle]

Hi all

This is a review request for some minor javadoc clarifications for JDBC 4.2 
based on some feedback that I received.

The webrev can be found http://cr.openjdk.java.net/~lancea/8029417/webrev.00

Best
Lance

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

hg: jdk8/tl/jdk: 8028368: There is no description whether or not java.util.ResourceBundle is thread-safe

[naoto . sato]

Changeset: 5b5ee2288306
Author:naoto
Date:  2013-12-02 11:29 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5b5ee2288306

8028368: There is no description whether or not java.util.ResourceBundle is 
thread-safe
Reviewed-by: okutsu

! src/share/classes/java/util/ListResourceBundle.java
! src/share/classes/java/util/PropertyResourceBundle.java
! src/share/classes/java/util/ResourceBundle.java

Re: Initial request for review of JDK-8006572 DoubleStream.sum()/average() implementations that reduce numerical errors

[Mike Duigou]

On Dec 1 2013, at 23:34 , Joe Darcy  wrote:

> Hello,
> 
> I've heard back from my numerical reviewer. Given the signs of the terms, he 
> recommend when two partial sums are combined, the leading word be added in 
> first. I've reflected this change in the revision I'm about to push:
> 
>http://cr.openjdk.java.net/~darcy/8006572.5
> 
> He had some suggestions for further numerical improvements, which I've 
> captured in
> 
>JDK-8029372 Improve combination of partial sums/averages in streams API
>https://bugs.openjdk.java.net/browse/JDK-8029372

Any description of what the improvements would be? The bug doesn't currently 
capture what will be changed/improved.

> 
> Thanks,
> 
> -Joe
> 
> On 11/25/2013 09:57 AM, Mike Duigou wrote:
>> Looks good
>> 
>> Mike
>> 
>> On Nov 22 2013, at 23:06 , Joe Darcy  wrote:
>> 
>>> Hello Mike and Paul,
>>> 
>>> Thanks for reviewing the earlier versions of the change; I've posted an 
>>> update revision for your consideration at
>>> 
>>>http://cr.openjdk.java.net/~darcy/8006572.4/
>>> 
>>> Two numerical aspects of the change are different in this iteration:
>>> 
>>>* when two running compensated sums and combined, the compensation 
>>> portion of the second is now added in first. Since it should have smaller 
>>> magnitude, this is favorable in terms of error analysis
>>> 
>>>* the final sum returned (or used to compute the average) is (sum + 
>>> sumCompensation). I checked one of my references, "Accuracy and Stability 
>>> of Numerical Algorithms" from Nicholas Higham, and that tweak gave a better 
>>> error bound
>>> 
>>> The test code has been updated as you've suggested (other than porting to 
>>> TestNG ;-) and I also added a test case for a bug Paul spotted on an 
>>> earlier review.
>>> 
>>> All the streams tests pass with the new code.
>>> 
>>> Thanks,
>>> 
>>> -Joe
>>> 
>>> On 11/20/2013 11:37 AM, Joe Darcy wrote:
 Hi Mike,
 
 On 11/20/2013 10:31 AM, Mike Duigou wrote:
> - Collectors averagingDouble could use the same @implNote as in 
> DoublePipeline.
> 
> - DoublePipeline implementation could document the usage of the double 
> array indices.
> 
> - @summary in test.
 I'll reflect these in the next iteration.
 
> - I agree with Paul that refactoring as a testNG test would be nice.
 While learning about testNG would be useful, I don't want to take that on 
 right at the moment ;-)
 
> - I wondered at the mechanism for combining compensation values. Do you 
> have a reference which explains the correctness? I thought perhaps 
> directly adding the compensations together before doing compensated 
> addition of the two sums might be better.
 One of the inquiries I have out to my numerical reviewer is how to best 
 combine two compensations.
 
 In the code as written, from the perspective of one compensation, the two 
 doubles in the other other compensation are just two more double values. 
 So I think this is correct, if not ideal.
 
 The compensation portion of one running sum could be of vastly different 
 magnitude than the compensation portion (or the high-order sum bits) of 
 the other sum. Therefore, I believe a direct combination of either (sum1 + 
 sum2) or (comp1 + comp2) would lose the numerical properties of interest 
 here.
 
 Thanks for the feedback,
 
 -Joe
 
> Mike
> 
> 
> On Nov 20 2013, at 00:21 , Joe Darcy  wrote:
> 
>> Hi Paul,
>> 
>> On 11/18/2013 07:38 AM, Paul Sandoz wrote:
>>> Hi Joe,
>>> 
>>> You can use the three arg form of collect for 
>>> DoublePipeline.sum/average impls, which is already used for average:
>>> 
>>> public final OptionalDouble average() {
>>> double[] avg = collect(() -> new double[2],
>>>(ll, i) -> {
>>>ll[0]++;
>>>ll[1] += i;
>>>},
>>>(ll, rr) -> {
>>>ll[0] += rr[0];
>>>ll[1] += rr[1];
>>>});
>>> return avg[0] > 0
>>>? OptionalDouble.of(avg[1] / avg[0])
>>>: OptionalDouble.empty();
>>> }
>>> 
>>> That would be the most expedient way.
>> Thanks for the tip. For the moment, I'm feeling a bit expedient and used 
>> the array-based approach in an iteration of the change:
>> 
>>http://cr.openjdk.java.net/~darcy/8006572.3/
> 

8028564: Concurrent calls to CHM.put can fail to add the key/value to the map

[Paul Sandoz]

Hi,

http://cr.openjdk.java.net/~psandoz/tl/JDK-8028564-concurrent-resize/webrev/

This patch is contributed by Doug Lea and fixes two issues found in 
ConcurrentHashMap:

  1) A problem with concurrent resizes; and

  2) The skipping of elements when traversing through bins that are trees of 
entries.

Both of these issues can result in elements "disappearing" from the map, either 
because an update operation such as put failed, or because a contains operation 
failed to find the entry in the map.

Issue 1) was causing stream tests to fail intermittently on systems with many 
cores (24 to 32) (furthermore the CHM-based JDK test ToArray was also 
intermittently failing with less frequency). 

After some investigation the cause was distilled down to the use of 
ConcurrentHashMap in the F/J task used by Stream.forEachOrdered. 

Issue 2) was serendipitously reported on concurrentcy-interest just yesterday 
:-)

Both issues have test cases associated with them that have been tuned to 
reproduce on systems with low cores i.e. my MacBook!

Paul.

hg: jdk8/tl/jdk: 8025211: Intermittent test failure: java/net/DatagramSocket/PortUnreachable.java

[mark . sheppard]

Changeset: 39b3b0e77af5
Author:msheppar
Date:  2013-12-02 14:01 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/39b3b0e77af5

8025211: Intermittent test failure: java/net/DatagramSocket/PortUnreachable.java
Summary: modified test to execute in a single thread to eliminate potential 
race condition
Reviewed-by: alanb, chegar, dfuchs

! test/java/net/DatagramSocket/PortUnreachable.java

hg: jdk8/tl/jdk: 8029369: Shell tests in sun/security/pkcs11/ do not compile PKCS11Test

[vincent . x . ryan]

Changeset: 4ca1027a130a
Author:vinnie
Date:  2013-12-02 14:19 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4ca1027a130a

8029369: Shell tests in sun/security/pkcs11/ do not compile PKCS11Test
Reviewed-by: mullan

! test/sun/security/pkcs11/KeyStore/Basic.sh
! test/sun/security/pkcs11/KeyStore/ClientAuth.sh
! test/sun/security/pkcs11/KeyStore/Solaris.sh
! test/sun/security/pkcs11/Provider/ConfigQuotedString.sh
! test/sun/security/pkcs11/Provider/Login.sh

Re: RFR: JDK-8025211 - Intermittent test failure: java/net/DatagramSocket/PortUnreachable.java

[Chris Hegarty]

Looks good to me Mark.

-Chris.

On 02/12/13 12:35, Mark Sheppard wrote:

Hi
based on feedback the changes for issue:
https://bugs.openjdk.java.net/browse/JDK-8025211

have been amended to the following:
http://cr.openjdk.java.net/~msheppar/8025211/webrev.02/

please oblige and review

regards
Mark

On 29/11/2013 17:17, Mark Sheppard wrote:


Alan, Daniel,
   thanks for the replies.

yes, 'tis a possibility, and it appears to work ok as a test, as it
avoids the sender's Thread.sleep creating a race condition, such that
the send doesn't happen prior to the timeout of the client receive.

The context of the test appears to be creating conditions
such that an ICMP  PortUnreachable event is generated,
which caused a close a particular version of windows.
The test is for socket closed scenario, afaik

tested on the errant test machine with consistent success.

regards
Mark

On 29/11/2013 16:41, Daniel Fuchs wrote:

On 11/29/13 5:19 PM, Alan Bateman wrote:

On 29/11/2013 14:21, Mark Sheppard wrote:

Hi
please oblige and review the following changes

http://cr.openjdk.java.net/~msheppar/8025211/webrev/
which address the issue raised in the bug
https://bugs.openjdk.java.net/browse/JDK-8025211

an intermittent failure occurs on some windows test machines.

this replaces a Thread.sleep(5000) with explicit synchronization
between sender
and receiver via a CountDownLatch

(cc'ing net-dev)

Would an alternative here be to just get rid of the server thread (do
both the send + receive on the main thread)?


Hmm... The message sent by the server is short enough so that it
should be stored in the client's socket buffer and received later
by the client, even though the client is not yet blocked waiting
in clientSock.receive() when the message is sent.

That might work.

Unless multi-threading was relevant to the configuration being
tested?


-- daniel



-Alan

Re: RFR: JDK-8025211 - Intermittent test failure: java/net/DatagramSocket/PortUnreachable.java

[Daniel Fuchs]

Hi Mark,

This is much simpler indeed.
Thumbs up from me :-)

best regards,

-- daniel

On 12/2/13 1:35 PM, Mark Sheppard wrote:

Hi
based on feedback the changes for issue:
https://bugs.openjdk.java.net/browse/JDK-8025211

have been amended to the following:
http://cr.openjdk.java.net/~msheppar/8025211/webrev.02/

please oblige and review

regards
Mark

On 29/11/2013 17:17, Mark Sheppard wrote:


Alan, Daniel,
   thanks for the replies.

yes, 'tis a possibility, and it appears to work ok as a test, as it
avoids the sender's Thread.sleep creating a race condition, such that
the send doesn't happen prior to the timeout of the client receive.

The context of the test appears to be creating conditions
such that an ICMP  PortUnreachable event is generated,
which caused a close a particular version of windows.
The test is for socket closed scenario, afaik

tested on the errant test machine with consistent success.

regards
Mark

On 29/11/2013 16:41, Daniel Fuchs wrote:

On 11/29/13 5:19 PM, Alan Bateman wrote:

On 29/11/2013 14:21, Mark Sheppard wrote:

Hi
please oblige and review the following changes

http://cr.openjdk.java.net/~msheppar/8025211/webrev/
which address the issue raised in the bug
https://bugs.openjdk.java.net/browse/JDK-8025211

an intermittent failure occurs on some windows test machines.

this replaces a Thread.sleep(5000) with explicit synchronization
between sender
and receiver via a CountDownLatch

(cc'ing net-dev)

Would an alternative here be to just get rid of the server thread (do
both the send + receive on the main thread)?


Hmm... The message sent by the server is short enough so that it
should be stored in the client's socket buffer and received later
by the client, even though the client is not yet blocked waiting
in clientSock.receive() when the message is sent.

That might work.

Unless multi-threading was relevant to the configuration being
tested?


-- daniel



-Alan

Re: RFR: JDK-8025211 - Intermittent test failure: java/net/DatagramSocket/PortUnreachable.java

[Mark Sheppard]

Hi
   based on feedback the changes for issue:
https://bugs.openjdk.java.net/browse/JDK-8025211

have been amended to the following:
http://cr.openjdk.java.net/~msheppar/8025211/webrev.02/

please oblige and review

regards
Mark

On 29/11/2013 17:17, Mark Sheppard wrote:


Alan, Daniel,
   thanks for the replies.

yes, 'tis a possibility, and it appears to work ok as a test, as it
avoids the sender's Thread.sleep creating a race condition, such that
the send doesn't happen prior to the timeout of the client receive.

The context of the test appears to be creating conditions
such that an ICMP  PortUnreachable event is generated,
which caused a close a particular version of windows.
The test is for socket closed scenario, afaik

tested on the errant test machine with consistent success.

regards
Mark

On 29/11/2013 16:41, Daniel Fuchs wrote:

On 11/29/13 5:19 PM, Alan Bateman wrote:

On 29/11/2013 14:21, Mark Sheppard wrote:

Hi
please oblige and review the following changes

http://cr.openjdk.java.net/~msheppar/8025211/webrev/
which address the issue raised in the bug
https://bugs.openjdk.java.net/browse/JDK-8025211

an intermittent failure occurs on some windows test machines.

this replaces a Thread.sleep(5000) with explicit synchronization
between sender
and receiver via a CountDownLatch

(cc'ing net-dev)

Would an alternative here be to just get rid of the server thread (do
both the send + receive on the main thread)?


Hmm... The message sent by the server is short enough so that it
should be stored in the client's socket buffer and received later
by the client, even though the client is not yet blocked waiting
in clientSock.receive() when the message is sent.

That might work.

Unless multi-threading was relevant to the configuration being
tested?


-- daniel



-Alan