Flat Combining universal construct #313
Conversation
The WFIStack is added to the set of experimental data structures. This data structure has, from experience, proven useful for implementing a number of other concurrency primitives.
- Remove `Thread.yield()` call in spin-wait loop. - Add padding around the `WFIStack.head` field. - Add a push/pop concurrent stress/sanity test for WFIStack.
| if (operation == null) { | ||
| throw new NullPointerException("The operation cannot be null."); | ||
| } | ||
| OpNode op = new OpNode(operation); |
There was a problem hiding this comment.
2 comments:
- I often see combiners (see https://github.com/franz1981/java-puzzles/blob/combiner/src/main/java/red/hat/puzzles/combiner/OptimizedCombiner.java#L150) can use a thread local reusable node to save allocating in the hot path, do you think could worth doing it? Given that execution completion mark the end of the node lifecycle it should make possible to pool it
- I see we poll with FIFO policy, why not leaving the choice of using different data-structures (eg lifo/fifo) to store the ops?
There was a problem hiding this comment.
We could do pooling. An apply method that separates enqueueing and application in time via a Future can't rely on pooling, though. Those operations will have to be unpooled. Perhaps they need to be in a separate stack as well to prevent the pooled stack from growing too big. The lock could be turned from a boolean to an int, and include a sequence so we can periodically truncate the stack and avoid leaking.
Applying the operations in FIFO order is how the linearisable consistency is achieved. Would be easy to add an option to turn that off.
There was a problem hiding this comment.
Thinking about the consistency... since concurrent blocking apply calls overlap in time, I think we can pick an arbitrary ordering of these overlapping operations, and still be considered linearisable. The operations were already racing, so there's no externally observable sequence of events that can disagree with applying the operations in LIFO order.
Pull Request Test Coverage Report for Build 692
馃挍 - Coveralls |
|
|
||
| for (; ; ) { | ||
| if (tryLock()) { | ||
| Iterable<OpNode> nodes = combineAndApplyAllOperations(); |
There was a problem hiding this comment.
Although counter-intuitive I think that would be nice to have a version of combineAndApplyAllOperations that allow to limit the amount of work to be performed (the simpler way in the max number of combined operations, but can think to other metrics, or by using a cooperative suspention of some form eg ControlledRunnable?) to allow other(s) Threads to pick remaining jobs (if the combiner thread isn't alone).
There was a problem hiding this comment.
Thinking it twice probably the limit in the combined list of pending ops work only if the list is FIFO. Need to think about it better
| lock.set(false); | ||
| OpNode peek = ops.peek(); | ||
| if (peek != null) { | ||
| peek.unpark(); |
There was a problem hiding this comment.
I would add a comment to explain how much is important to do this :)
This PR is using #312 as a base line. Work harder, @nitsanw 馃槢
This is not yet ready to merge, I think. But it is ready for discussing the API. I'm on the fence about using type parameters for exceptions. Perhaps there should be more methods and operation types with different signatures that would make Java 8 method references more easily align with common signature patterns seen in the Java collections framework. It's also possible, I think, to separate the enqueueing from the execution, and have an apply method that returns a Future.