Better coalescing supplier #4508
Conversation
This is a moderate improvement on the present coalescing supplier. Won't describe present state of the world, but the new implementation hopefully describes the algorithm better. For each round, either we are the first to arrive (execute and return) or we are not. In the case we are not, we await the current round ending and then perform the check again. If not this time, we wait for the executor to finish and return their result. The improvement is modest - with 16 threads looping and a task that takes 2ms (the benchmark) we see throughput of 6886 +- 73 operations per second. With this change, we see a result of 7232 +- 89 operations per second. While the change is minimal, the result is closer to optimal; 16 / 0.002 = 8000 as perfect (which we can never really achieve in such a benchmark).
Generate changelog in
|
…ir/atlasdb into jbaker/faster_coalescing_supplier
| private final class Round { | ||
| private final AtomicBoolean hasStarted = new AtomicBoolean(false); | ||
| private final CompletableFuture<T> future = new CompletableFuture<>(); | ||
| private volatile Round next; |
There was a problem hiding this comment.
it's unclear to me based on Java semantics whether this needs to be volatile or not. But I'm gonna leave it as such.
There was a problem hiding this comment.
I think not, since completing a future does happen before a join on that future. Not 100% confident in this one so agree with leaving it as such.
| private volatile Round next; | ||
|
|
||
| boolean isFirstToArrive() { | ||
| return !hasStarted.get() && hasStarted.compareAndSet(false, true); |
There was a problem hiding this comment.
This is much faster than the equivalent 'hasStarted.compareAndSet(false, true)'. I believe this is because the proposed solution can do the read in a MESI shared state, whereas the CAS will always make it exclusive.
There was a problem hiding this comment.
Makes sense. I think we should document this in a comment here.
…ir/atlasdb into jbaker/faster_coalescing_supplier
| try { | ||
| future.join(); | ||
| } catch (CompletionException e) { | ||
| // ignore |
There was a problem hiding this comment.
why are we eating this and not at least logging at lower level?
There was a problem hiding this comment.
it's intended behaviour - we're literally just awaiting the conclusion of the future
There was a problem hiding this comment.
we just want it to be complete. We shouldn't do anything with the exception because it's never relevant in this method.
There was a problem hiding this comment.
Generally makes sense. I think the build is currently failing as this needs to be rebased on top of #4505
| private final class Round { | ||
| private final AtomicBoolean hasStarted = new AtomicBoolean(false); | ||
| private final CompletableFuture<T> future = new CompletableFuture<>(); | ||
| private volatile Round next; |
There was a problem hiding this comment.
I think not, since completing a future does happen before a join on that future. Not 100% confident in this one so agree with leaving it as such.
| private volatile Round next; | ||
|
|
||
| boolean isFirstToArrive() { | ||
| return !hasStarted.get() && hasStarted.compareAndSet(false, true); |
There was a problem hiding this comment.
Makes sense. I think we should document this in a comment here.
…ir/atlasdb into jbaker/faster_coalescing_supplier
delete-merged-branch
bot
deleted the
jbaker/faster_coalescing_supplier
branch
|
Released 0.179.1 |
This is a more readable version of the present coalescing supplier. Won't describe present state of world (algorithm is confusing) but the new implementation is easier to understand, and in some cases demonstrates better performance.
For each round, either we are the first to arrive (execute and return)
or we are not.
In the case we are not, we await the current round ending and then
perform the check again. If not this time, we wait for the executor to
finish and return their result.
The improvement is hard to measure in synthetic benchmarks. With spinning threads it's actually slower than the present day (due to the fair queue, the present code maximizes the likelihood that the batch will be full, whereas the new code will likely ensure that the completing task starts the next batch.
With some random noise delays added to smooth out this effect, performance is very similar to at present, possibly slightly less variance. It's possible to tweak any benchmark by adding extraneous loading.