THRIFT-5984: Cache function_exists() capability checks in PHP runtime#3464
Merged
Conversation
Client: php
Memoize PHP capability probes that previously ran on every hot call:
- TBinarySerializer::serialize / deserialize — previously called
function_exists('thrift_protocol_{write,read}_binary') on every
invocation. Now cached via static nullable bool flags +
hasAcceleratedWrite() / hasAcceleratedRead() helpers using the
null-coalescing assignment (??=) operator.
- TSocket::open — previously called function_exists() twice per
open() to detect the sockets extension. Cached via
hasSocketsExtension() helper.
- TSocketPool — replace per-instance $useApcuCache property with
static $hasApcuCache + hasApcuCache() helper for consistency with
the rest of the cleanup. APCu availability is a process-level
property, not per-pool.
Each helper performs the check at most once per PHP process. Pure
optimization — no behavior change.
Tests:
- Add cache resets in setUp() of TBinarySerializerTest, TSocketTest,
TSocketPoolTest via reflection so mocked function_exists is
re-evaluated per test method.
- Register lib/php/src/ext/thrift_protocol/php_thrift_protocol.stub.php
in phpstan.neon scanFiles so PHPStan knows about the C-extension
functions guarded by the new helpers (previously the inline
function_exists() gave PHPStan a narrowing hint that the helper
hides).
Part of the umbrella ticket THRIFT-5960 (PHP modernization).
Generated-by: Claude Opus 4.7
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
1 participant
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.
Memoize PHP capability probes that previously ran on every hot call.
Changes:
TBinarySerializer::serialize/deserialize—function_exists('thrift_protocol_{write,read}_binary')was called on every invocation. Now cached via static?boolflags +hasAcceleratedWrite()/hasAcceleratedRead()helpers using the null-coalescing assignment (??=) operator.TSocket::open—function_exists()was called twice peropen()to detect the sockets extension. Cached viahasSocketsExtension()helper.TSocketPool— replaces per-instance$useApcuCacheproperty with static$hasApcuCache+hasApcuCache()helper for consistency. APCu availability is a process-level property, not per-pool.Each helper performs the check at most once per PHP process. Pure optimization — no behavior change.
Tests:
setUp()ofTBinarySerializerTest,TSocketTest,TSocketPoolTestvia reflection so the mockedfunction_existsis re-evaluated per test method (otherwise the cache would persist across tests in a single PHPUnit process).lib/php/src/ext/thrift_protocol/php_thrift_protocol.stub.phpinphpstan.neonscanFilesso PHPStan knows about the C-extension functions guarded by the new helpers — previously the inlinefunction_exists()gave PHPStan a narrowing hint that the helper now hides.Validation (Docker
thrift-php-dev:local):phpcs— 0 errorsphpstan(level 1) — 0 errorsphpunitUnit Suite — 635 tests, 0 failuresphpunitIntegration Suite — 108 tests, 0 failuresPart of the umbrella ticket THRIFT-5960 (PHP modernization).
TSocketPool::$useApcuCachewas private)Generated-by: Claude Opus 4.7