fix: make Core::pointerAtAddress allocation-free (leaked ~116 B per call) - #135
Merged
Conversation
pointerAtAddress() built its result by allocating a fresh `{$type}[1]` slot,
writing the address through an integer view of it and returning `$slot[0]`.
That element view pins its owning slot for as long as it lives, so the owned
allocation was never released: every call retained ~116 bytes for the rest of
the request.
It affects every caller, and the hot one is hashtable construction -
uninitializedBucketData() calls it once per table, so any workload that mints
persistent hashtables in a loop grew without bound. Introduced in a56d59d,
which moved that helper from pure cast arithmetic to pointerAtAddress().
FFI::cast() reinterprets a plain integer as a pointer value, so the typed
pointer needs no backing buffer at all. Semantics are unchanged: the result is
still a `T*` CData carrying exactly the requested address, verified by an
addressOf() round trip. The instanceof assert goes with the old body - it
existed to narrow the untyped `$slot[0]`, while cast() is declared to return
CData, and PHPStan rejects it as provably redundant.
Measured over 50k calls (PHP 8.4.19, release build):
before +5832192 request bytes (116.6 B/call), RSS +5556 kB
after +0 request bytes, RSS +0 kB
Downstream php-shared-data-extension, whose reclamation soak this was failing:
tools/soak-drop.php 5000 request +9211080 -> +32048 bytes (budget 65536),
resident 5.63 -> 3.73 kB/cycle (budget 6)
tools/soak.php 5000 stays flat at -1728 bytes
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HRDZ2XsoVuB5uG4qXKL3ny
lisachenko
marked this pull request as ready for review
August 6, 2026 17:49
Owner
Author
|
Please make all branches 8.x not deletable after merge. |
Owner
Author
|
I can't change repository rules with the tooling available in this session (no ruleset/branch-protection API access), so this one needs a minute of your admin UI — two equivalent options: Option A — ruleset (recommended): Settings → Rules → Rulesets → New branch ruleset
Option B — classic protection rule: Settings → Branches → Add branch protection rule → pattern Either one makes "Automatically delete head branches" skip Generated by Claude Code |
The pull_request run for this branch (31123959841) has been wedged pre-queue in the GitHub Actions incident backlog for 7 hours - it cannot be cancelled (409) or re-run. A fresh head mints a fresh run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HRDZ2XsoVuB5uG4qXKL3ny
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a process-lifetime memory leak in
Core::pointerAtAddress(): it minted an ownedT*[1]FFI slot on every call and returned$slot[0]— a view that pins the owning slot forever, so ~116 B leaked per call for every caller. Introduced ina56d59dwhenHashTable::uninitializedBucketData()switched to this primitive, which made every hashtable construction leak.Fix:
FFI::cast()reinterprets a plain integer as a pointer value with no backing buffer, so the method collapses to a single allocation-free cast. Theassert($pointer instanceof CData)is gone because the guarantee is now static (cast()returnsCData; PHPStan level max rejects the redundant runtime check).Regression tests (
tests/CoreMemoryTest.php): anaddressOf()round-trip, and a 50k-call loop asserting RSS growth stays under 1 MB — verified to fail (+5.4 MB) with the fix reverted.Measured, PHP 8.4.19, 50 000 calls: request memory +5 832 192 B → +0 B; RSS +5 556 kB → +0 kB.
Verification on this branch: full phpunit 396/396,
--group opcache --fail-on-skipped26/26,--group internal --process-isolation138/138, PHPStan clean, cs clean.Downstream proof (lisachenko/php-shared-data-extension#13, which surfaced this): with this fix overlaid on tag 8.4.0, its failing "Reclaiming soak" goes from +9 211 080 B FAIL to +32 048 B PASS (budget 65 536), resident 3.73 kB/cycle (budget 6 kB).
Release note: master (8.5) has the same bug via the cascade lineage — the
8.4 → mastermerge-up will carry this fix. A8.4.1tag after merge lets the~8.4.0-pinned dependents pick it up.Note: branch
8.4was auto-deleted when cascade PR #133 merged ("automatically delete head branches" acted on the PR head); it has been restored at its previous SHA. Consider excluding version branches from auto-delete, or the next cascade merge will delete it again.🤖 Generated with Claude Code
https://claude.ai/code/session_01HRDZ2XsoVuB5uG4qXKL3ny
Generated by Claude Code