Repository navigation
perf: scan the environment once when building config classes - #10657
Open
lonnieezell wants to merge 1 commit into
Open
lonnieezell wants to merge 1 commit into
lonnieezell wants to merge 1 commit into
Conversation
…fig classes `BaseConfig::__construct()` checked every leaf property against `$_ENV`, `$_SERVER` and four `getenv()` calls, so the common no-override path was also the slowest. The environment is now indexed once, grouped by the text before the first "." or "_", and each class only walks the keys that match its prefixes. The index is rebuilt when `$_ENV` or `$_SERVER` changes (once per request in Worker Mode) or on `BaseConfig::reset()`. PHP's `getenv()` array omits names containing ".", " " or "[", so those are looked up with `getenv()` on each class's first build and cached for the process. Classes that override `getEnvValue()` or `initEnvValue()` keep the previous live lookups. Lookup precedence is unchanged. Building 20 core config classes (PHP 8.3 CLI, no opcache, median): | Scenario | Before | After | |---------------------------------------|----------|----------| | Warm, no overrides, new $_SERVER | 0.456 ms | 0.073 ms | | Warm, no overrides, unchanged env | 0.456 ms | 0.031 ms | | Warm, 5 overrides, new $_SERVER | 0.467 ms | 0.110 ms | | Warm, 5 overrides, unchanged env | 0.471 ms | 0.067 ms | | Cold first build, no overrides | 0.493 ms | 0.205 ms | | Cold first build, 5 overrides | 0.505 ms | 0.267 ms | Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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
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.
Description
Fixes #10615
Without config caching, and on every request in FrankenPHP Worker Mode,
BaseConfig::__construct()checks every leaf property against the environment. That is 8 failedarray_key_exists()calls plus 4 failedgetenv()calls per leaf, even though most properties have no override.This PR indexes the environment once and reuses it:
$_ENV,$_SERVERandgetenv()are grouped by the text before their first.or_. Each class only looks at the keys that match its short or full prefix. A leaf is skipped unless one of its 4 candidate names exists.[$_ENV, $_SERVER]snapshot and is rebuilt when either changes (once per request in Worker Mode) or onBaseConfig::reset(). Copy-on-write makes the unchanged check cheap.getenv()doesn't list: PHP'sgetenv()array,$_ENVand$_SERVERall leave out OS env var names containing., a space or[. For example,app.baseURLset through Docker or systemd is only readable withgetenv('app.baseURL'). These names are looked up withgetenv()on each class's first build and cached for the process, so they keep working. This is the main difference from the issue's proposal, which would have dropped them.getEnvValue()orinitEnvValue()keep the previous live lookups. This is detected with reflection once per class.$_ENVover$_SERVERovergetenv(), dot form over underscore form, and arrays only accept existing keys.getEnvValue()and the new path now share a single lookup.There is no new public or protected API.
Behavior changes (listed in the changelog and
upgrade_480.rst):putenv(), without a matching$_ENV/$_SERVERwrite, made after the first config class was built may not be seen untilBaseConfig::reset()runs. Values from.envare unaffected, becauseDotEnvalso writes$_ENVand$_SERVER.Benchmark
Time to build the 20 core config classes from the issue: PHP 8.3.23 CLI on macOS, OPcache off,
testingenvironment. Warm rows are the median of 9 rounds of 500 builds. Cold rows are the median of 41 fresh processes, each timing its first build after autoloading and Registrar discovery. "5 overrides" meansapp.baseURLplus fourdatabase.default.*keys in$_ENV,$_SERVERandputenv().$_SERVERchanges per build (Worker Mode)$_SERVERchanges per buildThe cold gain is smaller because each class's first build still makes 2
getenv()calls per leaf for the names thatgetenv()doesn't list. I also tried keeping livegetenv()calls for those names on every build instead of caching them. That came out slower than the current code (about 0.59 ms), so the per-process cache is needed. Requests served from config caching (FactoriesCache) already skip this work and gain nothing.Benchmark scripts
Warm (
php bench-config-env.php /path/to/CodeIgniter4):Cold (
php bench-cold.php /path/to/CodeIgniter4 [overrides], run 41 times and take the median):Tests
New tests in
BaseConfigTestcover:$_ENV/$_SERVERwrites after a build being picked upreset()picking up a bareputenv()getenv()_The full suite has the same failures as plain
4.8locally: SQLSRV and Predis are unavailable, and a fewFeatureTestTraittests fail only when run in the full suite.Checklist:
🤖 Generated with Claude Code