belliottsmith commented on code in PR #10:
URL: https://github.com/apache/cassandra-accord/pull/10#discussion_r989460013
##########
accord-core/src/main/java/accord/messages/Apply.java:
##########
@@ -54,15 +63,59 @@ public Apply(Node.Id to, Topologies topologies, TxnId
txnId, Txn txn, Key homeKe
this.result = result;
}
+ @VisibleForImplementation
+ public Apply(Keys scope, long waitForEpoch, TxnId txnId, Txn txn, Key
homeKey, Timestamp executeAt, Deps deps, Writes writes, Result result)
+ {
+ super(scope, waitForEpoch);
+ this.txnId = txnId;
+ this.txn = txn;
+ this.homeKey = homeKey;
+ this.executeAt = executeAt;
+ this.deps = deps;
+ this.writes = writes;
+ this.result = result;
+ }
+
+ static Future<Void> waitAndReduce(Future<Void> left, Future<Void> right)
Review Comment:
> So that would penalize futures an additional cas instruction when there is
more than one future to listen to
Three, I think? One to set the `Future` result, one to register a listener,
and one to notify the listeners. I am playing with a `Future` implementation
that could at least downgrade that to a couple of volatile writes in the common
case though.
> With callbacks, this separation does not exist.
Could you give me an example, as I'm not seeing it? The callback is
explicitly the responsibility _only_ of the producer of the outcome, it's just
flipping the interface but otherwise is isomorphic. While the `Future` is
passed back to the caller, the callback is only passed to the producer. The
main benefit of flipping this is that we declare our receiver before the work
begins, so we don't have any race condition where the work may have completed
before the next step is registered; and by not maintaining a reference to the
outcome we save some additional work and memory.
> I think we should upgrade to Futures as soon as the callback needs to be
passed around to areas of the program not responsible for completing it.
I think this is the source of our disconnect. Where would this happen? In my
mind, the callback is never passed to such areas - the callback may be a
conduit to propagate that value elsewhere, but it should only ever be the glue
(propagating it to some other object/recipient), and should never itself escape
the scope of the method that declares it...
I agree that there are cases where it is convenient to return some
encapsulation of the result. But `Future` is really for synchronous consumers
of a result, that want to invoke `get` - as async users really just use it as a
conduit for a callback. Some method nesting may make it easier to pass back a
`Future` to attach this to rather than passing down a callback, but I think it
is probably not all that commonly essential. Most of the time the same chain of
`return` types that yields a `Future` can be transformed into an extra
parameter that accepts a callback.
I actually think `Future` will become much more useful again when virtual
threads are a thing, as synchronous programming is fundamentally easier. We're
already avoiding the OG pattern for `Future` for even bigger perf reasons (of
thread management). I think in future we'll go back to submitting the work and
invoking `get`. But today they're sort of a bodge of asynchronous on-top of a
synchronous API, IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
