Skip to content

perf: Avoid heap copy of status/integer single-line replies#2860

Open
iliaal wants to merge 1 commit into
phpredis:developfrom
iliaal:perf/status-reply-no-estrndup
Open

perf: Avoid heap copy of status/integer single-line replies#2860
iliaal wants to merge 1 commit into
phpredis:developfrom
iliaal:perf/status-reply-no-estrndup

Conversation

@iliaal

@iliaal iliaal commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

redis_boolean_response and redis_long_response read replies with redis_sock_read, which estrndups the +OK / :N line onto the heap only to inspect one byte (or parse the int) and free it. That wasted an allocation on every SET/INCR/EXPIRE/SADD/DEL/EXISTS. This extracts the dispatch into redis_sock_read_into, which borrows the single-line reply into a caller buffer (no heap) and only allocates for bulk bodies. redis_sock_read stays a thin wrapper, so its other callers are unaffected, and the handlers still drain an unexpected bulk to keep the stream in sync.

Verified on PHP 8.4 across integer, boolean, and error responses, with the bulk path unchanged through the wrapper: no leaks, 8 project suite methods pass. The ownership contract was independently reviewed.

redis_boolean_response and redis_long_response read the reply with
redis_sock_read, which estrndup's the +OK / :N line onto the heap, then
inspect only the first byte (or parse the integer) and efree it. Every
SET, INCR, EXPIRE, SADD, DEL, EXISTS and similar command paid a wasted
allocation.

Extract the shared dispatch into redis_sock_read_into, which writes a
single-line reply into a caller-provided buffer (borrowed, no heap) and
only heap-allocates for bulk bodies. redis_sock_read becomes a thin
wrapper that estrndup's the borrowed line so its existing callers are
unaffected. The two handlers now read into a stack buffer and only free
when the reply was an (unexpected) bulk, which the shared reader still
drains so the stream stays in sync.

Verified on PHP 8.4: status/integer responses (incr/decr/zero/negative,
near INT64_MAX, +OK booleans, :1/:0, error replies via the '-' path),
bulk path unchanged through the wrapper, no leaks under report_memleaks,
and testSetGet/testIncr/testExpire/testExists/testRename/testDel/
testHashes/testKeys.
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