Skip to content

Add support for PHP resp-bench - #28

Open
prateek-kumar-improving wants to merge 4 commits into
mainfrom
php/resp-bench
Open

prateek-kumar-improving wants to merge 4 commits into
mainfrom
php/resp-bench

Conversation

@prateek-kumar-improving

@prateek-kumar-improving prateek-kumar-improving commented Sep 14, 2026

Copy link
Copy Markdown

Implements the PHP benchmark engine, at parity with the Java (reference), Ruby, and C# engines.

What

  • New engine under php/ (composer install, php bin/resp-bench), driving:
    • valkey-glide-php — the Valkey GLIDE PHP client (native ext-valkey_glide extension, PHPRedis-compatible API)
    • phpredis — PHPRedis (ext-redis), the de-facto standard PHP client and the incumbent comparison baseline (analogous to redis-rb for Ruby / StackExchange.Redis for C#)
    • recording — in-memory driver for server-free tests
  • Consumes the shared driver/workload JSON and emits the exact NDJSON schema (metadata/phase/totals/metrics, unit:"us", uppercased command keys, HDR compressed base64).

Concurrency model

PHP's GLIDE client is synchronous and the standard PHP build is non-thread-safe (NTS), which rules out ext-parallel. The engine therefore uses process-per-connection via pcntl — the faithful, dependency-free analogue of Java's virtual-thread-per-client and Ruby's thread-per-client:

  • connections = N → fork N worker processes (capped at 256), each with one in-flight request at a time (the client == connection invariant), keeping results comparable across engines.
  • Each worker connects after forking — a connection is never inherited across a fork (required for correctness with the native extension).
  • Workers stream partial metrics to the parent over a stream_socket_pair; the parent reconstructs and merges the HdrHistograms losslessly (sparse bucket counts) before writing NDJSON.
  • Phase-level rps_limit is divided across workers so the aggregate matches the target.
  • An inline mode (--concurrency inline) runs all connections sequentially in one process; it's selected automatically for the recording driver and when pcntl is unavailable, and exercises the full pipeline for server-free tests.

Cross-engine parity

  • JavaRandom LCG port — verified byte-identical to Java's canonical new Random(0).nextInt() sequence [-1155484576, -723955400, 1033096058, -1690734402, -1557280266, ...]. PHP lacks 64-bit integer overflow wraparound, so the multiply uses a 24-bit split to preserve the low 48 bits.
  • sequential_int walks the keyspace; uniform_rand seeds per worker (seed + idx) for reproducible-yet-distinct sequences. Key formatting %0Nd honoring key_size_bytes.
  • Pure-PHP HdrHistogram (1, 600_000_000, 3); the V2 compressed base64 payload is byte-structured to match Java's encodeIntoCompressedByteBuffer(). Leaky-bucket rate limiter.

Harness

  • Makefile php-build/test/integration-test/run/clean/info targets.
  • Registered the PHP driver_id in scripts/run_benchmark_matrix.py (DRIVER_ENGINE_MAP) and scripts/generate_graphs.py (DRIVER_LANGUAGE_MAP).
  • Driver configs for both clients under configs/drivers/{default,high-throughput}/ + example-*-standalone.json, plus configs/matrices/php-driver-comparison.json (GLIDE-PHP vs PHPRedis across connection counts — the head-to-head that mirrors the Java/Node comparison matrices).
  • CI: a fast server-free test-php job (unit + integration) and a benchmark-php job that builds the valkey_glide extension from source and runs against a live server; wired into generate-graphs needs.
  • Docs: php/README.md, docs/BENCHMARKS_PHP.md, README "Supported Languages" + Make Targets updates.

Tests

58 tests (cd php && vendor/bin/phpunit): parity anchors (JavaRandom seed-0 Java anchor, key generator), rate limiter, command selector, config loader, client factory, HDR encode structure + percentiles; integration tests covering the recording-driver pipeline (NDJSON schema, inline + process fork modes, agreement on totals), rate limiting (RPS enforcement, shared limit across concurrent connections, unlimited throughput), error metrics (per-command error counts, errors excluded from latency histogram), and NDJSON/metrics output. Gated live tests for both valkey-glide-php and phpredis skip cleanly without their extension + a server. No server needed for the 50 non-live tests.

Verified locally (PHP 8.5, no server, no extension): full suite = 58 tests pass, 8 skipped (the gated live tests for the two real drivers). CLI smoke test with the recording driver produces valid NDJSON with correct totals (WARMUP 400, STEADY 1000 = GET+SET) and non-empty HDR payloads in both inline and --concurrency process (fork) modes.

Notes

  • valkey-glide-php follows the valkey-glide-ruby / valkey-glide-csharp naming convention (not the bare valkey-glide, which is Java's).
  • Live-server runs require the valkey_glide extension installed (php/README.md documents install via pie/PECL/source; the benchmark-php CI job builds it). The recording-driver e2e covers the engine paths without it.
  • The HDR payload's binary structure was verified (cookies, header fields, IEEE754 conversion ratio), but a live cross-decode against Java's decodeFromCompressedByteBuffer() was not run in this environment (no JRE); it's ported from the Java-validated Ruby encoder.

Signed-off-by: Kumar <kupratec@amazon.com>
Signed-off-by: Kumar <kupratec@amazon.com>
Signed-off-by: Kumar <kupratec@amazon.com>
Resolve conflicts from the merged Node.js engine (#27), which touched the same
shared files as the PHP engine PR. All resolutions are additive — both engines
coexist:

- scripts/{run_benchmark_matrix,generate_graphs}.py: keep php + node driver maps
- Makefile: keep both PHP and Node engine target sections and .PHONY entries
- README.md: keep PHP and Node rows in languages/structure/target tables
- .github/workflows/benchmark.yml: keep test-php + benchmark-php and benchmark-node
  jobs; add 'php' to the engines input; generate-graphs needs all four engines

@jamesx-improving jamesx-improving left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three blocking measurement bugs, each reproduced by running the engine — details inline.

The worst is that the HDR payload_b64 cannot be decoded at all. V2 payload_length is the counts-array length only; HdrEncoder.php:71 adds the 32 bytes of header fields that follow it, so org.hdrhistogram:HdrHistogram:2.2.2 throws IllegalArgumentException: The buffer does not contain the indicated payload amount. I confirmed this by running this PR's own php/tools/hdr-crosscheck — the gate its README calls "the definitive cross-language latency parity gate". With $payloadLen = strlen($countsBytes); Java decodes it cleanly and count plus all five percentiles match exactly. Worth wiring that tool into test-php, since the workflow already provisions Java for benchmark-java.

The other two are silent by construction: every forked worker restarts sequential_int at 0, so at connections: 16 the reference workload's WARMUP populates 62,500 of 1,000,000 keys (6.25%) and STEADY then misses ~94% of its GETs — cheaper server-side, counted as success, errors: 0. And a phase in which every worker dies is written out as status: COMPLETED with errors: 0 and exit code 0.

Net effect: PHP results are not comparable to the Java/C#/Ruby/Node engines today. Six Medium findings follow inline.

On validation — no CI has run here, and I don't think the PR should read as if it had:

  • Zero check runs and zero workflow runs exist on 36df6349.
  • .github/workflows/benchmark.yml is the repo's only workflow and is workflow_dispatch-only, so the new test-php job cannot fire on a pull request. The 58 passing tests are a local result.
  • benchmark-php runs basic-standalone-single-client-1M-reqs.json, which is connections: 1 — the one setting in which findings 2, 5 and 8 are all invisible. Even a green run would not have caught them.

This is a repo-wide gap (Java/Ruby/Node have no PR-triggered lane either), not something this PR introduced.


// payload_len = everything after the payload_len field itself:
// normalizing(4) + sigfigs(4) + lowest(8) + highest(8) + ratio(8) + counts
$payloadLen = 4 + 4 + 8 + 8 + 8 + strlen($countsBytes);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java defines V2 payload_length as the counts-array length only — buffer.position() - payloadStartPosition, measured after the 40-byte header. Adding the 32 bytes of header fields here over-declares it, and Java refuses to decode: running this PR's own tools/hdr-crosscheck against org.hdrhistogram:HdrHistogram:2.2.2 throws IllegalArgumentException: The buffer does not contain the indicated payload amount.

So every payload_b64 this engine emits is unusable by the Java/Ruby/C# tooling — invisibly, because summary is computed in PHP and looks fine. Archived results would need a re-run, not a re-parse.

Suggested fix: $payloadLen = strlen($countsBytes); — with that one change Java decodes the payload and count plus all five percentiles match exactly. The rest of the encoder (cookies, header fields, ZigZag/LEB128 counts, zlib wrapper) is correct.

HdrEncoderTest.php:58 unpacks plen and then never asserts on it; asserting plen === strlen($countsBytes) would have caught this.

return KeyGenerator::createWithSeed($keyspace, $keyspace->seedValue() + $workerIndex);
}

return KeyGenerator::create($keyspace);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each worker gets a fresh generator whose counter starts at 0, and workerRequestTarget also pre-divides the budget — so all N workers walk the same prefix of the keyspace instead of interleaving across it.

On the reference workload this PR's own CI job and matrix both use, that means WARMUP populates 1/N of the keyspace. Measured with the real classes:

  • connections: 1 → 1,000,000 / 1,000,000 keys (100%)
  • connections: 16 → 62,500 / 1,000,000 keys (6.25%)

STEADY then issues uniform_rand GETs across the full keys_count: 1000000, so ~94% of them miss keys that were never written. A miss is cheaper server-side than returning 512B and is a perfectly successful GET, so it lands as a success with a real latency: high throughput, low GET latency, errors: 0. The matrix sweeps [1, 2, 4, 8, 16], so the PHP curve diverges from the other engines further right on the x-axis — the axis the matrix exists to compare.

Suggested fix: Partition deterministically instead of duplicating — worker i strides i, i+N, i+2N, …, or takes a contiguous i * total/N block. That reproduces Java's key set under process isolation even though the interleaving order differs.

Note KeyGenerator.php:22-23 already documents "disjoint, deterministic starting offsets by the engine (see Benchmark)", but no offset parameter exists anywhere — createWithSeed only sets the RNG seed, which sequential_int never reads.


foreach ($this->workloadConfig->phases as $phase) {
$collector = $this->runPhase($phase);
$writer->writePhaseResults($phase->id, 'COMPLETED', $phase->connections, $collector);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status is a hardcoded 'COMPLETED', and no worker failure can reach it: a child that throws before exit(0) writes nothing, the parent treats an empty payload as "nothing to merge" at :132, and pcntl_waitpid's $status at :141 is written and never read.

I ran a 4-connection phase where every worker failed at connect. The NDJSON recorded status: COMPLETED, requests: 0, errors: 0, and the process exited 0 — so run_benchmark_matrix.py:1067 books the cell as successful and a zero-request phase enters the results set as valid data.

Partial failure is worse than the total case: if 3 of 16 workers die, the surviving 13 report normally, totals are silently short by 3/16, duration_ms still spans the whole phase, and the result is a plausible RPS number that is quietly ~19% low with nothing marking it suspect. Java returns "ERROR" here (BenchmarkEngine.java:326-329).

Suggested fix: Report failures from the child, check exit status per child in the parent, and thread a real status through to writePhaseResults and the CLI exit code.

Separately: metrics serialises as [] rather than {} when empty, so generate_graphs.py:311 and generate_interactive_graphs.py:293 both call .items() on a list and die with AttributeError. JSON_FORCE_OBJECT (or an (object) cast) fixes that.

private function runPhase(PhaseConfig $phase): Collector
{
$collector = new Collector();
$collector->start();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The collector clock starts before the fork, so duration_ms covers N pcntl_fork calls, N client connects and the whole warmup on top of the workload. All four reference engines deliberately start it after both — Java :263, C# :172, Ruby :388 ("Submit warmup requests BEFORE starting metrics"), Node :168.

That matters because duration_ms is the denominator of the headline throughput in both consumers (generate_graphs.py:307, generate_interactive_graphs.py:276, each computing totals.requests / (duration_ms / 1000)). Inflating it understates PHP's RPS by an amount that grows with connection count, so it bends the scalability curve rather than shifting it uniformly.

Suggested fix: Start the clock after connect and warmup. For the fork path, have workers report their own measured start/stop and take min/max in the parent — Collector already exposes setStartTime/setEndTime.

Two related notes: runWarmup at :225 issues weighted workload commands where every other engine sends PING, and because those consume from the shared $keyGen they also shift the measured key sequence. And NdjsonWriter::iso8601 truncates the phase timestamps to whole seconds, so the true window can't be recovered from them either.

$keyGen = $this->keyGeneratorForWorker($phase->keyspace, $workerIndex);

// Divide the phase-level rate limit across workers.
$rps = $phase->hasRpsLimit() ? max(1, intdiv($phase->rpsLimit, $workerCount)) : -1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The max(1, …) floor converts a truncated-to-zero share into 1 rps per worker, which multiplies straight back up: rps_limit: 50 with connections: 64 gives an aggregate of 64 rps against a target of 50, and the overshoot grows with connection count.

Truncation alone also breaches the checklist's 5% tolerance well before the floor engages — rps_limit: 100 at connections: 64 is 1 rps each, i.e. 64 rps (36% under).

RateLimitingTest::testSharedRpsLimitAcrossConnections won't catch it: rps=50, connections=4 → 12 each → 48 aggregate, 4% under, inside its own 10% tolerance. Nothing exercises rps_limit < connections.

Suggested fix: Distribute the remainder rather than flooring — the same pattern already used for the warmup and budget splits at :219-222 and :239-243 — and let a worker's share be genuinely 0 (no limiter, no requests) instead of silently 1.

while (true) {
$bits = $this->nextBits(31);
$val = $bits % $bound;
if ($bits - $val + ($bound - 1) >= 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java's rejection test relies on signed 32-bit overflow to be reachable at all. PHP ints are 64-bit, so this expression is never negative, the condition is always true, and the branch never fires — from the first draw Java would have rejected, the two LCG streams are permanently out of step.

Measured at the reference workload's keys_count: 1000000: 209 draws per 1,000,000 that Java rejects and this accepts, the first at key #8907, after which 99.1% of the 1M keys differ from Java's sequence.

Aggregate latency and throughput are unaffected (the draws are still uniform) — what's lost is the documented property that a given seed yields the same key sequence in every engine.

Suggested fix: Wrap to int32 before comparing, as the Node port does via toInt32: if ($bits - $val + ($bound - 1) <= 2147483647) return $val;

The seed-0 anchor test can't detect this: at nextInt(1000) the branch fires roughly once in 5,000,000 draws (measured) and the test makes 10. The LCG core itself, including the 24-bit split multiply, is correct. Worth noting javaRandom.ts calls this out explicitly — it emulates the wraparound "so it matches the Java reference rather than the Ruby approximation", and this is a line-for-line copy of the Ruby version.


public function max(): int
{
return $this->maxValue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java's getMaxValue()/getMinValue() return the bucket's equivalent bounds, not the raw samples. On the crosscheck's sample set PHP reports max: 123456 where Java reports 123519, and that tool's README requires count, min and max to match exactly — so the gate still fails on max even after the payload_length fix.

merge() compounds it by re-recording each bucket at valueFromIndex(), the bucket floor, so in process mode — where the parent's histogram is built entirely from merged partials — the reported max is the floor while Java reports the ceiling.

Suggested fix: Return valueAtPercentile(0) / valueAtPercentile(100), and follow Java in returning 0 for min when the zero bucket is non-empty. valueAtPercentile already applies highestEquivalentValue, so p50–p999 are consistent and only these two drift.

return 'inline';
}

return function_exists('pcntl_fork') ? 'process' : 'inline';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This falls back to inline, which runs workers serially, while phase.connections is still written as the configured N — so the output is indistinguishable from a real N-connection run. No warning is logged.

On a host where pcntl is absent or disabled (a routine disable_functions entry), a request-based phase at connections: 16 issues everything serially: requests and wall time scale together, so the reported RPS is single-connection throughput labelled connections: 16, with no error and exit 0. A duration-based phase is worse — each of the 16 workers runs the full durationSeconds, so the phase takes 16× as long as configured and still reports one connection's throughput.

Suggested fix: For a non-recording driver with connections > 1, fail with a message pointing at pcntl rather than emitting a number that looks valid. If the fallback must stay usable, record the effective mode in the NDJSON metadata. Keeping inline for recording and for connections == 1 is fine.

return $this->rpsLimit > 0;
}

public function effectivePipelineDepth(): int

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

effectivePipelineDepth() and hasCpsLimit() have no callers in the engine. Outside src/Config/, the only references anywhere in php/ are two assertions in ConfigLoaderTest.php:98,101; hasCpsLimit and pipelineDepth have zero. Java honours both — a CPS limiter gates connection creation in createClients, and pipeline_depth > 1 selects runPipelinedLoop.

Latent today, which is why it's last: no shipped config sets pipeline_depth (it appears only in the schema) and every cps_limit in configs/ is -1. It becomes a live comparability bug the first time a workload sets pipeline_depth: 8 — Java keeps 8 in flight per connection, PHP silently runs at depth 1, and the gap gets attributed to the client library. The NDJSON records nothing about the depth actually used.

Suggested fix: Honour them, or reject a config that sets a knob this engine ignores, so a non-comparable run fails loudly instead of producing a number. command_timeout_ms (set to 10000 in configs/drivers/high-throughput/valkey-glide-node.json) is likewise unread by the PHP clients.

private function keyGeneratorForWorker(KeyspaceConfig $keyspace, int $workerIndex): KeyGenerator
{
if ($keyspace->isUniformRand()) {
return KeyGenerator::createWithSeed($keyspace, $keyspace->seedValue() + $workerIndex);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every worker writes the same key sequence, so a run populates keys_count / connections distinct keys rather than keys_count.

keyGeneratorForWorker assigns a per-worker offset only on the uniform_rand path; the sequential_int path gets KeyGenerator::create, whose $sequentialCounter starts at 0, and workers are separate processes. Java's KeyGenerator.forkForThread passes the parent's AtomicLong into the private constructor (this.sequentialCounter = sharedCounter; // Share counter across threads), so N threads walk 0..total-1 without duplicates. The KeyGenerator class docstring states that "workers are assigned disjoint, deterministic starting offsets by the engine (see Benchmark)".

800 requests across 8 workers, PHP 8.5.7:

PHP   100 distinct keys, each written 8 times
Java  800 distinct keys

configs/matrices/php-driver-comparison.json sweeps connections 1 to 16 over basic-standalone-single-client-1M-reqs.json, whose STEADY phase is 80% uniform_rand GET over keys_count: 1000000:

  • 1 connection — 1,000,000 keys populated by WARMUP, 0% GET miss
  • 4 connections — 250,000 populated, 75% miss
  • 16 connections — 62,500 populated, 93.75% miss

A nil GET reads no value and no 512-byte response, so the miss rate lands in latency and throughput, and it rises with connections along the x-axis of the chart that matrix produces.


foreach ($this->workloadConfig->phases as $phase) {
$collector = $this->runPhase($phase);
$writer->writePhaseResults($phase->id, 'COMPLETED', $phase->connections, $collector);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A phase reports "status":"COMPLETED" when every worker died.

run passes the literal 'COMPLETED' to writePhaseResults, the $status that pcntl_waitpid fills in is never read, and the drain loop continues past an empty payload. Java's executePhase returns "ERROR" when a worker throws.

All 8 workers exited on PHP fatal errors and the parent wrote a well-formed line (PHP 8.5.7):

{"phase":{"status":"COMPLETED","duration_ms":381},"totals":{"requests":0,"errors":0},"metrics":[]}

The partial case carries further: 3 of 16 workers dying mid-run publishes 13/16 of the traffic as COMPLETED, and no field in the NDJSON separates that from a full run.


// payload_len = everything after the payload_len field itself:
// normalizing(4) + sigfigs(4) + lowest(8) + highest(8) + ratio(8) + counts
$payloadLen = 4 + 4 + 8 + 8 + 8 + strlen($countsBytes);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

payload_b64 cannot be decoded by HdrHistogram — the V2 payload_length field counts the 32 header bytes that follow it.

AbstractHistogram.encodeIntoByteBuffer writes the counts length alone:

buffer.putInt(initialPosition + 4, buffer.position() - payloadStartPosition);

This PR's own php/tools/hdr-crosscheck/HdrCrossCheck.java against org.hdrhistogram:HdrHistogram:2.2.2, PHP 8.5.7:

java.lang.IllegalArgumentException: The buffer does not contain the indicated payload amount
    at org.HdrHistogram.AbstractHistogram.decodeFromByteBuffer(AbstractHistogram.java:2161)
$payloadLen = 4 + 4 + 8 + 8 + 8 + strlen($countsBytes);  ->  IllegalArgumentException
$payloadLen = strlen($countsBytes);                      ->  p50=503 p90=905 p95=955 p99=995 p999=50015, matching Java

The rest of the V2 structure decodes: both cookies including the 0x10 word-size flag, normalizing offset, sigfigs, the int64 lowest/highest fields, the IEEE754 conversion ratio, and the ZigZag-LEB128 counts with negative zero-runs.

The same expression is in ruby/lib/resp_bench/metrics/hdr_histogram_encoder.rb. Ruby's decoder reads counts from byte 40 to the end and ignores the field, so its round-trip test passes.

private function runPhase(PhaseConfig $phase): Collector
{
$collector = new Collector();
$collector->start();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A phase's reported duration_ms includes fork, per-worker connect, warmup, IPC, and reap.

runPhase calls Collector::start() before the fork loop and stop() after the drain loop and pcntl_waitpid. Java's executePhase calls metrics.start() after createClients() and warmupClients() have returned, and stop() in the finally immediately after the workers finish. generate_graphs.py derives total_rps as totals.requests / (duration_ms / 1000).

8 workers x 2.000s of steady state, recording driver, PHP 8.5.7:

true aggregate  12,730,932 / 2.000 = 6,365,466 rps
reported        12,730,932 / 2.179 = 5,842,557 rps

With a real driver the N connects fall inside the window too, and that overhead grows with connections.


// --- Inline execution (fallback / recording) ---------------------------

private function runPhaseInline(PhaseConfig $phase, int $workerCount, Collector $collector): void

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In inline mode a duration phase runs N times as long as configured and reports 1/connections of actual throughput.

runPhaseInline runs workers sequentially and each honours the full duration_seconds, while the merged collector's elapsed time spans all N runs. Java runs its clients concurrently for one duration_seconds.

Configured 2s / 8 connections, recording driver, PHP 8.5.7:

wall clock     16.24s   (configured 2s)
reported      959,764 rps
true        7,791,361 rps

mode() selects inline when pcntl_fork is absent and when --concurrency holds any string other than exactly process; Cli passes that option through unvalidated, so --concurrency proccess runs inline. No field in the NDJSON distinguishes the two modes.

private array $store = [];

/** @var list<array{op:string,key:string,size:int}> */
private array $recorded = [];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recording driver grows by 440 bytes per request and dies at roughly 300k requests under the default memory_limit.

$recorded takes one array per ping, get, set, and del, nothing drains it, and recordedOperations() is read only by tests. It is the one piece of per-request state not folded into the histogram.

Through Benchmark::run with this PR's smoke-workload.json, recording driver, 4 connections, PHP 8.5.7:

 50k requests   8 MB
200k requests  24 MB
400k requests  46 MB

With memory_limit=128M on a duration-based phase:

PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 20480 bytes)
  in .../Client/Impl/RecordingClient.php on line 78

mode() routes the recording driver to inline, so one process holds one RecordingClient for the whole run. The parent then writes "status":"COMPLETED" with requests: 0.

if ($pid === 0) {
// CHILD: connect after fork, run slice, write partial, exit.
fclose($parentEnd);
$this->runWorkerAndReport($phase, $workerCount, $i, $childEnd);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A worker whose body throws never reaches exit(0) and goes on to execute the parent's code path.

The $pid === 0 branch is unwrapped, so a Throwable from runWorkerAndReportClientFactory::createAndConnect failing is the reachable one — unwinds past the exit(0) below, out of runPhaseMultiProcess and run.

phpredis driver with ext-redis absent, 8 connections, PHP 8.5.7:

THREW in pid=24371 ... 24378                                    (8 children)
TAIL OF SCRIPT reached in pid=24371 ... 24378, and 24366         (8 children + parent)

Cli::run catches and returns 1, so on the CLI this surfaces as N+1 duplicated Error: lines on a shared stderr plus a COMPLETED / 0 requests phase. PHPUnit also catches, so under the test suite each child continues into the remaining tests and forks again. RateLimitingTest, LiveClientTest, and RecordingWorkloadTest all run concurrencyMode: 'process', and the integration suite runs in the test-php job.

// Reap children.
foreach (array_keys($children) as $pid) {
$status = 0;
pcntl_waitpid($pid, $status);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A parent-side failure leaves the already-forked workers running with no one to reap them.

The reap loop sits after both the fork loop and the drain loop with no try/finally around them, and four throw sites precede it: the stream_socket_pair and pcntl_fork failure paths, and mergePartial's json_decode(..., JSON_THROW_ON_ERROR).

60-connection duration phase under ulimit -n 40, PHP 8.5.7 — stream_socket_pair failed at about worker 34 and the parent exited, while the orphans' fwrite(): Broken pipe notice and their OOM fatal both printed after the parent had gone.

At the MAX_WORKERS cap on a 300s phase, a pair failure at worker 200 leaves 199 orphans driving load for the rest of the phase, while the matrix runner proceeds to the next cell against that same server.

"valkey-glide-csharp": "csharp",
# PHP drivers
"valkey-glide-php": "php",
"phpredis": "php",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--language php exits at argument parsing.

Both PHP drivers are in DRIVER_LANGUAGE_MAP, and the --language choices list still reads ["java", "ruby", "csharp", "node", "python"]. The comment directly above this map asks for the two to be kept in sync.

--language node  ->  runs
--language php   ->  argparse: invalid choice: 'php'

.github/workflows/benchmark.yml carries graph steps for Java, Ruby, and Node.js and none for PHP, so benchmark-php currently builds the valkey_glide extension from source, uploads its artifact, and yields no graph; a PHP step added to that job then reaches the choices list.

return null;
}
$total = $phase->completion->totalRequests();
$share = intdiv($total, $workerCount);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A request-based phase ends when the slowest worker drains its own share, not when the aggregate count is reached, so its wall clock and reported throughput are straggler-bound.

workerRequestTarget divides total_requests statically. Java holds one AtomicLong per phase that workers claim through getAndIncrement, and node/src/engine/benchmark.ts does the same through RequestBudget.claim(). docs/ADDING_LANGUAGE.md lists the shared budget as a submission requirement: "The request budget is shared across workers and claimed one request at a time, not pre-divided per worker".

The docblock above this method says the split "matches Java's forkForThread". forkForThread is KeyGenerator's fork and shares the key counter; it takes no part in dividing the request budget.

configs/matrices/php-driver-comparison.json pairs connections 1 to 16 with a type: "requests" workload. At connections: 1 the split is a no-op, so a single-connection smoke run does not exercise it.

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.

3 participants