Channel perf #2751
Channel perf #2751
Conversation
nikiforo
force-pushed
the
channel-perf
branch
from
21b5bb2 to
066253e
Compare
It should be wrapped in an instance of |
I think I have poorly formulated. I believe that combination of Benchmarks show that we might gain 20%+ in this class. The question is: should we consider 20% increase in throughput in this class as an optimization worth exploring? |
|
Yeah, that's very possible. |
| closed: Boolean | ||
| ) | ||
|
|
||
| val initial = State(Vector.empty, 0, None, Vector.empty, false) | ||
| val initial = State(List.empty, 0, None, List.empty, false) |
There was a problem hiding this comment.
| val initial = State(List.empty, 0, None, List.empty, false) | |
| def empty(isClosed: Boolean) = State(List.empty, 0, None, List.empty, false) | |
| val initial = empty(isClosed = false) |
| case prev @ State(values, size, ignorePreviousWaiting @ _, producers, closed) => | ||
| if (shouldEmit(prev)) (State(List.empty, 0, None, List.empty, closed), prev) | ||
| else (State(values, size, waiting.some, producers, closed), prev) |
There was a problem hiding this comment.
| case prev @ State(values, size, ignorePreviousWaiting @ _, producers, closed) => | |
| if (shouldEmit(prev)) (State(List.empty, 0, None, List.empty, closed), prev) | |
| else (State(values, size, waiting.some, producers, closed), prev) | |
| case prev if shouldEmit(prev) => | |
| empty(prev.closed) -> prev | |
| case prev => | |
| prev.copy(waiting = Some(waiting)) -> prev |
|
I've changed Using updated benchmarks I've tested four versions:
Comparison of scores of each of benchmarks with the |
|
@nikiforo Think this is ready for merge? Anyone else you want a review from? |
I think it is. I'm sure that it shouldn't make things worse. For some scenarios it even shows 30% performance increase.
Because I haven't changed the behavior, there is no strong requirement for @SystemFw's review. Yet, I would love to hear his thoughts about the PR. |
benchmark/ jmh:run -i 10 -wi 6 -f1 -t4 -gc true -jvmArgs "-XX:MaxRecursiveInlineLevel=3 -XX:MaxInlineSize=50 -Dcats.effect.tracing.mode=none" fs2.benchmark.ChannelBenchmarkchannel-perfmainNew versions differs from the one from the main in two aspects. Firstly, it replaces
VectorwithListin theState. I don’t think that this change is responsible for the observed performance change.Yet, currently fs2.Chunk doesn’t have a vector-specialized implementation, therefore it’ll anyway be converted to Array.A more specialized implementation reduces the amount of computations required.The second change reduces critical section in the CAS loop. Previously, not only the next state was computed in the loop, but also the old state was transformed into the output. Updated version splits this action into two distinct pieces of work. While we compute new state in CAS, the conversion of the previous state to the emitted chunk is done after the CAS. It reduces contention and eliminates duplicated emitted chunk preparations.