Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently

2014-01-20 Thread Peter Levart

On 01/20/2014 02:51 AM, David Holmes wrote:

Hi Peter,

On 17/01/2014 11:24 PM, Peter Levart wrote:

On 01/17/2014 02:13 PM, Peter Levart wrote:

// Fast path for cleaners
boolean isCleaner = false;
try {
  isCleaner = r instanceof Cleaner;
} catch (OutofMemoryError oome) {
  continue;
}

if (isCleaner) {
  ((Cleaner)r).clean();
  continue;
}



Hi David, Kalyan,

I've caught-up now. Just thinking: is instanceof Cleaner throwing
OOME as a result of loading the Cleaner class? Wouldn't the above
code then throw some error also in ((Cleaner)r) - the checkcast,
since Cleaner class would not be successfully initialized?


Well, no. The above code would just skip Cleaner processing in this
situation. And will never be doing it again after the heap is freed...
So it might be good to load and initialize Cleaner class as part of
ReferenceHandler initialization to ensure correct operation...


Well, yes and no. Let me try once more:

Above code will skip Cleaner processing if the 1st time instanceof
Cleaner is executed, OOME is thrown as a consequence of full heap while
loading and initializing the Cleaner class.


Yes - I was assuming that this would not fail the very first time and 
so the Cleaner class would already be loaded. Failing to be able to 
load the Cleaner class was one of the potential issues flagged earlier 
with this problem. I was actually assuming that Cleaner would be 
loaded already due to some actual Cleaner subclasses being used, but 
this does not happen as part of the default initialization. :( The 
irony being that if the Cleaner class is not loaded then r can not be 
an instance of Cleaner and so we would fail to load the class in a 
case where we didn't need it anyway.


What I wanted to focus on here was an OOME from the instanceof itself, 
but as you say that might trigger classloading of Cleaner (which is 
not what I was interested in).



The 2nd time the instanceof
Cleaner is executed after such OOME, the same line would throw
NoClassDefFoundError as a consequence of referencing a class that failed
initialization. Am I right?


instanceof is not one of the class initialization triggers, so we 
should not see an OOME generated due to a class initialization 
exception and so the class will not be put into the Erroneous state 
and so subsequent attempts to use the class will not automatically 
trigger NoClassdefFoundError.


If OOME occurs during actual loading/linking of the class Cleaner it 
is unclear what would happen on subsequent attempts. OOME is not a 
LinkageError that must be rethrown on subsequent attempts, and it is 
potentially a transient condition, so I would expect a re-load attempt 
to be allowed. However we are now deep into the details of the VM and 
it may well depend on the exact place from which the OOME originates.


The bottom line with the current problem is that there are multiple 
non-obvious paths by which the ReferenceHandler can encounter an OOME. 
In such cases we do not want the ReferenceHandler to terminate - which 
implies catching the OOME and continuing. However we also do not want 
to silently skip Cleaner processing or reference queue processing - as 
that would lead to hard to diagnoze bugs. But trying to report the 
problem may not be possible due to being out-of-memory. It may be that 
we need to break things up into multiple try/catch blocks, where each 
catch does a System.gc() and then reports that the OOME occurred. Of 
course the reporting must still be in a try/catch for the OOME. Though 
at some point letting the ReferenceHandler die may be the only way to 
report a major memory problem.


David


Hm... If I give -verbose:class option to run a simple test program:

public class Test { public static void main(String... a) {} }

I see Cleaner class being loaded before Test class. I don't see by which 
tread or if it might get loaded after main() starts, but I suspect that 
loading of Cleaner is not a problem here. Initialization of Cleaner 
class is not performed by ReferenceHandler thread as you pointed out. 
The instanceof does not trigger it and if it returns true then Cleaner 
has already been initialized. So there must be some other cause for 
instanceof throwing OOME...


What do you say about this variant of ReferenceHandler.run() method:

public void run() {
for (;;) {
Reference r;
Cleaner c;
synchronized (lock) {
r = pending;
if (r != null) {
// instanceof operator might throw OOME 
sometimes. Just retry after

// yielding - might have better luck next time...
try {
c = r instanceof Cleaner ? (Cleaner) r : null;
} catch (OutOfMemoryError x) {
Thread.yield();
continue;
}
pending = r.discovered;
 

Re: Review request for JDK-6760902: inconsistent behavior in bootstrap class loader for classes and resources

2014-01-20 Thread Paul Sandoz

On Jan 17, 2014, at 10:10 PM, Mandy Chung mandy.ch...@oracle.com wrote:

 Paul,
 
 Here is the updated webrev:
 http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6760902/webrev.01/
 
 This cleans up GetResource.sh and merges with GetResource2.sh.  Also fixed 
 if (pos - lastPoc  0) line and javadoc typo.
 

Looks good.

--

In GetResource.sh:
  89 GetResource $expected || exit 10
Should it be exit $count ?, i suppose it does not matter. Regardless no need 
for another review round for that.

Paul.


Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-20 Thread Volker Simonis
On Fri, Jan 17, 2014 at 10:15 PM, Volker Simonis
volker.simo...@gmail.com wrote:
 On Tue, Jan 14, 2014 at 10:19 AM, Alan Bateman alan.bate...@oracle.com 
 wrote:
 On 14/01/2014 08:40, Volker Simonis wrote:

 Hi,

 could you please review the following changes for the ppc-aix-port
 stage/stage-9 repositories (the changes are planned for integration into
 ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):

 I'd like to review this but I won't have time until later in the week. From
 an initial look then there are a few things are not pretty (the changes to
 fix the AIX problems with I/O cancellation in particular) and I suspect that
 some refactoring is going to be required to handle some of this cleanly. A
 minor comment is that bug synopsis doesn't really communicate what these
 changes are about.

 -Alan.

 Just forwarded the following message from another thread here where it 
 belongs:

 On 17/01/2014 16:57, Alan Bateman wrote:

 I've finally got to this one. As the event translation issue is now a
 separate issue then I've ignored that part.

 I'm not comfortable with the changes to FileDispatcherImpl.c as I
 don't think we shouldn't be calling into IO_ or NET_* functions here.
 I think I get the issue that you have on AIX (and assume it's the
 preClose/dup2 that blocks rather than close) but need a bit of time to
 suggest alternatives. It may be that it will require an AIX specific
 SocketDispatcher. Do you happen to know which tests fail due to this
 part?

 The other changes look okay. There is a typo in the change to
 zip_util.c, s/legel/legal/.

 In DatagramChannelImpl.c then you handle connect failing with
 EAFNOSUPPORT. I would be tempted to replace the comment to say that it
 EAFNOSUPPORT can be ignored on AIX. A minor comment but the
 indentation for rv = errno can be fixed (I see the BSD code has it
 wrong too).

 On 17/01/2014 21:23, Volker Simonis wrote:

  You're right, one race is with preClose/dup2 but also with other calls
  like read/fcntl/...
 
  There were several tests that failed and once I fixed it they all
  succeeded. But I can recreate some of the failures for you. The
  symptoms are always the same: the VMis locked. If you trigger a stack
  trace you can see that at least on thread is blocked in a I/O
  operation on a file descriptor like fcntl (e.g. for file locking),
  read, etc. while another thread is trying to close that socket.
 

 As it happens, we have some carry over issues from the Mac port,
 one of which is that async close of FileChannels will block
 indefinitely in dup2 when there is another thread blocked (on
 fnctl or reading from a pipe ...). I haven't time time to work on
 it but this discussion has reminded me that we need to sort it
 out. I've put a preliminary webrev with the changes here:

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

 The important part is that it's using signal consistently on
 Linux/Solaris/OSX so that any blocked threads are interrupted. My
 guess is that if NativeThread.c is updated to define a signal on
 AIX they this should resolve some of the issues on AIX.

 I would like to see the list of tests failing. If there is an
 issue with dup2 with sockets (and OS X doesn't seem to have that
 issue) then it will require further work but I would at least
 like to start by understanding if this patch will help with the
 FileChannel issues.

Hi Alan,

yes, that's interesting. Sounds like a very similar problem on Mac.

I would suggest the following:

I cut out the Async Close AIX FIX stuff from this change (i.e.
8031581: PPC64: Addons and fixes for AIX to pass the jdk regression
tests and send out a new webrev for the remaining part. I think that
the remaining part was more or less reviewed and we can then push it
faster.

In the mean time, I'll recheck which tests exactly fail with my
missing Async Close AIX FIX stuff and which of these tests will be
fixed by your 7133499 webrev. Maybe we can really get trough with it
or with it and a few enhancements. I'll let you know my results later
today. By the way, my webrev already contained a AixNativeThread.c
implementation in src/aix/native/sun/nio/ch.

The only remaining problem I see with this approach is that we would
need to downport your 7133499 change to 8u-dev in the 8u20 time frame
to make our AIX port work. Would this be OK for you?

Regards,
Volker


Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-20 Thread Alan Bateman

On 20/01/2014 09:59, Volker Simonis wrote:

:
Hi Alan,

yes, that's interesting. Sounds like a very similar problem on Mac.

I would suggest the following:

I cut out the Async Close AIX FIX stuff from this change (i.e.
8031581: PPC64: Addons and fixes for AIX to pass the jdk regression
tests and send out a new webrev for the remaining part. I think that
the remaining part was more or less reviewed and we can then push it
faster.

In the mean time, I'll recheck which tests exactly fail with my
missing Async Close AIX FIX stuff and which of these tests will be
fixed by your 7133499 webrev. Maybe we can really get trough with it
or with it and a few enhancements. I'll let you know my results later
today. By the way, my webrev already contained a AixNativeThread.c
implementation in src/aix/native/sun/nio/ch.

The only remaining problem I see with this approach is that we would
need to downport your 7133499 change to 8u-dev in the 8u20 time frame
to make our AIX port work. Would this be OK for you?

I'm okay with this plan and if you re-generate the webrev without the 
async close changes then I can look at it quickly so that you can get it 
into the stage-9 forest.


On 7133499 then it would be a good candidate for 8u-dev too, I don't 
expect any problems but we will need to get it approved on the jdk8u-dev 
list.


-Alan.


Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-20 Thread Volker Simonis
On Mon, Jan 20, 2014 at 12:41 PM, Alan Bateman alan.bate...@oracle.com wrote:
 On 20/01/2014 09:59, Volker Simonis wrote:

 :
 Hi Alan,

 yes, that's interesting. Sounds like a very similar problem on Mac.

 I would suggest the following:

 I cut out the Async Close AIX FIX stuff from this change (i.e.
 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression
 tests and send out a new webrev for the remaining part. I think that
 the remaining part was more or less reviewed and we can then push it
 faster.

 In the mean time, I'll recheck which tests exactly fail with my
 missing Async Close AIX FIX stuff and which of these tests will be
 fixed by your 7133499 webrev. Maybe we can really get trough with it
 or with it and a few enhancements. I'll let you know my results later
 today. By the way, my webrev already contained a AixNativeThread.c
 implementation in src/aix/native/sun/nio/ch.

 The only remaining problem I see with this approach is that we would
 need to downport your 7133499 change to 8u-dev in the 8u20 time frame
 to make our AIX port work. Would this be OK for you?

 I'm okay with this plan and if you re-generate the webrev without the async
 close changes then I can look at it quickly so that you can get it into the
 stage-9 forest.

 On 7133499 then it would be a good candidate for 8u-dev too, I don't expect
 any problems but we will need to get it approved on the jdk8u-dev list.

 -Alan.

Hi everybody,

so here's the second version of this webrev:

http://cr.openjdk.java.net/~simonis/webrevs/8031581_2/

The main changes compared to the first webrew are as follows:

 - the POLL-constants related stuff has been factored out into its own
webrev (8031997: PPC64: Make the various POLL constants system
dependant - https://bugs.openjdk.java.net/browse/JDK-8031997).
 - the Async close on AIX workarounds have been taken out as well
and will be handled separately (probably together with Alans fix for
http://cr.openjdk.java.net/~alanb/7133499/webrev/).

 - in the remaining files I've applied the changes suggested by
Staffan, so I think the changes to the following files can be
considered as reviewed:

src/share/native/sun/management/DiagnosticCommandImpl.c
src/solaris/native/sun/management/OperatingSystemImpl.c
src/share/transport/socket/socketTransport.c
src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider

 - I've added the following additional files to the change:

src/aix/classes/sun/nio/ch/sctp/SctpChannelImpl.java
src/aix/classes/sun/nio/ch/sctp/SctpMultiChannelImpl.java
src/aix/classes/sun/nio/ch/sctp/SctpServerChannelImpl.java

which are just empty stub implementations of the SCTP classes needed
to pass the SCTP jtreg tests.

All other changes should be the same like in the first review round.

Thanks,
Volker


Re: RFR: 8032190 It's unclear that flatMap will ensure each stream will be closed.

2014-01-20 Thread Chris Hegarty

It is good to clarify that the streams are closed.

I find the following updated wording a little odd, If a mapped stream 
is {@code null} then it treated as if it was an empty stream. I thought 
the previous wording was better, but that could be just me.


-Chris.

On 20/01/14 10:38, Paul Sandoz wrote:

Hi,

For the flatMap operations of streams we forgot to say what it does with the mapped 
streams after it has processed them i.e. closes them, which is important for I/O backed 
streams (e.g. map Path - StreamString for lines of a file). The following 
patch fixes that omission in the docs:

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

I think this should be backported to 8.

Paul.

diff -r 9bf43f25eacd src/share/classes/java/util/stream/DoubleStream.java
--- a/src/share/classes/java/util/stream/DoubleStream.java  Sat Jan 18 
10:57:41 2014 -0800
+++ b/src/share/classes/java/util/stream/DoubleStream.java  Mon Jan 20 
11:34:17 2014 +0100
@@ -150,10 +150,11 @@

  /**
   * Returns a stream consisting of the results of replacing each element of
- * this stream with the contents of the stream produced by applying the
- * provided mapping function to each element.  (If the result of the 
mapping
- * function is {@code null}, this is treated as if the result was an empty
- * stream.)
+ * this stream with the contents of a mapped stream produced by applying
+ * the provided mapping function to each element.  Each mapped stream is
+ * closed (see {@link java.util.stream.BaseStream#close()}) after its
+ * contents have been placed into this stream.  (If a mapped stream is
+ * {@code null} then it treated as if it was an empty stream.)
   *
   * pThis is an a href=package-summary.html#StreamOpsintermediate
   * operation/a.
diff -r 9bf43f25eacd src/share/classes/java/util/stream/IntStream.java
--- a/src/share/classes/java/util/stream/IntStream.java Sat Jan 18 10:57:41 
2014 -0800
+++ b/src/share/classes/java/util/stream/IntStream.java Mon Jan 20 11:34:17 
2014 +0100
@@ -146,10 +146,11 @@

  /**
   * Returns a stream consisting of the results of replacing each element of
- * this stream with the contents of the stream produced by applying the
- * provided mapping function to each element.  (If the result of the 
mapping
- * function is {@code null}, this is treated as if the result was an empty
- * stream.)
+ * this stream with the contents of a mapped stream produced by applying
+ * the provided mapping function to each element.  Each mapped stream is
+ * closed (see {@link java.util.stream.BaseStream#close()}) after its
+ * contents have been placed into this stream.  (If a mapped stream is
+ * {@code null} then it treated as if it was an empty stream.)
   *
   * pThis is an a href=package-summary.html#StreamOpsintermediate
   * operation/a.
diff -r 9bf43f25eacd src/share/classes/java/util/stream/LongStream.java
--- a/src/share/classes/java/util/stream/LongStream.javaSat Jan 18 
10:57:41 2014 -0800
+++ b/src/share/classes/java/util/stream/LongStream.javaMon Jan 20 
11:34:17 2014 +0100
@@ -151,10 +151,11 @@

  /**
   * Returns a stream consisting of the results of replacing each element of
- * this stream with the contents of the stream produced by applying the
- * provided mapping function to each element.  (If the result of the 
mapping
- * function is {@code null}, this is treated as if the result was an empty
- * stream.)
+ * this stream with the contents of a mapped stream produced by applying
+ * the provided mapping function to each element.  Each mapped stream is
+ * closed (see {@link java.util.stream.BaseStream#close()}) after its
+ * contents have been placed into this stream.  (If a mapped stream is
+ * {@code null} then it treated as if it was an empty stream.)
   *
   * pThis is an a href=package-summary.html#StreamOpsintermediate
   * operation/a.
diff -r 9bf43f25eacd src/share/classes/java/util/stream/Stream.java
--- a/src/share/classes/java/util/stream/Stream.javaSat Jan 18 10:57:41 
2014 -0800
+++ b/src/share/classes/java/util/stream/Stream.javaMon Jan 20 11:34:17 
2014 +0100
@@ -227,10 +227,11 @@

  /**
   * Returns a stream consisting of the results of replacing each element of
- * this stream with the contents of the stream produced by applying the
- * provided mapping function to each element.  (If the result of the 
mapping
- * function is {@code null}, this is treated as if the result was an empty
- * stream.)
+ * this stream with the contents of a mapped stream produced by applying
+ * the provided mapping function to each element.  Each mapped stream is
+ * closed (see {@link java.util.stream.BaseStream#close()}) after its
+ * contents have been placed into this stream.  (If a mapped stream is
+ * {@code null} then it treated as if it was an 

Re: RFR [8027348] (process) Enhancement of handling async close of ProcessInputStream

2014-01-20 Thread Ivan Gerasimov

Hello everybody!

Can I consider this change approved for jdk9?

Thanks in advance,
Ivan Gerasimov

On 20.11.2013 0:56, Martin Buchholz wrote:

Ivan,
Thanks for taking ownership of my proposed change.  Looks good to me!

(I can't actually push changes like this myself because I only do 
Linux these days)



On Tue, Nov 19, 2013 at 12:42 PM, Ivan Gerasimov 
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com wrote:


Hello all!

May I have a review for the improvement contributed by Martin
Buchholz?
https://bugs.openjdk.java.net/browse/JDK-8027348

First, it the change performs the code cleanup, and second it
makes the test much faster.
This should also help to resolve the issue with the current
version of the test, which was reported to fail intermittently by
timeout.
https://bugs.openjdk.java.net/browse/JDK-8028574

Here's the webrev with the change:
http://cr.openjdk.java.net/~igerasim/8027348/0/webrev/
http://cr.openjdk.java.net/%7Eigerasim/8027348/0/webrev/

Sincerely yours,
Ivan Gerasimov






Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-20 Thread Alan Bateman

On 20/01/2014 13:45, Volker Simonis wrote:

:
Hi everybody,

so here's the second version of this webrev:

http://cr.openjdk.java.net/~simonis/webrevs/8031581_2/

This looks okay to me.

The typo (legel - legal) still exists in zip_util.c and maybe that 
can be fixed before you push this (no need to generate a few webrev of 
course).


For the JDWP socket transport then it's interesting that shutdown is 
being used to cause the reader thread to be preempted. That may be 
useful when it comes to addressing the bigger async close issue.




The main changes compared to the first webrew are as follows:

  - the POLL-constants related stuff has been factored out into its own
webrev (8031997: PPC64: Make the various POLL constants system
dependant - https://bugs.openjdk.java.net/browse/JDK-8031997).
I see this has been pushed to ppc-aix-port/stage-9. Would you have any 
objection if I brought this into jdk9/dev (minus the AixPollPort 
change)? We can use a different bug number so as not to cause duplicate 
bug issues. It should trivially merge when you come to sync'ing up the 
staging forest.




  - the Async close on AIX workarounds have been taken out as well
and will be handled separately
Thanks for separating this one out as I suspect this that doing this 
cleanly is going to involve changes for all platforms.


-Alan.


Re: RFR: 8032190 It's unclear that flatMap will ensure each stream will be closed.

2014-01-20 Thread Paul Sandoz

On Jan 20, 2014, at 3:18 PM, Chris Hegarty chris.hega...@oracle.com wrote:

 It is good to clarify that the streams are closed.
 
 I find the following updated wording a little odd, If a mapped stream is 
 {@code null} then it treated as if it was an empty stream. I thought the 
 previous wording was better, but that could be just me.
 

I was hopping to use the term mapped stream to avoid some repetition for the 
result of the mapping function etc, but wording seems a little garbled.

How about:

  If a mapped stream is {@code null} an empty stream is used, instead. 

Paul.






Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-20 Thread Volker Simonis
On Mon, Jan 20, 2014 at 4:24 PM, Alan Bateman alan.bate...@oracle.com wrote:
 On 20/01/2014 13:45, Volker Simonis wrote:

 :
 Hi everybody,

 so here's the second version of this webrev:

 http://cr.openjdk.java.net/~simonis/webrevs/8031581_2/

 This looks okay to me.

Thanks.


 The typo (legel - legal) still exists in zip_util.c and maybe that can
 be fixed before you push this (no need to generate a few webrev of course).


Sorry, I've just fixed it in my patch queue and will used the fixed
version for pushing.

@Vladimir: could you please run this change
(http://cr.openjdk.java.net/~simonis/webrevs/8031581_2) through JPRT
as well. I'll push it (together with the fixed typo in the comment) if
everything is OK.

 For the JDWP socket transport then it's interesting that shutdown is being
 used to cause the reader thread to be preempted. That may be useful when it
 comes to addressing the bigger async close issue.



 The main changes compared to the first webrew are as follows:

   - the POLL-constants related stuff has been factored out into its own
 webrev (8031997: PPC64: Make the various POLL constants system
 dependant - https://bugs.openjdk.java.net/browse/JDK-8031997).

 I see this has been pushed to ppc-aix-port/stage-9. Would you have any
 objection if I brought this into jdk9/dev (minus the AixPollPort change)? We
 can use a different bug number so as not to cause duplicate bug issues. It
 should trivially merge when you come to sync'ing up the staging forest.


I have no objections of course. I'm just not sure what exact
implications this will have.

@Vladimir: what do you think - can Alan push 8031997: PPC64: Make the
various POLL constants system dependant minus the Aix-specific stuff
to jdk9/dev now, without causing you any harm during integration.

@Alan: on the other hand, the bulk integration from
ppc-aix-port/stage-9 to jdk9/dev is planned for next week anyway, so
maybe you could wait until that happens?

Thanks,
Volker


   - the Async close on AIX workarounds have been taken out as well
 and will be handled separately

 Thanks for separating this one out as I suspect this that doing this cleanly
 is going to involve changes for all platforms.

 -Alan.


Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-20 Thread Alan Bateman

On 20/01/2014 16:29, Volker Simonis wrote:

:

@Alan: on the other hand, the bulk integration from
ppc-aix-port/stage-9 to jdk9/dev is planned for next week anyway, so
maybe you could wait until that happens?

In that case then ignore my request, I assumed it would not be pushed to 
jdk9/dev until end-Feb.


-Alan


Re: RFR: JDK-8032012, , String.toLowerCase/toUpperCase performance improvement

2014-01-20 Thread Paul Sandoz
On Jan 16, 2014, at 7:08 PM, Xueming Shen xueming.s...@oracle.com wrote:
 Hi,
 
 The proposed changeset is to improve the performance (both speed and memory
 usage) of String.toLowerCase/toUpperCase, by
 
 (1) to separate the most likely use scenario (non supplementary character, 
 no special
 case mapping handling) into simple/quick iteration loop code path
 (2) to use the package private constructor String(char[], boolean) to avoid 
 unnecessary
 target char[] copy in fast-path case.
 (3) some tiny code cleanup.
 
 Since it's supposed to be a simple/quick round of code cleanup, I did not go 
 too far to
 optimize the supplementary/special mapping code.
 
 The webrev is
 
 http://cr.openjdk.java.net/~sherman/8032012/
 

Some quick comments.

In String.toLowerCase:

- it would be nice to get rid of the pseudo goto using the scan labelled 
block.

- there is no need for the localeDependent local variable.

- you might be able to optimize by doing (could depend on the answer to the 
next point):

 int c = (int)value[i];
 int lc = Character.toLowerCase(c);
 if (.) { result[i] = (char)lc; } else { return toLowerCaseEx(result, i, 
locale, localeDependent); }

- Do you need to check ERROR for the result of toLowerCase?

2586 if (c == Character.ERROR ||

Paul.


Re: JDK 9 RFR for 8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281

2014-01-20 Thread Mandy Chung

On 1/17/2014 6:51 AM, Daniel Fuchs wrote:

Hi,

Please find below a webrev for:

8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281
https://bugs.openjdk.java.net/browse/JDK-8031980

http://cr.openjdk.java.net/~dfuchs/webrev_8031980/webrev.00/


The new test looks reasonable.foo.bar5.level = INFO is commented 
out in deadlock.conf.properties.  Is this intentional? The test runs in 
the same VM first without security manager and then with security 
manager.   The loggers have been created and may/may not be GC'ed.  I 
wonder if it's worth breaking them into two separate runs so that each 
run starts with no logger. Have you also tried running this with 
-Xcomp and see if any intermittent failure due to GC?   It looks fine 
when reviewing the test but better to verify it.


Mandy


Re: JDK 9 RFR for 8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281

2014-01-20 Thread Daniel Fuchs

On 1/20/14 7:17 PM, Mandy Chung wrote:

On 1/17/2014 6:51 AM, Daniel Fuchs wrote:

Hi,

Please find below a webrev for:

8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281
https://bugs.openjdk.java.net/browse/JDK-8031980

http://cr.openjdk.java.net/~dfuchs/webrev_8031980/webrev.00/


The new test looks reasonable.foo.bar5.level = INFO is commented
out in deadlock.conf.properties.  Is this intentional?


Yes - I still wanted to have a case where a logger wouldn't
have a parent in the configuration file - just in case
(although that should already be tested by the 'old' test).


The test runs in
the same VM first without security manager and then with security
manager.   The loggers have been created and may/may not be GC'ed.  I
wonder if it's worth breaking them into two separate runs so that each
run starts with no logger.


hmm. good point. I don't think it really needs to though.
Most of the time the deadlock will happen with with no security, but
sometimes you have to wait for the with security before it does
happen. And sometime you have to rerun the test - but that is rare.

IMHO a bit of randomness is not bad for that kind of test.


Have you also tried running this with
-Xcomp and see if any intermittent failure due to GC?   It looks fine
when reviewing the test but better to verify it.


No - haven't tried yet. Will do. However I saw it fail with errors
indicating that loggers which should not have been gc'ed had been
gc'ed when I ran it on a platform that didn't have JDK-8029281,
which means that it's also a good test for the changes in
LoggerWeakRef.dispose() from JDK-8029281.

May I push after verifying that it doesn't fail with -Xcomp?

best regards,

-- daniel



Mandy




Re: JDK 9 RFR for 8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281

2014-01-20 Thread Mandy Chung


On 1/20/2014 10:35 AM, Daniel Fuchs wrote:

On 1/20/14 7:17 PM, Mandy Chung wrote:

On 1/17/2014 6:51 AM, Daniel Fuchs wrote:

Hi,

Please find below a webrev for:

8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281
https://bugs.openjdk.java.net/browse/JDK-8031980

http://cr.openjdk.java.net/~dfuchs/webrev_8031980/webrev.00/


The new test looks reasonable.foo.bar5.level = INFO is commented
out in deadlock.conf.properties.  Is this intentional?


Yes - I still wanted to have a case where a logger wouldn't
have a parent in the configuration file - just in case
(although that should already be tested by the 'old' test).



That's what I suspect too - I suggest adding a comment to make it clear.




Have you also tried running this with
-Xcomp and see if any intermittent failure due to GC?   It looks fine
when reviewing the test but better to verify it.


No - haven't tried yet. Will do. However I saw it fail with errors
indicating that loggers which should not have been gc'ed had been
gc'ed when I ran it on a platform that didn't have JDK-8029281,
which means that it's also a good test for the changes in
LoggerWeakRef.dispose() from JDK-8029281.

May I push after verifying that it doesn't fail with -Xcomp?



Yes and thanks for being proactive to verify that.

Mandy


Re: RFR: 8032190 It's unclear that flatMap will ensure each stream will be closed.

2014-01-20 Thread Chris Hegarty

 On 20 Jan 2014, at 15:18, Paul Sandoz paul.san...@oracle.com wrote:
 
 
 On Jan 20, 2014, at 3:18 PM, Chris Hegarty chris.hega...@oracle.com wrote:
 
 It is good to clarify that the streams are closed.
 
 I find the following updated wording a little odd, If a mapped stream is 
 {@code null} then it treated as if it was an empty stream. I thought the 
 previous wording was better, but that could be just me.
 
 I was hopping to use the term mapped stream to avoid some repetition for 
 the result of the mapping function etc, but wording seems a little garbled.
 
 How about:
 
  If a mapped stream is {@code null} an empty stream is used, instead. 

Thanks Paul. Seems fine to me.

-Chris.


 Paul.
 
 
 
 


Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently

2014-01-20 Thread Peter Levart


On 01/21/2014 03:22 AM, David Holmes wrote:

Hi Peter,

I do not see Cleaner being loaded prior to the main class on either 
Windows or Linux. Which platform are you on? Did you see it loaded 
before the main class or as part of executing it?


Before. The main class is empty:

public class Test { public static void main(String... a) {} }

Here's last few lines of -verbose:class:

[Loaded java.util.TimeZone from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.ZoneInfo from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.ZoneInfoFile from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.ZoneInfoFile$1 from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.io.DataInput from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.io.DataInputStream from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
*[Loaded sun.misc.Cleaner from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]*
[Loaded java.io.ByteArrayInputStream from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.ZoneInfoFile$ZoneOffsetTransitionRule from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.util.zip.Checksum from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.util.zip.CRC32 from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.ZoneInfoFile$Checksum from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.util.TimeZone$1 from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.CalendarDate from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.BaseCalendar$Date from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.Gregorian$Date from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.CalendarUtils from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.util.jar.JarEntry from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.util.jar.JarFile$JarFileEntry from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.util.zip.ZipFile$ZipFileInputStream from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.util.AbstractSequentialList from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.util.LinkedList from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.util.LinkedList$Node from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.security.PrivilegedActionException from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.misc.URLClassPath$FileLoader from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.misc.Resource from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.misc.URLClassPath$FileLoader$1 from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.nio.ByteBuffered from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.security.PermissionCollection from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.security.Permissions from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.net.URLConnection from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.net.www.URLConnection from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.net.www.protocol.file.FileURLConnection from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.net.www.MessageHeader from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.io.FilePermission from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.io.FilePermission$1 from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.io.FilePermissionCollection from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.security.AllPermission from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.security.UnresolvedPermission from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.security.BasicPermissionCollection from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]

*[Loaded Test from file:/tmp/]*
[Loaded sun.launcher.LauncherHelper$FXHelper from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.lang.Shutdown from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.lang.Shutdown$Lock from 
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]



I'm on linux, 64bit and using official EA build 121 of JDK 8...

But if I try with JDK 7u45, I don't see it. So perhaps it would be good 
to trigger Cleaner loading and initialization as part of 
ReferenceHandler initialization to play things safe.




Also, it is not that I think ReferenceHandler is responsible for 
reporting OOME, but that it is responsible for reporting that it was 
unable to perform a clean or enqueue because of OOME.


This would be necessary if we