deepjavalibrary / deepjavalibrary/djl-serving
High p50 latency under concurrent streaming due to ForkJoinPool.commonPool() and missing TCP_NODELAY
- 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
Assessment
This issue has not been assessed yet.