Skip to content

[Benchmark] Share one ClientSession across benchmark requests - #8114

Closed
kevincheng2 wants to merge 1 commit into
PaddlePaddle:developfrom
kevincheng2:bench/shared-client-session
Closed

[Benchmark] Share one ClientSession across benchmark requests#8114
kevincheng2 wants to merge 1 commit into
PaddlePaddle:developfrom
kevincheng2:bench/shared-client-session

Conversation

@kevincheng2

@kevincheng2 kevincheng2 commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Motivation

async_request_eb_openai_chat_completions already accepts an optional session, but benchmark_serving never passes one, so every request opens a new aiohttp.ClientSession and therefore a new TCP connection. On long runs (--num-prompts in the tens of thousands) the accumulated TIME_WAIT sockets can exhaust local ports and perturb the latency being measured.

Modifications

Create one ClientSession with a bounded connection pool in benchmark() and hand it to every request when the backend request function accepts a session parameter, detected via inspect.signature. Backends without that parameter are called exactly as before. The session is closed after asyncio.gather completes.

This only touches the benchmark client. No engine, kernel or model code is changed.

Usage or Command

No new flags. Run the benchmark as before:

python benchmarks/benchmark_serving.py \
    --backend openai-chat \
    --model <model> \
    --endpoint /v1/chat/completions \
    --dataset-name EBChat \
    --dataset-path <dataset> \
    --num-prompts 20000 \
    --max-concurrency 64

The connection pool is sized at max_concurrency * 2 (unbounded when --max-concurrency is not given). Socket accumulation can be observed with ss -tan state time-wait | wc -l during a long run.

Accuracy Tests

Not applicable — this PR does not affect model outputs. It changes only connection reuse in the benchmark client.

pre-commit run passes on the changed file. Backends whose request function has no session parameter keep their previous call signature, so their behavior is unchanged by construction.

Checklist

  • Add at least a tag in the PR title. ([Benchmark])
  • Format your code, run pre-commit before commit.
  • Add unit tests. — the benchmark scripts under benchmarks/ have no unit test harness in this repo; the change is guarded by an inspect.signature check so unsupported backends are called exactly as before.
  • Provide accuracy results. — not applicable, see above.
  • If the current PR is submitting to the release branch... — targets develop.

async_request_eb_openai_chat_completions already accepts an optional
session, but benchmark_serving never passes one, so every request opens a
new aiohttp ClientSession and therefore a new TCP connection. On long runs
(--num-prompts in the tens of thousands) the accumulated TIME_WAIT sockets
can exhaust local ports and perturb the latency being measured.

Create one ClientSession with a bounded connection pool and hand it to
every request when the backend function accepts a session parameter, so
backends without that parameter keep working unchanged.
@kevincheng2
kevincheng2 force-pushed the bench/shared-client-session branch from 8d14090 to b3f5aff Compare August 24, 2026 08:38
@kevincheng2

Copy link
Copy Markdown
Collaborator Author

/re-run all-failed

@kevincheng2

Copy link
Copy Markdown
Collaborator Author

Closing this — the connection reuse change is not needed for now. The decode-speed measurement fix is tracked separately in #8113.

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.

1 participant