Re: Review request: 8011380: FX dependency on PlatformLogger broken
HI Mandy, On 04/04/2013 06:52 AM, Mandy Chung wrote: Peter, Laurent, History and details are described below. Webrev at: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8011380/webrev.00/ I add back the getLevel(int), setLevel(int) and isLoggable(int) methods but marked deprecated and also revert the static final variables to resolve the regression. They can be removed when JavaFX transitions to use the Level enums (I'll file one issue for FX to use PlatformLogger.Level and one for core-libs to remove the deprecated methods). The performance overhead is likely small since it's direct mapping from int to the Level enum that was used in one of your previous performance measurement. I think that it is not strictly necessary to restore the PlatformLogger static final fields back to int type. They are compile-time constants and so are already baked into the classes compiled with older version of PlatformLogger. Am I right? The only problem I see if fields are kept as Level type is when JFX is compiled with JDK8 and then run on JDK7... But changing them temporarily back to int is a conservative approach and I support it. Laurent - you have a patch to add isLoggable calls in the awt/swing code. Can you check if there is any noticeable performance difference? I also take the opportunity to reconsider what JavaLoggerProxy.getLevel() should return when it's a custom Level. Use of logging is expected not to cause any fatal error to the running application. The previous patch throwing IAE needs to be fixed. I propose to return the first Level.intValue() = the custom level value since the level value is used to evaluate if it's loggable. That's a good compromise. History and details: JavaFX 8 has converted to use sun.util.logging.PlatformLogger (https://javafx-jira.kenai.com/browse/RT-24458). I was involved in the early discussion but wasn't aware of the decision made. Thanks to Alan for catching this regression out before it's integrated to jdk8. jfxrt.jar is cobundled with jdk8 during the installer build step. My build doesn't build installer and thus we didn't see this regression. I ran jdeps on jdk8/lib/ext/jfxrt.jar (windows-i586) that shows 112 references to PlatformLogger and on jdk7/lib/jfxrt.jar that shows no reference to sun.util.logging. I have a method finder tool (planning to include it in jdk8) to search for use of PlatformLogger.getLevel and it did find 2 references from jdk8/lib/ext/jfxrt.jar. JavaFX 8 is going to upgrade to use JDK 8 for JavaFX build (https://javafx-jira.kenai.com/browse/RT-27794) soon (currently it's built with JDK 7). As soon as JavaFX code are changed to reference PlatformLogger.Level enum instead, we can remove the deprecated methods and change the PlatformLogger constants. What do you think of deprecating the PlatformLogger constants altogether (regardless of their type). The code using them could be migrated to use PlatformLogger.Level enum members directly and if PlatformLogger.Level.INFO looks to verbose, static imports can help or there could be some more helper methods added to PlatformLogger that would minimize the need to access the constants like: isInfoLoggable(); isWarningLoggable(); ... Regards, Peter JavaFX 2.2.x doesn't use sun.util.logging and so this has no impact to it. In any case, JavaFX 2.2.x only runs either bundled with a corresponding JDK 7u release, or as a standalone library for JDK 6 only. Thanks Mandy
Re: Review request: 8011380: FX dependency on PlatformLogger broken
Mandy, Peter, here are my comments: On 04/04/2013 06:52 AM, Mandy Chung wrote: Peter, Laurent, History and details are described below. Webrev at: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8011380/webrev.00/ I add back the getLevel(int), setLevel(int) and isLoggable(int) methods but marked deprecated and also revert the static final variables to resolve the regression. They can be removed when JavaFX transitions to use the Level enums (I'll file one issue for FX to use PlatformLogger.Level and one for core-libs to remove the deprecated methods). The performance overhead is likely small since it's direct mapping from int to the Level enum that was used in one of your previous performance measurement. Look OK for me; the Level valueOf(int level) is back and is basically an efficient switch case that performs well (performance overhead is minor). I hope other projects will be built again soon to remove such workarrounds. I think that it is not strictly necessary to restore the PlatformLogger static final fields back to int type. They are compile-time constants and so are already baked into the classes compiled with older version of PlatformLogger. Am I right? The only problem I see if fields are kept as Level type is when JFX is compiled with JDK8 and then run on JDK7... But changing them temporarily back to int is a conservative approach and I support it. I think you're right: static constants are copied into class bytecode; maybe the old API (int level in method signatures) could be kept and marked deprecated but the class will become quite big (double API: one with int and one with Level) !! Laurent - you have a patch to add isLoggable calls in the awt/swing code. Can you check if there is any noticeable performance difference? The awt patch consists in adding missing isLoggable statements to avoid object creation (string concatenations) and method calls (Component.toString() ...). Maybe I should also check that calls to log(msg, varargs) are using isLoggable() tests to avoid Object[] creation due to varargs: what is your opinion ? I also take the opportunity to reconsider what JavaLoggerProxy.getLevel() should return when it's a custom Level. Use of logging is expected not to cause any fatal error to the running application. The previous patch throwing IAE needs to be fixed. I propose to return the first Level.intValue() = the custom level value since the level value is used to evaluate if it's loggable. That's a good compromise. I think custom logger are ILLEGAL in the PlatformLogger API and must be discarded. Maybe comments should explain such design decision and returning an IllegalStateException is OK for me. History and details: JavaFX 8 has converted to use sun.util.logging.PlatformLogger ( https://javafx-jira.kenai.com/browse/RT-24458). I was involved in the early discussion but wasn't aware of the decision made. Thanks to Alan for catching this regression out before it's integrated to jdk8. jfxrt.jar is cobundled with jdk8 during the installer build step. My build doesn't build installer and thus we didn't see this regression. I ran jdeps on jdk8/lib/ext/jfxrt.jar (windows-i586) that shows 112 references to PlatformLogger and on jdk7/lib/jfxrt.jar that shows no reference to sun.util.logging. I have a method finder tool (planning to include it in jdk8) to search for use of PlatformLogger.getLevel and it did find 2 references from jdk8/lib/ext/jfxrt.jar. Personally I doubt getLevel() method is useful: why not use isLoggable() instead !! maybe getLevel() should become @deprecated too. JavaFX 8 is going to upgrade to use JDK 8 for JavaFX build ( https://javafx-jira.kenai.com/browse/RT-27794) soon (currently it's built with JDK 7). As soon as JavaFX code are changed to reference PlatformLogger.Level enum instead, we can remove the deprecated methods and change the PlatformLogger constants. Great, any idea about their roadmap ? do you think other projects are also depending on PlatformLogger (JMX ...) and may be impacted ? What do you think of deprecating the PlatformLogger constants altogether (regardless of their type). The code using them could be migrated to use PlatformLogger.Level enum members directly and if PlatformLogger.Level.INFO looks to verbose, static imports can help or there could be some more helper methods added to PlatformLogger that would minimize the need to access the constants like: isInfoLoggable(); isWarningLoggable(); ... It starts to mimic log4j / slf4j / logback syntax I love but I think it will change so many files that I think it is a waste of time for now. I vote for adding such useful shortcuts in the new API but not change other methods. Cheers, Laurent
Re: AWT Dev Swing Dev sun.awt.X11 logs still using String + (waste)
Dear all,
I just realized there is another problem with PlatformLogger log statements:
XBaseWindow:
public boolean grabInput() {
grabLog.fine(Grab input on {0}, this);
...
}
This calls the PlatformLogger.fine( varargs):
public void fine(String msg, Object... params) {
logger.doLog(FINE, msg, params);
}
Doing so, the JVM creates a new Object[] instance to provide params as
varargs.
I would recommend using isLoggable() test to avoid such waste if the log is
disabled (fine, finer, finest ...).
Do you want me to provide a new patch related to this problem ?
Does somebody have an idea to automatically analyze the JDK code and detect
missing isLoggable statements ...
Regards,
Laurent
2013/4/3 Laurent Bourgès bourges.laur...@gmail.com
Anthony,
could you tell me once it is in the OpenJDK8 repository ?
I would then perform again profiling tests to check if there is no more
missing isLoggable() statements.
Once JMX and other projects switch to PlatformLogger, I could check again.
Maybe I could write a small java code checker (pmd rule) to test if there
is missing isLoggable() statements wrapping PlatformLogger log statements.
Any idea about how to reuse java parser to do so ?
Regards,
Laurent
2013/4/2 Anthony Petrov anthony.pet...@oracle.com
Looks fine to me as well. Thanks for fixing this, Laurent.
Let's wait a couple more days in case Net or Swing folks want to review
the fix. After that I'll push it to the repository.
--
best regards,
Anthony
On 4/2/2013 15:35, Laurent Bourgès wrote:
Here is the updated patch:
http://jmmc.fr/~bourgesl/**share/webrev-8010297.2/http://jmmc.fr/%7Ebourgesl/share/webrev-8010297.2/
Fixed inconsistencies between FINE / FINER log statements:
- XScrollbarPeer
- XWindowPeer
Laurent
2013/4/2 Anthony Petrov anthony.pet...@oracle.com mailto:
anthony.petrov@oracle.**com anthony.pet...@oracle.com
1. Sergey: I believe this is for purposes of better formating the
log output and making it more readable by separating or highlighting
some sections. I don't think this should be changed.
2. Laurent: can you please address this issue and send us a new
patch?
--
best regards,
Anthony
On 4/1/2013 16:08, Sergey Bylokhov wrote:
Hi, Anthony
Only two comments:
1 Why we need some special text in the log output like *** and
###
2 XScrollbarPeer.java:
+if (log.isLoggable(__**PlatformLogger.FINEST)) {
+log.finer(KeyEvent on scrollbar: + event);
+}
On 4/1/13 12:20 PM, Anthony Petrov wrote:
Awt, Swing, Net engineers,
Could anyone review the fix please? For your convenience:
Bug:
http://bugs.sun.com/view_bug._**_do?bug_id=8010297http://bugs.sun.com/view_bug.__do?bug_id=8010297
http://bugs.sun.com/view_bug.**do?bug_id=8010297http://bugs.sun.com/view_bug.do?bug_id=8010297
Fix:
http://cr.openjdk.java.net/~__**anthony/8-55-isLoggable-__**
8010297.0/http://cr.openjdk.java.net/%7E__anthony/8-55-isLoggable-__8010297.0/
http://cr.openjdk.java.net/%**7Eanthony/8-55-isLoggable-**
8010297.0/http://cr.openjdk.java.net/%7Eanthony/8-55-isLoggable-8010297.0/
-- best regards,
Anthony
On 3/22/2013 2:26, Anthony Petrov wrote:
Hi Laurent,
The fix looks great to me. Thank you very much.
We still need at least one review, though. Hopefully
net-dev@ and/or swing-dev@ folks might help us out a
bit.
-- best regards,
Anthony
On 3/20/2013 15:10, Anthony Petrov wrote:
Hi Laurent,
Thanks for the patch. I've filed a bug at:
http://bugs.sun.com/view_bug._**_do?bug_id=8010297http://bugs.sun.com/view_bug.__do?bug_id=8010297
http://bugs.sun.com/view_bug.**do?bug_id=8010297http://bugs.sun.com/view_bug.do?bug_id=8010297
(should be available in a day or two)
and published a webrev generated from your patch at:
http://cr.openjdk.java.net/~__**
anthony/8-55-isLoggable-__**8010297.0/http://cr.openjdk.java.net/%7E__anthony/8-55-isLoggable-__8010297.0/
http://cr.openjdk.java.net/%**
7Eanthony/8-55-isLoggable-**8010297.0/http://cr.openjdk.java.net/%7Eanthony/8-55-isLoggable-8010297.0/
I'm also copying swing-dev@ and net-dev@ because the
fix affects those areas too. I myself will review
the fix a bit later but am sending it now for other
folks to take a look at it.
On 3/19/2013 15:29, Laurent Bourgès wrote:
I am
Re: AWT Dev Swing Dev sun.awt.X11 logs still using String + (waste)
Yes, this makes sense. Do you want to update your fix for 8010297 to
include these changes as well?
--
best regards,
Anthony
On 04/04/13 15:47, Laurent Bourgès wrote:
Dear all,
I just realized there is another problem with PlatformLogger log statements:
XBaseWindow:
public boolean grabInput() {
grabLog.fine(Grab input on {0}, this);
...
}
This calls the PlatformLogger.fine( varargs):
public void fine(String msg, Object... params) {
logger.doLog(FINE, msg, params);
}
Doing so, the JVM creates a new Object[] instance to provide params as
varargs.
I would recommend using isLoggable() test to avoid such waste if the log
is disabled (fine, finer, finest ...).
Do you want me to provide a new patch related to this problem ?
Does somebody have an idea to automatically analyze the JDK code and
detect missing isLoggable statements ...
Regards,
Laurent
2013/4/3 Laurent Bourgès bourges.laur...@gmail.com
mailto:bourges.laur...@gmail.com
Anthony,
could you tell me once it is in the OpenJDK8 repository ?
I would then perform again profiling tests to check if there is no
more missing isLoggable() statements.
Once JMX and other projects switch to PlatformLogger, I could check
again.
Maybe I could write a small java code checker (pmd rule) to test if
there is missing isLoggable() statements wrapping PlatformLogger log
statements. Any idea about how to reuse java parser to do so ?
Regards,
Laurent
2013/4/2 Anthony Petrov anthony.pet...@oracle.com
mailto:anthony.pet...@oracle.com
Looks fine to me as well. Thanks for fixing this, Laurent.
Let's wait a couple more days in case Net or Swing folks want to
review the fix. After that I'll push it to the repository.
--
best regards,
Anthony
On 4/2/2013 15:35, Laurent Bourgès wrote:
Here is the updated patch:
http://jmmc.fr/~bourgesl/__share/webrev-8010297.2/
http://jmmc.fr/%7Ebourgesl/share/webrev-8010297.2/
Fixed inconsistencies between FINE / FINER log statements:
- XScrollbarPeer
- XWindowPeer
Laurent
2013/4/2 Anthony Petrov anthony.pet...@oracle.com
mailto:anthony.pet...@oracle.com
mailto:anthony.petrov@oracle.__com
mailto:anthony.pet...@oracle.com
1. Sergey: I believe this is for purposes of better
formating the
log output and making it more readable by separating or
highlighting
some sections. I don't think this should be changed.
2. Laurent: can you please address this issue and send
us a new patch?
--
best regards,
Anthony
On 4/1/2013 16:08, Sergey Bylokhov wrote:
Hi, Anthony
Only two comments:
1 Why we need some special text in the log output
like *** and
###
2 XScrollbarPeer.java:
+if
(log.isLoggable(PlatformLogger.FINEST)) {
+log.finer(KeyEvent on scrollbar: +
event);
+}
On 4/1/13 12:20 PM, Anthony Petrov wrote:
Awt, Swing, Net engineers,
Could anyone review the fix please? For your
convenience:
Bug:
http://bugs.sun.com/view_bug.do?bug_id=8010297
http://bugs.sun.com/view_bug.__do?bug_id=8010297
http://bugs.sun.com/view_bug.__do?bug_id=8010297
http://bugs.sun.com/view_bug.do?bug_id=8010297
Fix:
http://cr.openjdk.java.net/~anthony/8-55-isLoggable-8010297.0/
http://cr.openjdk.java.net/%7E__anthony/8-55-isLoggable-__8010297.0/
http://cr.openjdk.java.net/%__7Eanthony/8-55-isLoggable-__8010297.0/
http://cr.openjdk.java.net/%7Eanthony/8-55-isLoggable-8010297.0/
-- best regards,
Anthony
On 3/22/2013 2:26, Anthony Petrov wrote:
Hi Laurent,
The fix looks great to me. Thank you very much.
We still need at least one review, though.
Hopefully
net-dev@ and/or swing-dev@ folks might help
us out a bit.
-- best regards,
Anthony
On 3/20/2013 15:10, Anthony Petrov wrote:
Hi Laurent,
Thanks for the patch. I've filed a bug at:
Re: AWT Dev Swing Dev sun.awt.X11 logs still using String + (waste)
Ok, I can provide an updated patch after finding / fixing all usages.
Probably in 1 or 2 days,
Laurent
2013/4/4 Anthony Petrov anthony.pet...@oracle.com
Yes, this makes sense. Do you want to update your fix for 8010297 to
include these changes as well?
--
best regards,
Anthony
On 04/04/13 15:47, Laurent Bourgès wrote:
Dear all,
I just realized there is another problem with PlatformLogger log
statements:
XBaseWindow:
public boolean grabInput() {
grabLog.fine(Grab input on {0}, this);
...
}
This calls the PlatformLogger.fine( varargs):
public void fine(String msg, Object... params) {
logger.doLog(FINE, msg, params);
}
Doing so, the JVM creates a new Object[] instance to provide params as
varargs.
I would recommend using isLoggable() test to avoid such waste if the log
is disabled (fine, finer, finest ...).
Do you want me to provide a new patch related to this problem ?
Does somebody have an idea to automatically analyze the JDK code and
detect missing isLoggable statements ...
Regards,
Laurent
2013/4/3 Laurent Bourgès bourges.laur...@gmail.com
mailto:bourges.laurent@gmail.**com bourges.laur...@gmail.com
Anthony,
could you tell me once it is in the OpenJDK8 repository ?
I would then perform again profiling tests to check if there is no
more missing isLoggable() statements.
Once JMX and other projects switch to PlatformLogger, I could check
again.
Maybe I could write a small java code checker (pmd rule) to test if
there is missing isLoggable() statements wrapping PlatformLogger log
statements. Any idea about how to reuse java parser to do so ?
Regards,
Laurent
2013/4/2 Anthony Petrov anthony.pet...@oracle.com
mailto:anthony.petrov@oracle.**com anthony.pet...@oracle.com
Looks fine to me as well. Thanks for fixing this, Laurent.
Let's wait a couple more days in case Net or Swing folks want to
review the fix. After that I'll push it to the repository.
--
best regards,
Anthony
On 4/2/2013 15:35, Laurent Bourgès wrote:
Here is the updated patch:
http://jmmc.fr/~bourgesl/__**share/webrev-8010297.2/http://jmmc.fr/%7Ebourgesl/__share/webrev-8010297.2/
http://jmmc.fr/%7Ebourgesl/**share/webrev-8010297.2/http://jmmc.fr/%7Ebourgesl/share/webrev-8010297.2/
Fixed inconsistencies between FINE / FINER log statements:
- XScrollbarPeer
- XWindowPeer
Laurent
2013/4/2 Anthony Petrov anthony.pet...@oracle.com
mailto:anthony.petrov@oracle.**comanthony.pet...@oracle.com
mailto:anthony.petrov@oracle.**__com
mailto:anthony.petrov@oracle.**comanthony.pet...@oracle.com
1. Sergey: I believe this is for purposes of better
formating the
log output and making it more readable by separating or
highlighting
some sections. I don't think this should be changed.
2. Laurent: can you please address this issue and send
us a new patch?
--
best regards,
Anthony
On 4/1/2013 16:08, Sergey Bylokhov wrote:
Hi, Anthony
Only two comments:
1 Why we need some special text in the log output
like *** and
###
2 XScrollbarPeer.java:
+if
(log.isLoggable(**PlatformLogger.FINEST)) {
+log.finer(KeyEvent on scrollbar: +
event);
+}
On 4/1/13 12:20 PM, Anthony Petrov wrote:
Awt, Swing, Net engineers,
Could anyone review the fix please? For your
convenience:
Bug:
http://bugs.sun.com/view_bug._**___do?bug_id=8010297http://bugs.sun.com/view_bug.do?bug_id=8010297
http://bugs.sun.com/view_bug.**__do?bug_id=8010297http://bugs.sun.com/view_bug.__do?bug_id=8010297
http://bugs.sun.com/view_bug.**__do?bug_id=8010297http://bugs.sun.com/view_bug.__do?bug_id=8010297
http://bugs.sun.com/view_bug.**do?bug_id=8010297http://bugs.sun.com/view_bug.do?bug_id=8010297
Fix:
http://cr.openjdk.java.net/~__**
__anthony/8-55-isLoggable-**8010297.0/http://cr.openjdk.java.net/%7Eanthony/8-55-isLoggable-8010297.0/
http://cr.openjdk.java.net/%**7E__anthony/8-55-isLoggable-__
**8010297.0/http://cr.openjdk.java.net/%7E__anthony/8-55-isLoggable-__8010297.0/
http://cr.openjdk.java.net/%_**_7Eanthony/8-55-isLoggable-__
**8010297.0/
Re: Review request: 8011380: FX dependency on PlatformLogger broken
Hi Mandy, Laurent, What do you think of this variation (changed just PlatformLogger, other files exactly equal to your webrev): http://dl.dropbox.com/u/101777488/jdk8-tl/PlatformLogger/webrev.enumapi.02/index.html Moved the nearest = intValue search to PlatformLogger.Level.valueOf(int) since it is used also by deprecated API. With this change the same logic is used for mapping from j.u.l.Level to PlatformLogger.Level in all cases. I wonder if PlatformLogger.Level.intValue() should be exposed as public API. Currently it is used by the test (which is not in the same package). But this could be circumvented with reflection. I only see the use of it if we also make the PlatformLogger.Level.valueOf(int) public with anticipation of enabling PlatformLogger to use custom PlatformLogger.Level(s) like j.u.logging. This extension could be performed compatibly by transforming Level enum into a plain class like j.u.l.Level. But until then, I don't see the use of Level.intValue() as a public API. What do you think? Regards, Peter On 04/04/2013 06:52 AM, Mandy Chung wrote: Peter, Laurent, History and details are described below. Webrev at: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8011380/webrev.00/ I add back the getLevel(int), setLevel(int) and isLoggable(int) methods but marked deprecated and also revert the static final variables to resolve the regression. They can be removed when JavaFX transitions to use the Level enums (I'll file one issue for FX to use PlatformLogger.Level and one for core-libs to remove the deprecated methods). The performance overhead is likely small since it's direct mapping from int to the Level enum that was used in one of your previous performance measurement. Laurent - you have a patch to add isLoggable calls in the awt/swing code. Can you check if there is any noticeable performance difference? I also take the opportunity to reconsider what JavaLoggerProxy.getLevel() should return when it's a custom Level. Use of logging is expected not to cause any fatal error to the running application. The previous patch throwing IAE needs to be fixed. I propose to return the first Level.intValue() = the custom level value since the level value is used to evaluate if it's loggable. History and details: JavaFX 8 has converted to use sun.util.logging.PlatformLogger (https://javafx-jira.kenai.com/browse/RT-24458). I was involved in the early discussion but wasn't aware of the decision made. Thanks to Alan for catching this regression out before it's integrated to jdk8. jfxrt.jar is cobundled with jdk8 during the installer build step. My build doesn't build installer and thus we didn't see this regression. I ran jdeps on jdk8/lib/ext/jfxrt.jar (windows-i586) that shows 112 references to PlatformLogger and on jdk7/lib/jfxrt.jar that shows no reference to sun.util.logging. I have a method finder tool (planning to include it in jdk8) to search for use of PlatformLogger.getLevel and it did find 2 references from jdk8/lib/ext/jfxrt.jar. JavaFX 8 is going to upgrade to use JDK 8 for JavaFX build (https://javafx-jira.kenai.com/browse/RT-27794) soon (currently it's built with JDK 7). As soon as JavaFX code are changed to reference PlatformLogger.Level enum instead, we can remove the deprecated methods and change the PlatformLogger constants. JavaFX 2.2.x doesn't use sun.util.logging and so this has no impact to it. In any case, JavaFX 2.2.x only runs either bundled with a corresponding JDK 7u release, or as a standalone library for JDK 6 only. Thanks Mandy
Re: RFR 8005696: Add CompletableFuture - JEP 155
This is finally ready for integration. Going once... going twice...
Updated webrev and specdiff:
http://cr.openjdk.java.net/~chegar/8005696/ver.02/
Doug,
I just finalized the jtreg test, and found a small issue. Please find
below a suggested change (included in the webrev), and a small test to
demonstrate. Since the jtreg test reproduces the problem, the small test
is just for your convenience to validate the change, etc.
: cvs diff -u -U5 CompletableFuture.java
Index: CompletableFuture.java
===
RCS file:
/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/CompletableFuture.java,v
retrieving revision 1.80
diff -u -U 5 -r1.80 CompletableFuture.java
--- CompletableFuture.java 1 Apr 2013 20:16:05 - 1.80
+++ CompletableFuture.java 4 Apr 2013 13:34:58 -
@@ -2812,12 +2812,15 @@
}
if (dst == null)
dst = new CompletableFutureU();
}
}
-if (e == null ex != null)
+if (ex != null) {
+if (dst == null)
+dst = new CompletableFutureU();
dst.internalComplete(null, ex);
+}
}
helpPostComplete();
dst.helpPostComplete();
return dst;
}
Here is a test to reproduce:
: cat ThenCompose.java
import java.util.concurrent.CompletableFuture;
public class ThenCompose {
public static void main(String[] args) throws Exception {
CompletableFuture? cf1 = CompletableFuture.supplyAsync(() -
{ throw new RuntimeException(); });
CompletableFutureVoid cf2 = cf1.thenCompose((x) -
{ return new
CompletableFutureVoid(); });
}
}
:
/export/home/chris/repos/jdk8/tl/cf/build/solaris-sparc-normal-server-release/jdk/bin/java
ThenCompose
Exception in thread main java.lang.NullPointerException
at
java.util.concurrent.CompletableFuture.doCompose(CompletableFuture.java:2847)
at
java.util.concurrent.CompletableFuture.thenCompose(CompletableFuture.java:2756)
at ThenCompose.main(ThenCompose.java:8)
lection
-Chris.
On 03/27/2013 07:46 PM, Doug Lea wrote:
On 03/23/13 07:08, Doug Lea wrote:
There was a signature mismatch between the public
public static CompletableFutureVoid anyOf(CompletableFuture?...
cfs)
and the internal method performing most of the work:
private static CompletableFuture? anyTree(...)
(Void and ? are subtly different.)
Thanks to Martin especially for knocking some sense into me
about this. The type signature *was* incompatible, but
changing to Void rather the Object made this updated
version less useful (it could only return indication, not
result). So it is now updated with a signature-correct
version of anyOf with its original semantics.
Additionally, while being slightly disruptive anyway:
This class was lacking a little convenience method found in
other Future-based frameworks that surely will be requested.
So now added:
/**
* Returns a new CompletableFuture that is already completed with
* the given value.
*
* @param value the value
* @return the completed CompletableFuture
*/
public static U CompletableFutureU completedFuture(U value)
Re: RFR JDK-8011200 (was 7143928) : (coll) Optimize for Empty ArrayList and HashMap
On Tue, Apr 2, 2013 at 3:50 PM, Mike Duigou mike.dui...@oracle.com wrote: - T I will file a spec change request for the addition of , a power of 2 to the @serialData tag for this existing but previously unstated requirement. Hi Mike, I think changing the serialization spec is a mistake. There are a bunch of collection classes that use power-of-two backing arrays, but it's clearly the intent of the authors that this be an implementation detail. As often happens, implementation details get accidentally exposed (leaked) in a serial form. Here we see that the size of the backing array is embedded in the serial form. There is an incompatibility between the OpenJDK HashMap implementation and other implementations that don't use power-of-two backing arrays (which may not exist) but the right fix is to make the OpenJDK implementation more resilient to the input serial form. Treat the backing array size as merely a hint. If it's not a power of two, make it so! That is, remove the previously unstated requirement.
Re: Review request: 8011380: FX dependency on PlatformLogger broken
Alan - can you review this change? I have changed Level.valueOf(int) to return the nearest Level as Peter suggests using binary search: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8011380/webrev.01/ I want to push the changeset tomorrow since we need this fix before the TL integration. Several questions were brought up and I'm answering them in one shot: 1. The PlatformLogger static final fields have to retain the same since existing code can call: int level = PlatformLogger.FINE; logger.setLevel(level); There is also native code accessing PlatformLogger fields (will need to investigate more). Once we fix this type of incompatibility references, we can change them. Or we could remove these static final constants completely but it's less preferable since it touches many of the JDK files. I would keep these static fields as a shortcut. 2. New convenient methods (isInfoLoggable, isWarningLoggable) to minimize the dependency to the constants It's not a problem to have the dependencies. This is an issue this time since we were not aware of such dependency. The isLoggable method is simple enough. 3. 3 methods with two versions (one with int and one with Level parameter) As I said, I'll file a bug to remove the 3 deprecated methods in jdk8. I'm certain I can do so but just take some time to synchronize the changes. 4. It's true that you can set a PlatformLogger with a custom level via PlatformLogger API. But you can access a platform logger using java.util.logging API. Logger.getLogger(sun.awt.someLogger).setLevel(MyLevel.CUSTOM_LEVEL); PlatformLogger.getLevel() has to return some thing. Laurent suggests to remove (deprecate) PlatformLogger.getLevel() or level() method. I have to understand how the FX code uses getLevel(). We have to keep it due to the regression and for testing. For API perspective, having a method to find what level it's at is reasonable. Since we now have a clean solution to deal with custom level, I don't see any issue keeping it. 5. JavaFX 8 is likely to switch to build with JDK8 in a few weeks (really soon). 6. Level.intValue() public or not It mirrors java.util.logging.Level but may be able to do without it. Let's leave it public for now until FX converts to use the new code. I can clean this up at the same time. Mandy On 4/3/13 9:52 PM, Mandy Chung wrote: Peter, Laurent, History and details are described below. Webrev at: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8011380/webrev.00/ I add back the getLevel(int), setLevel(int) and isLoggable(int) methods but marked deprecated and also revert the static final variables to resolve the regression. They can be removed when JavaFX transitions to use the Level enums (I'll file one issue for FX to use PlatformLogger.Level and one for core-libs to remove the deprecated methods). The performance overhead is likely small since it's direct mapping from int to the Level enum that was used in one of your previous performance measurement. Laurent - you have a patch to add isLoggable calls in the awt/swing code. Can you check if there is any noticeable performance difference? I also take the opportunity to reconsider what JavaLoggerProxy.getLevel() should return when it's a custom Level. Use of logging is expected not to cause any fatal error to the running application. The previous patch throwing IAE needs to be fixed. I propose to return the first Level.intValue() = the custom level value since the level value is used to evaluate if it's loggable. History and details: JavaFX 8 has converted to use sun.util.logging.PlatformLogger (https://javafx-jira.kenai.com/browse/RT-24458). I was involved in the early discussion but wasn't aware of the decision made. Thanks to Alan for catching this regression out before it's integrated to jdk8. jfxrt.jar is cobundled with jdk8 during the installer build step. My build doesn't build installer and thus we didn't see this regression. I ran jdeps on jdk8/lib/ext/jfxrt.jar (windows-i586) that shows 112 references to PlatformLogger and on jdk7/lib/jfxrt.jar that shows no reference to sun.util.logging. I have a method finder tool (planning to include it in jdk8) to search for use of PlatformLogger.getLevel and it did find 2 references from jdk8/lib/ext/jfxrt.jar. JavaFX 8 is going to upgrade to use JDK 8 for JavaFX build (https://javafx-jira.kenai.com/browse/RT-27794) soon (currently it's built with JDK 7). As soon as JavaFX code are changed to reference PlatformLogger.Level enum instead, we can remove the deprecated methods and change the PlatformLogger constants. JavaFX 2.2.x doesn't use sun.util.logging and so this has no impact to it. In any case, JavaFX 2.2.x only runs either bundled with a corresponding JDK 7u release, or as a standalone library for JDK 6 only. Thanks Mandy
hg: jdk8/tl/jdk: 8000406: change files using @GenerateNativeHeader to use @Native
Changeset: 7b1189bf1d7b Author:dxu Date: 2013-04-04 15:39 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7b1189bf1d7b 8000406: change files using @GenerateNativeHeader to use @Native Summary: Use @Native annotation to mark constants interested by native codes Reviewed-by: alanb, anthony, prr ! src/macosx/classes/apple/laf/JRSUIConstants.java ! src/macosx/classes/com/apple/eawt/FullScreenHandler.java ! src/macosx/classes/com/apple/eawt/event/GestureHandler.java ! src/macosx/classes/sun/java2d/OSXSurfaceData.java ! src/macosx/classes/sun/lwawt/macosx/CocoaConstants.java ! src/macosx/native/jobjc/src/core/PrimitiveCoder.hs ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/CFType.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/Coder.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/FFIType.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/Function.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/ID.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/Invoke.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/JObjCRuntime.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/MacOSXFramework.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/NSClass.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/NativeArgumentBuffer.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/NativeBuffer.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/NativeObjectLifecycleManager.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/Opaque.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/Pointer.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/PrimitiveCoder.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/SEL.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/Struct.java ! src/macosx/native/jobjc/src/core/java/com/apple/jobjc/Subclassing.java ! src/macosx/native/jobjc/src/core/native/Invoke.m ! src/macosx/native/jobjc/src/core/native/JObjCRuntime.m ! src/macosx/native/sun/awt/PrinterView.m ! src/share/classes/java/awt/Adjustable.java ! src/share/classes/java/awt/AlphaComposite.java ! src/share/classes/java/awt/BasicStroke.java ! src/share/classes/java/awt/Choice.java ! src/share/classes/java/awt/DisplayMode.java ! src/share/classes/java/awt/Image.java ! src/share/classes/java/awt/List.java ! src/share/classes/java/awt/PopupMenu.java ! src/share/classes/java/awt/SystemColor.java ! src/share/classes/java/awt/TextComponent.java ! src/share/classes/java/awt/Transparency.java ! src/share/classes/java/awt/color/ColorSpace.java ! src/share/classes/java/awt/color/ICC_Profile.java ! src/share/classes/java/awt/datatransfer/StringSelection.java ! src/share/classes/java/awt/dnd/DnDConstants.java ! src/share/classes/java/awt/event/ActionEvent.java ! src/share/classes/java/awt/event/AdjustmentEvent.java ! src/share/classes/java/awt/event/ComponentEvent.java ! src/share/classes/java/awt/event/FocusEvent.java ! src/share/classes/java/awt/event/InputMethodEvent.java ! src/share/classes/java/awt/event/MouseWheelEvent.java ! src/share/classes/java/awt/event/WindowEvent.java ! src/share/classes/java/awt/geom/PathIterator.java ! src/share/classes/java/awt/image/AffineTransformOp.java ! src/share/classes/java/awt/image/ConvolveOp.java ! src/share/classes/java/awt/image/DataBuffer.java ! src/share/classes/java/awt/image/ImageConsumer.java ! src/share/classes/java/awt/image/ImageObserver.java ! src/share/classes/java/awt/peer/ComponentPeer.java ! src/share/classes/java/awt/print/PageFormat.java ! src/share/classes/java/awt/print/Pageable.java ! src/share/classes/java/awt/print/Printable.java ! src/share/classes/sun/awt/EmbeddedFrame.java ! src/share/classes/sun/awt/SunHints.java ! src/share/classes/sun/awt/dnd/SunDragSourceContextPeer.java ! src/share/classes/sun/awt/image/BufImgSurfaceData.java ! src/share/classes/sun/font/FontManager.java ! src/share/classes/sun/java2d/SunGraphics2D.java ! src/share/classes/sun/java2d/opengl/OGLBlitLoops.java ! src/share/classes/sun/java2d/opengl/OGLContext.java ! src/share/classes/sun/java2d/pipe/BufferedContext.java ! src/share/classes/sun/java2d/pipe/BufferedOpCodes.java ! src/share/classes/sun/java2d/pipe/BufferedPaints.java ! src/share/classes/sun/java2d/pipe/BufferedTextPipe.java ! src/share/classes/sun/java2d/pipe/RenderBuffer.java ! src/share/classes/sun/java2d/pipe/hw/AccelDeviceEventNotifier.java ! src/share/classes/sun/java2d/pipe/hw/AccelSurface.java ! src/share/classes/sun/java2d/pipe/hw/ContextCapabilities.java ! src/share/classes/sun/nio/ch/DatagramChannelImpl.java ! src/share/classes/sun/nio/ch/sctp/SctpStdSocketOption.java ! src/share/classes/sun/security/pkcs11/Secmod.java ! src/share/classes/sun/security/pkcs11/wrapper/PKCS11.java ! src/solaris/classes/sun/awt/X11/XComponentPeer.java ! src/solaris/classes/sun/nio/ch/sctp/AssociationChange.java ! src/solaris/classes/sun/nio/ch/sctp/PeerAddrChange.java !
Looking for a sponsor to review changes made to two unit tests under jdk/test
Hi all, During #TestFest in London last month we hacked away on jtreg and tests in the OpenJDK. Myself and another participant managed to change two unit tests. I haven't looked for a sponsor in the past so I'm fairly new to the process, hence my email on the list is to request for someone to review, and process these patches further. I'm a committer (signed OCA). Here's details of the changes made to the tests under the .*./jdk8_tl/jdk*folders: 1) *test/java/lang/ref/Basic.java.patch* - changed to not use Thread.sleep() any more rather use the java.util.concurrent.CountdownLatch functionality 2) *test/java/lang/Runtime/exec/LotsOfOutput.java.patch* - refactor-ing and tidy-ing of existing code (removing string literals and replacing with constants, etc...) Please let me know what you would like me to do next with these patches. Regards, mani -- *Twitter:* @theNeomatrix369 *Blog:* http://neomatrix369.wordpress.com *JUG activity:* LJC Advocate (@adoptopenjdk @adoptajsr programs) *Meet-a-Project:* https://github.com/MutabilityDetector *Devoxx UK 2013 was a grand success:* http://www.devoxx.com/display/UK13/Home *Don't chase success, rather aim for Excellence, and success will come chasing after you!*
Re: Looking for a sponsor to review changes made to two unit tests under jdk/test
Hi all, I'd like to rectify that I;m a contributor (and not a committer as mentioned earlier), so I don't have access to the webrev system but would love to have these patches hosted on them when a sponsor becomes available. Cheers, mani On Thu, Apr 4, 2013 at 11:57 PM, Mani Sarkar sadhak...@gmail.com wrote: Hi all, During #TestFest in London last month we hacked away on jtreg and tests in the OpenJDK. Myself and another participant managed to change two unit tests. I haven't looked for a sponsor in the past so I'm fairly new to the process, hence my email on the list is to request for someone to review, and process these patches further. I'm a committer (signed OCA). Here's details of the changes made to the tests under the .*./jdk8_tl/jdk*folders: 1) *test/java/lang/ref/Basic.java.patch* - changed to not use Thread.sleep() any more rather use the java.util.concurrent.CountdownLatch functionality 2) *test/java/lang/Runtime/exec/LotsOfOutput.java.patch* - refactor-ing and tidy-ing of existing code (removing string literals and replacing with constants, etc...) Please let me know what you would like me to do next with these patches. Regards, mani -- *Twitter:* @theNeomatrix369 *Blog:* http://neomatrix369.wordpress.com *JUG activity:* LJC Advocate (@adoptopenjdk @adoptajsr programs) *Meet-a-Project:* https://github.com/MutabilityDetector *Devoxx UK 2013 was a grand success:* http://www.devoxx.com/display/UK13/Home *Don't chase success, rather aim for Excellence, and success will come chasing after you!* -- *Twitter:* @theNeomatrix369 *Blog:* http://neomatrix369.wordpress.com *JUG activity:* LJC Advocate (@adoptopenjdk @adoptajsr programs) *Meet-a-Project:* https://github.com/MutabilityDetector *Devoxx UK 2013 was a grand success:* http://www.devoxx.com/display/UK13/Home *Don't chase success, rather aim for Excellence, and success will come chasing after you!*
Re: Looking for a sponsor to review changes made to two unit tests under jdk/test
Hi Mani, Attachments get stripped so you will need to inline the patches in your email to the list. Thanks, David On 5/04/2013 9:07 AM, Mani Sarkar wrote: Hi all, I'd like to rectify that I;m a contributor (and not a committer as mentioned earlier), so I don't have access to the webrev system but would love to have these patches hosted on them when a sponsor becomes available. Cheers, mani On Thu, Apr 4, 2013 at 11:57 PM, Mani Sarkar sadhak...@gmail.com wrote: Hi all, During #TestFest in London last month we hacked away on jtreg and tests in the OpenJDK. Myself and another participant managed to change two unit tests. I haven't looked for a sponsor in the past so I'm fairly new to the process, hence my email on the list is to request for someone to review, and process these patches further. I'm a committer (signed OCA). Here's details of the changes made to the tests under the .*./jdk8_tl/jdk*folders: 1) *test/java/lang/ref/Basic.java.patch* - changed to not use Thread.sleep() any more rather use the java.util.concurrent.CountdownLatch functionality 2) *test/java/lang/Runtime/exec/LotsOfOutput.java.patch* - refactor-ing and tidy-ing of existing code (removing string literals and replacing with constants, etc...) Please let me know what you would like me to do next with these patches. Regards, mani -- *Twitter:* @theNeomatrix369 *Blog:* http://neomatrix369.wordpress.com *JUG activity:* LJC Advocate (@adoptopenjdk @adoptajsr programs) *Meet-a-Project:* https://github.com/MutabilityDetector *Devoxx UK 2013 was a grand success:* http://www.devoxx.com/display/UK13/Home *Don't chase success, rather aim for Excellence, and success will come chasing after you!*
Re: Looking for a sponsor to review changes made to two unit tests under jdk/test
Thanks David,
Here are the patches, let me know if they have come in fine:
1) test/java/lang/ref/Basic.**java.patch - changed to not use
Thread.sleep() any more rather use the java.util.concurrent.**CountdownLatch
functionality
Authors: Mani (sadhak...@gmail.com) and Edward Yue Shung Wong (
edward.ys.w...@gmail.com)
x---
diff -r 38e1821c4472 test/java/lang/ref/Basic.java
--- a/test/java/lang/ref/Basic.java Wed Mar 06 18:35:51 2013 +0100
+++ b/test/java/lang/ref/Basic.java Sat Mar 23 14:51:25 2013 +
@@ -29,7 +29,7 @@
import java.lang.ref.*;
import java.util.Vector;
-
+import java.util.concurrent.CountDownLatch;
public class Basic {
@@ -64,22 +64,22 @@
fork(new Runnable() {
public void run() {
System.err.println(References: W + rw.get()
- + , W2 + rw2.get()
- + , P + rp.get()
- + , P2 + rp2.get());
++ , W2 + rw2.get()
++ , P + rp.get()
++ , P2 + rp2.get());
}
});
}
-static void createNoise() throws InterruptedException {
+static void createNoise(final CountDownLatch cdl) throws
InterruptedException {
fork(new Runnable() {
public void run() {
keep.addElement(new PhantomReference(new Object(), q2));
+createNoiseHasFinishedAddingToKeep(cdl);
}
});
}
-
public static void main(String[] args) throws Exception {
fork(new Runnable() {
@@ -97,13 +97,16 @@
int ndq = 0;
boolean prevFinalized = false;
-outer:
+outer:
for (int i = 1;; i++) {
Reference r;
-createNoise();
+CountDownLatch inQueueWaitLatch = new CountDownLatch(1);
+createNoise(inQueueWaitLatch);
+
System.err.println(GC + i);
-Thread.sleep(10);
+waitUntilCreateNoiseHasFinished(inQueueWaitLatch);
+
System.gc();
System.runFinalization();
@@ -137,7 +140,7 @@
if (ndq != 3) {
throw new Exception(Expected to dequeue 3 reference objects,
-+ but only got + ndq);
++ but only got + ndq);
}
if (! Basic.finalized) {
@@ -146,4 +149,13 @@
}
+
+private static void createNoiseHasFinishedAddingToKeep(CountDownLatch
cdl) {
+cdl.countDown();
+}
+
+private static void waitUntilCreateNoiseHasFinished(CountDownLatch
cdl) throws InterruptedException {
+cdl.await();
+}
+
}
x
2) test/java/lang/Runtime/exec/**LotsOfOutput.java.patch - refactor-ing and
tidy-ing of existing code (removing string literals and replacing with
constants, etc...)
Author: Edward Yue Shung Wong (edward.ys.w...@gmail.com)
x
diff -r 38e1821c4472 test/java/lang/Runtime/exec/LotsOfOutput.java
--- a/test/java/lang/Runtime/exec/LotsOfOutput.java Wed Mar 06 18:35:51
2013 +0100
+++ b/test/java/lang/Runtime/exec/LotsOfOutput.java Sat Mar 23 15:48:46
2013 +
@@ -33,17 +33,24 @@
public class LotsOfOutput {
static final String CAT = /usr/bin/cat;
-public static void main(String[] args) throws Exception{
-if (File.separatorChar == '\\' ||// Windows
-!new File(CAT).exists()) // no cat
+static final int MEMORY_GROWTH_LIMIT = 100;
+
+public static void main(String[] args) throws Exception {
+if (isWindowsOrCatNotAvailable()) {
return;
+}
+
Process p = Runtime.getRuntime().exec(CAT + /dev/zero);
long initMemory = Runtime.getRuntime().totalMemory();
-for (int i=1; i 10; i++) {
+for (int i = 1; i 10; i++) {
Thread.sleep(100);
-if (Runtime.getRuntime().totalMemory() initMemory + 100)
-throw new Exception(Process consumes memory.);
+if (Runtime.getRuntime().totalMemory() initMemory +
MEMORY_GROWTH_LIMIT)
+throw new Exception(Runtime memory has grown more than:
+ MEMORY_GROWTH_LIMIT);
}
}
+
+private static boolean isWindowsOrCatNotAvailable() {
+return File.separatorChar == '\\' || !new File(CAT).exists();
+}
}
x
Any queries please let me know.
Thanks.
Regards,
mani
On Fri, Apr 5, 2013 at 1:38 AM, David Holmes david.hol...@oracle.comwrote:
Hi Mani,
Attachments get stripped so you will need to inline the patches in your
email to the list.
Thanks,
David
On 5/04/2013 9:07 AM, Mani Sarkar wrote:
Hi all,
I'd like to rectify that I;m a contributor (and not a committer as
mentioned earlier), so I don't have access to the webrev system but would
love to have these patches hosted on them when a sponsor becomes
Re: Looking for a sponsor to review changes made to two unit tests under jdk/test
Hi Mani,
Patches came through ok.
I would not add the extra methods around the cdl.await() and
cdl.countDown() as they just obscure things in my opinion. But that
aside the latch is not needed. The fork() method starts a thread and
joins it. So when createNoise() returns we already know for certain that
the noise has been created. What the sleep is doing is giving the GC a
chance to run.
David
On 5/04/2013 10:55 AM, Mani Sarkar wrote:
Thanks David,
Here are the patches, let me know if they have come in fine:
1) test/java/lang/ref/Basic.__java.patch - changed to not use
Thread.sleep() any more rather use the java.util.concurrent.__CountdownLatch
functionality
Authors: Mani (sadhak...@gmail.com mailto:sadhak...@gmail.com) and
Edward Yue Shung Wong (edward.ys.w...@gmail.com
mailto:edward.ys.w...@gmail.com)
x---
diff -r 38e1821c4472 test/java/lang/ref/Basic.java
--- a/test/java/lang/ref/Basic.javaWed Mar 06 18:35:51 2013 +0100
+++ b/test/java/lang/ref/Basic.javaSat Mar 23 14:51:25 2013 +
@@ -29,7 +29,7 @@
import java.lang.ref.*;
import java.util.Vector;
-
+import java.util.concurrent.CountDownLatch;
public class Basic {
@@ -64,22 +64,22 @@
fork(new Runnable() {
public void run() {
System.err.println(References: W + rw.get()
- + , W2 + rw2.get()
- + , P + rp.get()
- + , P2 + rp2.get());
++ , W2 + rw2.get()
++ , P + rp.get()
++ , P2 + rp2.get());
}
});
}
-static void createNoise() throws InterruptedException {
+static void createNoise(final CountDownLatch cdl) throws
InterruptedException {
fork(new Runnable() {
public void run() {
keep.addElement(new PhantomReference(new Object(), q2));
+createNoiseHasFinishedAddingToKeep(cdl);
}
});
}
-
public static void main(String[] args) throws Exception {
fork(new Runnable() {
@@ -97,13 +97,16 @@
int ndq = 0;
boolean prevFinalized = false;
-outer:
+outer:
for (int i = 1;; i++) {
Reference r;
-createNoise();
+CountDownLatch inQueueWaitLatch = new CountDownLatch(1);
+createNoise(inQueueWaitLatch);
+
System.err.println(GC + i);
-Thread.sleep(10);
+waitUntilCreateNoiseHasFinished(inQueueWaitLatch);
+
System.gc();
System.runFinalization();
@@ -137,7 +140,7 @@
if (ndq != 3) {
throw new Exception(Expected to dequeue 3 reference objects,
-+ but only got + ndq);
++ but only got + ndq);
}
if (! Basic.finalized) {
@@ -146,4 +149,13 @@
}
+
+private static void
createNoiseHasFinishedAddingToKeep(CountDownLatch cdl) {
+cdl.countDown();
+}
+
+private static void waitUntilCreateNoiseHasFinished(CountDownLatch
cdl) throws InterruptedException {
+cdl.await();
+}
+
}
x
2) test/java/lang/Runtime/exec/__LotsOfOutput.java.patch - refactor-ing
and tidy-ing of existing code (removing string literals and replacing
with constants, etc...)
Author: Edward Yue Shung Wong (edward.ys.w...@gmail.com
mailto:edward.ys.w...@gmail.com)
x
diff -r 38e1821c4472 test/java/lang/Runtime/exec/LotsOfOutput.java
--- a/test/java/lang/Runtime/exec/LotsOfOutput.javaWed Mar 06 18:35:51
2013 +0100
+++ b/test/java/lang/Runtime/exec/LotsOfOutput.javaSat Mar 23 15:48:46
2013 +
@@ -33,17 +33,24 @@
public class LotsOfOutput {
static final String CAT = /usr/bin/cat;
-public static void main(String[] args) throws Exception{
-if (File.separatorChar == '\\' ||// Windows
-!new File(CAT).exists()) // no cat
+static final int MEMORY_GROWTH_LIMIT = 100;
+
+public static void main(String[] args) throws Exception {
+if (isWindowsOrCatNotAvailable()) {
return;
+}
+
Process p = Runtime.getRuntime().exec(CAT + /dev/zero);
long initMemory = Runtime.getRuntime().totalMemory();
-for (int i=1; i 10; i++) {
+for (int i = 1; i 10; i++) {
Thread.sleep(100);
-if (Runtime.getRuntime().totalMemory() initMemory + 100)
-throw new Exception(Process consumes memory.);
+if (Runtime.getRuntime().totalMemory() initMemory +
MEMORY_GROWTH_LIMIT)
+throw new Exception(Runtime memory has grown more
than: + MEMORY_GROWTH_LIMIT);
}
}
+
+private static boolean isWindowsOrCatNotAvailable() {
+return File.separatorChar == '\\' || !new File(CAT).exists();
+
Re: Looking for a sponsor to review changes made to two unit tests under jdk/test
On 5/04/2013 11:27 AM, Mani Sarkar wrote:
Hi David,
I'll reply in more detail later but to respond to your comment on:
I would not add the extra methods around the cdl.await() and
cdl.countDown() as they just obscure things
In general its meant to do the opposite. We come across a lot of code
that does not express intent, and the purpose of encapsulating such
blocks (even if its a single line) is to make the flow verbose and
readable - I have seen similar code-snippets in the Hotspot (C/C++
codebase) making it easy to follow what is going on. One of the things I
picked up from TestFest was to make the tests more legible, logical and
easy to maintain and scale - it was our intent when we changed this test.
Sorry, createNoiseHasFinishedAddingToKeep is certainly verbose but not
readable. This would be much more readable:
Countdownlatch noiseComplete = new ...
createNoise(noiseComplete);
...
noiseComplete.await();
and:
static void createNoise(final CountDownLatch complete) throws
InterruptedException {
...
complete.countDown();
}
just giving the latch a meaningful name is sufficient to convey intent.
Note: in this example a Semaphore is probably a better choice than a
CountDownLatch.
Cheers,
David
I'll come back with responses for your other comments.
Cheers
mani
On Fri, Apr 5, 2013 at 2:07 AM, David Holmes david.hol...@oracle.com
mailto:david.hol...@oracle.com wrote:
Hi Mani,
Patches came through ok.
I would not add the extra methods around the cdl.await() and
cdl.countDown() as they just obscure things in my opinion. But that
aside the latch is not needed. The fork() method starts a thread and
joins it. So when createNoise() returns we already know for certain
that the noise has been created. What the sleep is doing is giving
the GC a chance to run.
David
On 5/04/2013 10:55 AM, Mani Sarkar wrote:
Thanks David,
Here are the patches, let me know if they have come in fine:
1) test/java/lang/ref/Basic.java.patch - changed to not use
Thread.sleep() any more rather use the
java.util.concurrent.CountdownLatch
functionality
Authors: Mani (sadhak...@gmail.com mailto:sadhak...@gmail.com
mailto:sadhak...@gmail.com mailto:sadhak...@gmail.com) and
Edward Yue Shung Wong (edward.ys.w...@gmail.com
mailto:edward.ys.w...@gmail.com
mailto:edward.ys.wong@gmail.__com
mailto:edward.ys.w...@gmail.com)
x---
diff -r 38e1821c4472 test/java/lang/ref/Basic.java
--- a/test/java/lang/ref/Basic.__javaWed Mar 06 18:35:51 2013 +0100
+++ b/test/java/lang/ref/Basic.__javaSat Mar 23 14:51:25 2013 +
@@ -29,7 +29,7 @@
import java.lang.ref.*;
import java.util.Vector;
-
+import java.util.concurrent.__CountDownLatch;
public class Basic {
@@ -64,22 +64,22 @@
fork(new Runnable() {
public void run() {
System.err.println(__References: W + rw.get()
- + , W2 + rw2.get()
- + , P + rp.get()
- + , P2 + rp2.get());
++ , W2 + rw2.get()
++ , P + rp.get()
++ , P2 + rp2.get());
}
});
}
-static void createNoise() throws InterruptedException {
+static void createNoise(final CountDownLatch cdl) throws
InterruptedException {
fork(new Runnable() {
public void run() {
keep.addElement(new PhantomReference(new
Object(), q2));
+createNoiseHasFinishedAddingTo__Keep(cdl);
}
});
}
-
public static void main(String[] args) throws Exception {
fork(new Runnable() {
@@ -97,13 +97,16 @@
int ndq = 0;
boolean prevFinalized = false;
-outer:
+outer:
for (int i = 1;; i++) {
Reference r;
-createNoise();
+CountDownLatch inQueueWaitLatch = new
CountDownLatch(1);
+createNoise(inQueueWaitLatch);
+
System.err.println(GC + i);
-Thread.sleep(10);
+waitUntilCreateNoiseHasFinishe__d(inQueueWaitLatch);
+
System.gc();
System.runFinalization();
@@ -137,7 +140,7 @@
if (ndq != 3) {
throw new Exception(Expected to dequeue 3
reference objects,
-
hg: jdk8/tl/jdk: 7155720: PKCS11 minor issues in native code
Changeset: 7d4e30730f80 Author:valeriep Date: 2013-04-04 20:05 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7d4e30730f80 7155720: PKCS11 minor issues in native code Summary: Added OOM handling to address the two issues found by parfait. Reviewed-by: weijun ! src/solaris/native/sun/security/pkcs11/wrapper/p11_md.c
