Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build
On 9/16/2016 10:34 AM, Volker Simonis wrote: Hi Christoph, I think your change is fine as a quick-fix to fix the build. But you're completely right that this should be reworked in the long term. I hate to see that we now have the third version of these routines in the OpenJDK. Unfortunately a clean solution is not trivial. libjli is not linked against libjvm because libjli is actually used to load libjvm. So we can not put the dladdr routines for AIX there. But I think we should at least consolidate the two versions which will be in the class library after your change. Initially I intentionally put porting_aix.{c,h} into jdk/aix/porting with the intent to make it available to ALL jdk native libraries. Unfortunately, with the source code reorganization due to the modular changes, the common, top-level aix repository had to go away and the code was moved to src/java.desktop/aix/native/libawt/porting_aix.c. With the reorganized, modularized source code layout and build system it is not possible to share code between modules. We somehow have to fix this although I currently don't know how. IF somebody has an idea, please let me know :) Why doesn't AIX support a Standard C API that most other *nix based OS'es support ? Thanks Kumar Regarding your change: - can you please move +#if defined(_AIX) +#include "java_md_aix.h" +#endif + from java_md_common.c to the end of java.base/unix/native/libjli/java_md.h. It will then be automatically included into java_md_common.c trough java.h which includes java_md.h Also, this version of dladdr is inherently not thread safe. I don't think that's a problem here, but it would be nice if you could quickly check if that's indeed true. Besides that, looks good. Thanks for fixing, Volker On Fri, Sep 16, 2016 at 11:58 AM, Langer, Christoph wrote: Hi, the fix for https://bugs.openjdk.java.net/browse/JDK-8165524 breaks the AIX build as function dladdr is not available on AIX. There already exist ports of that API in hotspot and awt. With the proposed change I duplicate the awt port to libjli. This is maybe only a quick fix - eventually we should think about consolidating and using the hotspot port in all places by exporting it from libjvm.so for AIX. Bug: https://bugs.openjdk.java.net/browse/JDK-8166189 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8166189.0/ Thanks Christoph -Original Message- From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf Of Kumar Srinivasan Sent: Montag, 12. September 2016 22:57 To: core-libs-dev ; Mandy Chung ; Chris Bensen Subject: RFR: 8165524: Better detect JRE that Linux JLI will be using Hello, I am sponsoring this changeset for Chris Bensen of the java packager team, please review fix for the launcher to better locate java.home. http://cr.openjdk.java.net/~ksrini/8165524/ Thanks Kumar
Re: RFR: 8151832: Improve exception messages in exceptions thrown by jigsaw code
Resurrecting this old review thread. After some internal discussion, I've dropped the minor edit that was made in StackTraceElementCompositeData. It could be noisy data for exception purposes. I've corrected the other issues raised by Alan and Jim has long pushed the changes mentioned below. webrev : http://cr.openjdk.java.net/~coffeys/webrev.8151832.v3/webrev/ Regards, Sean. On 01/06/16 16:00, Seán Coffey wrote: On 01/06/16 10:21, Alan Bateman wrote: On 31/05/2016 18:57, Seán Coffey wrote: new webrev : http://cr.openjdk.java.net/~coffeys/webrev.8151832.v2/webrev/ Also in JprtPath.checkPath then I assume path.getClass() is enough as the toString is specified to return a useful string. In JrtPath then "nul" has been renamed to "null". I'm not sure why this has changed. If it is confusing the NUL (one L) should be fine. nul looked odd/wrong. I'll replace with NUL then. Jim might want to comment on the jimage updates. In most cases then hitting any of these means the JDK is hosed. That is, if we have a bug here or the jimage container file is corrupted then it will likely not start. Ok - will pass this by Jim. In StackTraceElementCompositeData then I'm not sure if printing the CompositeType adds anything useful. I might be better to extract wording from the table in ThreadInfo.from to make it clear that the stackTrace attribute is missing attributes. This reminds me, I suspect this table might need to be updated for JDK 9. I will create a bug for that. CompositeType.toString() is pretty comprehensive and iterates through the instance's keyset. I thought the extra output would hint at what went wrong. I could print both Objects if you want a better comparison. Or I can start delving into the ThreadInfo.from table if you think that's a more correct approach. regards, Sean. -Alan
RFR: 8166398: CatalogSupport tests need to be fixed -> Re: RFR: 8166220: Catalog API: JAXP XML Processor Support - add StAX test coverage
Hi Daniel, Here's the fix for the test issues, a couple of errors including missing handler in StAX test, and dtd was mistakingly typed as xsd . The root cause is a debugging assertEquals method that printed out debugging message without throwing Exception. I've searched the jaxp/test to make sure this is the only case where this is used. While fixing the tests, also refactored the code so that the use-catalog is checked as a top condition, removed a ObjectFactory call in the course. JBS: https://bugs.openjdk.java.net/browse/JDK-8166398 webrev: http://cr.openjdk.java.net/~joehw/jdk9/8166398/webrev/ Thanks, Joe On 9/20/16, 11:22 AM, Joe Wang wrote: On 9/20/16, 2:20 AM, Daniel Fuchs wrote: Hi Joe, test/javax/xml/jaxp/unittest/catalog/CatalogSupportBase.java 603 /** 604 * Returns the text of the first element found by the reader. 605 * @param streamReader the XMLStreamReader 606 * @return the text of the first element 607 * @throws XMLStreamException 608 */ 609 String getText(XMLStreamReader streamReader, int type) throws XMLStreamException { It would be good to update the javadoc of this method (in particular document the new 'type' parameter) so that future maintainer can understand what that method is supposed to do. Thanks Daniel! I'll fix this. As I went through the tests again, I found there's actually an error in the StAX test where the handler was missed. I filed a new bug: https://bugs.openjdk.java.net/browse/JDK-8166398 In particular would it be OK to encounter both CHARACTERS and ENTITY_REFERENCE, or are these exclusive. If they are should some exception be thrown? They can't. At any given point, the StAX parser returns an unique event and its event type. I can't say I understand how the new test works, but nothing else jumped at me in your webrev :-) The incorrect javadoc was enough to raise an alarm, that this part of code was copied and forgot to update. It led me to double-check the code and found the issue (JDK-8166398). I really appreciate it! Best, Joe best regards, -- daniel On 19/09/16 20:11, Joe Wang wrote: Hi, Please review an addition of StAX tests to the Catalog Support test set. JBS: https://bugs.openjdk.java.net/browse/JDK-8166220 webrev: http://cr.openjdk.java.net/~joehw/jdk9/8166220/webrev/ Thanks, Joe
Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build
> On Sep 21, 2016, at 6:38 AM, Kumar Srinivasan > wrote: > > > On 9/16/2016 10:34 AM, Volker Simonis wrote: >> Hi Christoph, >> >> I think your change is fine as a quick-fix to fix the build. But >> you're completely right that this should be reworked in the long term. >> I hate to see that we now have the third version of these routines in >> the OpenJDK. Unfortunately a clean solution is not trivial. >> >> libjli is not linked against libjvm because libjli is actually used to >> load libjvm. So we can not put the dladdr routines for AIX there. But >> I think we should at least consolidate the two versions which will be >> in the class library after your change. Initially I intentionally put >> porting_aix.{c,h} into jdk/aix/porting with the intent to make it >> available to ALL jdk native libraries. >> >> Unfortunately, with the source code reorganization due to the modular >> changes, the common, top-level aix repository had to go away and the >> code was moved to src/java.desktop/aix/native/libawt/porting_aix.c. >> With the reorganized, modularized source code layout and build system >> it is not possible to share code between modules. We somehow have to >> fix this although I currently don't know how. IF somebody has an idea, >> please let me know :) > > Why doesn't AIX support a Standard C API that most other > *nix based OS'es support ? I was wondering the same thing. Chris > > Thanks > > Kumar > >> >> Regarding your change: >> >> - can you please move >> >> +#if defined(_AIX) >> +#include "java_md_aix.h" >> +#endif >> + >> >> from java_md_common.c to the end of >> java.base/unix/native/libjli/java_md.h. It will then be automatically >> included into java_md_common.c trough java.h which includes java_md.h >> >> Also, this version of dladdr is inherently not thread safe. I don't >> think that's a problem here, but it would be nice if you could quickly >> check if that's indeed true. >> >> Besides that, looks good. >> >> Thanks for fixing, >> Volker >> >> >> >> >> On Fri, Sep 16, 2016 at 11:58 AM, Langer, Christoph >> wrote: >>> Hi, >>> >>> the fix for https://bugs.openjdk.java.net/browse/JDK-8165524 breaks the AIX >>> build as function dladdr is not available on AIX. >>> >>> There already exist ports of that API in hotspot and awt. With the proposed >>> change I duplicate the awt port to libjli. This is maybe only a quick fix - >>> eventually we should think about consolidating and using the hotspot port >>> in all places by exporting it from libjvm.so for AIX. >>> >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8166189 >>> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8166189.0/ >>> >>> Thanks >>> Christoph >>> >>> -Original Message- From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf Of Kumar Srinivasan Sent: Montag, 12. September 2016 22:57 To: core-libs-dev ; Mandy Chung ; Chris Bensen Subject: RFR: 8165524: Better detect JRE that Linux JLI will be using Hello, I am sponsoring this changeset for Chris Bensen of the java packager team, please review fix for the launcher to better locate java.home. http://cr.openjdk.java.net/~ksrini/8165524/ Thanks Kumar >
RFR: jsr166 jdk9 integration wave 11
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/ Notable here is an attempt to make a minimal completion stage more acceptable as a return value from APIs, by making completableFuture.minimalCompletionStage().toCompletableFuture() useful. Lots of boring changes to appease static analysis tools.
Re: RFR: jsr166 jdk9 integration wave 11
On 09/21/2016 09:33 PM, Martin Buchholz wrote: > http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/ Looks fine. Multi-story predicates sure look better now... -Aleksey
We need to add blocking methods to CompletionStage!
(Sorry to re-open this discussion) The separation of a read-only CompletionStage from CompletableFuture is great. I'm a fan of the scala style Promise/Future split as described in http://docs.scala-lang.org/overviews/core/futures.html, but: we need to re-add (safe, read-only) blocking methods like join. Java is not Node.js, where there are no threads but there is a universal event loop. Java programmers are used to Future, where the *only* way to use a future's value is to block waiting for it. The existing CompletionStage methods are a better scaling alternative to blocking all the time, but blocking is almost always eventually necessary in Java. For example, junit test methods that start any asynchronous computation need to block until the computation is done, before returning. As Viktor has pointed out, users can always implement blocking themselves by writing static CompletableFuture toCompletableFuture(CompletionStage stage) { CompletableFuture f = new CompletableFuture<>(); stage.handle((T t, Throwable ex) -> { if (ex != null) f.completeExceptionally(ex); else f.complete(t); return null; }); return f; } static T join(CompletionStage stage) { return toCompletableFuture(stage).join(); } but unlike Viktor, I think it's unreasonable to not provide this for users (especially when we can do so more efficiently). What is happening instead is API providers not using CompletionStage as return values in public APIs because of the lack of convenient blocking, and instead returning CompletableFuture, which is a tragic software engineering failure. Re-adding join is easy. We discourage CompletionStage.toCompletableFuture from throwing UnsupportedOperationException, and implement join as: public default T join() { return toCompletableFuture().join(); } There is a risk of multiple-inheritance conflict with Future if we add e.g. isDone(), but there are no current plans to turn those Future methods into default methods, and even if we did in some future release, it would be only a source, not binary incompatibility, so far less serious.
Re: [concurrency-interest] We need to add blocking methods to CompletionStage!
On Wed, Sep 21, 2016 at 2:38 PM, Pavel Rappo wrote: > On Wed, Sep 21, 2016 at 9:43 PM, Martin Buchholz > wrote: > > What is happening instead is API providers not using CompletionStage as > > return values in public APIs because of the lack of convenient blocking, > and > > instead returning CompletableFuture, which is a tragic software > engineering > > failure. > > On Wed, Sep 21, 2016 at 10:25 PM, Benjamin Manes > wrote: > > I've gradually come to terms using CF as part of an API and haven't > > experienced a downside yet. > > I agree with Benjamin and would like to hear more about how it's a > "tragic software engineering failure". > Obviously I was exaggerating, but the software engineering benefits of separating the producer and consumer users into separate types seems big (like the Builder pattern for immutable collections). Would you use CompletionStage if it had the other methods consumers need?