deepjavalibrary / deepjavalibrary/djl-serving

High p50 latency under concurrent streaming due to ForkJoinPool.commonPool() and missing TCP_NODELAY

Open
#3,071 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
253
Forks
96
Avg merge
23h 6m
Merged PRs (30d)
3

Description

### Description

While benchmarking DJL Serving against vLLM directly (same model, same GPU, same prompts, `async_mode=true`), I found DJL's median latency was ~5-7x worse than it should be. Traced it to two separate bottlenecks in the response path:

**1. `InferenceRequestHandler.java` — response completion runs on `ForkJoinPool.commonPool()`**

The streaming response path completes via `.whenCompleteAsync(...)` with no explicit executor, so it defaults to the common pool, which is capped at `availableProcessors() - 1` threads. Under concurrent load this pool is shared with every other unrelated `CompletableFuture` in the JVM, so response dispatch queues up behind whatever else is using it.

Fix: give it a dedicated cached thread pool instead of relying on the default.

**2. `ModelServer.java` — no `TCP_NODELAY` on the server socket**

The Netty `ServerBootstrap` never sets `ChannelOption.TCP_NODELAY`. Streaming responses write small chunks per token, and without `TCP_NODELAY` those can sit behind Nagle's algorithm / delayed ACK, adding latency per chunk.

**3. `python_async_engine.py` — `send_responses()` uses `asyncio.Queue` from a non-async thread**

The response sender thread pushes to an `asyncio.Queue` via `asyncio.run_coroutine_threadsafe(...).result()`, which is a full cross-thread round trip into the event loop for every single token. A plain `queue.Queue` does the same job with a blocking `get()`/`put_nowait()` and no event loop involvement.

### Results

Benchmarked with `meta-llama/Llama-3.2-3B-Instruct`, vLLM `async_mode=true`, 128 concurrent requests, generic public prompt set (not internal data, so this should be reproducible):

| | before | after |
|---|---|---|
| p50 latency | 6.993s | 1.026s |
| throughput | 2559 tok/s | 3743 tok/s |
| p99 latency | 8.025s | 8.698s (no change) |

Same pattern held at 8B scale. p50 and throughput improve a lot; p99 doesn't move, but that turned out to be expected — the remaining tail traces back to vLLM's own scheduler (`max_num_batched_tokens` / `max_num_seqs` / FCFS admission) rather than anything in DJL, since a patched build's p99 already beats vLLM's own standalone p99 at the same concurrency. Mentioning it here just so nobody goes looking for a fourth fix in this codebase for that part.

### How to reproduce

Run the same prompt set through `async_mode=true` before and after the change with concurrency around 100+ and streaming enabled — the effect doesn't really show up below maybe 20-30 concurrent requests since the common pool isn't saturated yet.

### Fix

Have a patch for all three (with unit tests) ready to open as a PR — wanted to open this issue first in case there's context on why the common pool / default queue were chosen that I'm missing, or a preference on how you'd want this split up (Java-only vs. including the Python change).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.