Skip to content

Add FairQueue single-peer fast path#271

Open
sunlei wants to merge 1 commit into
zeromq:masterfrom
sunlei:split-fair-queue-single-peer
Open

Add FairQueue single-peer fast path#271
sunlei wants to merge 1 commit into
zeromq:masterfrom
sunlei:split-fair-queue-single-peer

Conversation

@sunlei

@sunlei sunlei commented May 25, 2026

Copy link
Copy Markdown
Contributor

Summary

This splits the FairQueue change out of #267 for a focused review.

Changes:

  • store a single connected stream outside the multi-peer ready queue path
  • poll that single stream directly while it remains the only stream
  • promote back to the existing fair-queue path when a second stream is inserted
  • preserve the disconnect callback behavior for the single-stream path
  • add regression coverage for single-stream delivery, promotion, and disconnect callback handling

Validation

  • cargo fmt --check
  • cargo test --lib test_fair_queue

Review note

This PR is intentionally separate from the send batching and PUB fanout changes. If the FairQueue risk is not acceptable right now, this PR can stay draft or be dropped without blocking the other split PRs.

- Bypass the ready queue and streams map while only one stream is connected.

- Promote back to the fair queue path when a second stream is inserted.

- Cover single-stream delivery, promotion, and disconnect callback behavior.
@sunlei sunlei marked this pull request as ready for review May 25, 2026 06:07
@Alexei-Kornienko

Copy link
Copy Markdown
Collaborator

Do you have any kind of benchmark data that would confirm that this change actually makes a noticeable improvement in performance? For me it looks like a complication of logic without any clear reason

@sunlei

sunlei commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@Alexei-Kornienko

Thanks, that is a fair concern.

Yes, I measured this before splitting it out from the larger performance branch. The benchmark case where this showed up was the large-payload IPC receive path, especially the one-way benchmarks that avoid mixing receive cost with ROUTER echo-send cost.

The benchmark shape was:

  • DEALER/ROUTER one-way: DEALER sends BATCH_SIZE messages, ROUTER receives BATCH_SIZE messages, no echo/reply
  • PUSH/PULL one-way: PUSH sends BATCH_SIZE messages, PULL receives BATCH_SIZE messages
  • transport: ipc
  • payload: 4096B
  • local run settings included ZMQRS_BENCH_SAMPLE_SIZE=20, ZMQRS_BENCH_MEASUREMENT_MS=2000, ZMQRS_BENCH_WARMUP_MS=500

The local before/after numbers for this specific change were:

workload before after elapsed-time reduction
zmqrs D/R one-way ipc 4096 4.8989 ms 4.1979 ms about 14.3%
zmqrs P/P one-way ipc 4096 5.2582 ms 4.1240 ms about 21.6%

Since the workload size is fixed, the lower elapsed time corresponds to higher receive-side throughput. In throughput terms, those reductions are roughly equivalent to about 16.7% more throughput for D/R one-way IPC 4096B and about 27.5% more throughput for P/P one-way IPC 4096B.

The reason I tried this path is that the one-way D/R and P/P results both pointed at common receive-side cost rather than ROUTER identity or echo-send cost. With a single connected peer, the existing FairQueue path still pays the multi-peer bookkeeping cost on every message: ready queue handling, queued set updates, map remove/insert, and requeueing. This PR avoids that only while there is exactly one stream.

The intended safety boundary is:

  • with one stream, poll it directly
  • when a second stream is inserted, promote both streams back into the existing fair-queue path
  • keep the existing multi-peer polling semantics unchanged
  • preserve disconnect callback behavior

I added tests for those cases:

  • single-stream delivery
  • promotion when a second stream is inserted
  • disconnect callback behavior on the single-stream path
  • the existing FairQueue tests still pass

That said, I agree this is a core scheduling type, so the complexity needs to justify itself. I can add these benchmark numbers to the PR description/comment. If you still think the extra logic is not worth carrying in FairQueue, I am also fine deferring or closing this PR; it is independent from the send/PUB batching PR chain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants