Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: j.u.c APIs #195

Closed
DougLea opened this issue Jan 5, 2015 · 16 comments
Closed

RFC: j.u.c APIs #195

DougLea opened this issue Jan 5, 2015 · 16 comments
Labels

Comments

@DougLea
Copy link
Contributor

DougLea commented Jan 5, 2015

I'm in the process of (tentatively) adding reactive-stream APIs in java.util.concurrent (targeting JDK9). The current somewhat unusual plan is nest all four interfaces inside new class Async, that will also include some other new async support stuff. (None of the standard alternatives (creating new package or adding all four into j.u.c itself) work out very well.

A snapshot of the draft reactive-stream parts of Async.java is temporarily at
http:/gee.cs.oswego.edu/dl/wwwtmp/Async.java

It is written in standard jdk javadoc style, intending to correspond (and cites) the reactive-stream specs. But please let me know about errors or suggestions for improvement.

Also, comments welcome on a draft snapshot of one of the associated upcoming utility classes, BufferedSubmissionPublisher, that can be used as an adaptor for item generators that want to be Publishers.
http:/gee.cs.oswego.edu/dl/wwwtmp/BufferedSubmissionPublisher.java

@ouertani
Copy link
Contributor

ouertani commented Jan 6, 2015

@DougLea :
1- Parameters tag should be add to the inner interface documentation.
2- @FunctionalInterface annotation should be add to Publisher interface
question:
Why Async is not a final class nor interface ?

@DougLea
Copy link
Contributor Author

DougLea commented Jan 6, 2015

@ouertani Thanks! I added these. The missing "final" for Async was just a mistake I made while snapshotting this to get rid of other in-progress stuff.

@abersnaze
Copy link

@DougLea I noticed the addition of an onError(CancellationException) after calling Subscription.cancel(). If every subscription must be canceled wouldn't that cause a lot of additional work to ignore these when operators are trying to unsubscribe?
https://github.com/reactive-streams/reactive-streams#user-content-2.5

@DougLea
Copy link
Contributor Author

DougLea commented Jan 6, 2015

@abersnaze Rule 3.15 says:
Subscription.cancel MUST NOT throw an Exception and MUST signal onError to its Subscriber

You might argue that the spec should relax the overall protocol rules to allow no response. But as stated, I think this is the only allowed reasonable implementation.

@abersnaze
Copy link

I think 3.15 was intended for a situation where an exception occurred during the cleaning up resources during the cancel() not that every cancel() should result in an onError().

@viktorklang
Copy link
Contributor

Yes, @abersnaze is right, look at 3.16, perhaps we should clarify the wording to use the "Invoking Subscription.cancel MUST return normally."

@DougLea
Copy link
Contributor Author

DougLea commented Jan 6, 2015

OK (In some ways it is good to have a year pass between helping write specs and implementing -- at the moment I'm just cross-checking them!). Rule 3.15 should be reworded to say that on normal return, no signals are issued.

@viktorklang
Copy link
Contributor

On Tue, Jan 6, 2015 at 10:36 PM, DougLea notifications@github.com wrote:

OK (In some ways it is good to have a year pass between helping write
specs and implementing -- at the moment I'm just cross-checking them!).

Also, I'm finding again and again that a "rationale"-section for each rule
would be immensely valuable.

Rule 3.15 should be reworded to say that on normal return, no signals are
issued.

I love all improvements to the spec that doesn't change the semantics but
improves the clarity :)


Reply to this email directly or view it on GitHub
#195 (comment)
.

Cheers,

@DougLea
Copy link
Contributor Author

DougLea commented Jan 10, 2015

A few more notes after a touch up pass -- see
http:/gee.cs.oswego.edu/dl/wwwtmp/Async.java
and the (renamed)
http:/gee.cs.oswego.edu/dl/wwwtmp/SubmissionPublisher.java

One difference between jdk specs and r-s specs is that in cases where r-s specs say "MUST NOT X", the jdk versions say "if X, resulting behavior is undefined". These mean almost the same thing, but the latter more clearly allows implementations to try to be helpful in their undefinedness, for example somehow reporting Subscriber exceptions that must not be thrown in compliant implementations, or, as mentioned in the current javadoc, at least auto-cancelling if onNext throws. Which as far as I can see is legal in r-s spec.

@viktorklang
Copy link
Contributor

@DougLea the switch to positive wording (#200) is hopefully merged within a couple of days

@jbrisbin
Copy link

@DougLea I'm curious about the behavior specification itself, as well as the TCK. If we port the Reactor library to use the j.u.c.Flow classes, how will we check our implementation for adherence to the spec?

@smaldini
Copy link
Contributor

Does that mean only JDK 9 libraries (and apps) will be able to be interop ? Can't we have a javax based package that just moves over the reactive-streams jar ?

I am thinking about the interop of the current libraries (not only Reactor or Akka Streams who directly implement the interfaces from RS) but also for the micro libraries popping around the net (Kafka RS, MongoDB driver, Ratpack RS) directly accepting Publisher or Subscriber as input/output. What is the point I am missing to have JDK9 JSR166 RS repackaged ?

@DougLea
Copy link
Contributor Author

DougLea commented Jan 23, 2015

@jbrisbin j.u.c will only provide underlying support for building useful RS components. In principle, people should independently validate Publishers that actually publish items and subscribers that actually process them. But the intent is that all straightforward usages will be guaranteed to validate.

@smaldini Two reasons:
(1) Guaranteeing that anyone using JDK9 can interoperate. This may seem too long from now, but you have to start somewhere.
(2) Without placing these interfaces in the JDK, we cannot provide core library support for them. (Or even JVM support, if it ever came to it).

@viktorklang
Copy link
Contributor

@DougLea Please correct me if I am wrong:

Since j.u.c.Flow are carbon-copies of RS, it is dead simple to map it to RS—we can even provide a compat jar in RS. (Something like "reactive-streams-jdk9plus-compat-1.0.0.final.jar")
If one implements the j.u.c.Flow interfaces and want it to work on pre JDK9, one'll need to put a backport jar on your bootclasspath.

@DougLea
Copy link
Contributor Author

DougLea commented Jan 25, 2015

@viktorklang Yes. People have generally found it easy to replace import startements in analogous cases of moving from jsr166 pre-release/compat packages to new jdk releases.

@viktorklang
Copy link
Contributor

@DougLea Given that no changes are expected in neither spec nor interfaces, I propose to close this issue and presume that the JDK9 version will be verbatim "1.0.0" interfaces + spec.

Please reopen if this is contentious in any way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants