dcapwell commented on code in PR #17: URL: https://github.com/apache/cassandra-accord/pull/17#discussion_r1052797144
########## accord-core/src/main/java/accord/utils/async/AsyncResult.java: ########## @@ -0,0 +1,96 @@ +package accord.utils.async; + +import java.util.concurrent.Executor; +import java.util.function.BiConsumer; + +/** + * Handle for async computations that supports multiple listeners and registering + * listeners after the computation has started + */ +public interface AsyncResult<V> Review Comment: > There’s no easy way to impose a “no more than one hop” restriction. Based off usage, we don't have more than 1 hop... so are we trying to optimize for a case that we do not have? I am looking at both accord-core and the C* integration branch, and every call to `toChain` yields a single hop. We even have fun patterns such as this ``` private AsyncChain<Void> applyChain(SafeCommandStore safeStore, boolean canReschedule) ... return applyWithCorrectScope(safeStore.commandStore()).toChain(); // usage private AsyncResult<Void> applyWithCorrectScope(CommandStore unsafeStore) ... AsyncChain<Void> chain = super.applyChain(safeStore); result = AsyncResults.forChain(chain); ``` So, because of this, we go from `AsyncResult` to `AsyncChain` back to `AsyncResult`, costing more than just passing around the single `AsyncResult` (and require more cas/volatile overhead); this pattern happens several times... Given we don't have multiple hops this argument doesn't hold for me; what are we really trying to solve here? > it’s a lot of code duplication; Is it? Looking at my feedback branch and `AsyncChains` I really don't see duplicate code... they have duplicate signatures but not bodies. -- 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]
