Skip to content

Narrow the container cache key to the referenced %env.* variables - #5927

Draft
SanderMuller wants to merge 1 commit into
phpstan:2.2.xfrom
SanderMuller:container-cache-key-relevant-env
Draft

Narrow the container cache key to the referenced %env.* variables#5927
SanderMuller wants to merge 1 commit into
phpstan:2.2.xfrom
SanderMuller:container-cache-key-relevant-env

Conversation

@SanderMuller

@SanderMuller SanderMuller commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Closes phpstan/phpstan#14072

PHPStan puts the whole environment into the DI container cache key, so any unrelated environment change (a different CI job, a new shell session, an editor integration exporting a variable) invalidates the key and forces a full container recompile.

This keeps env a static parameter and narrows only the cache key to the %env.*% variables the loaded configs actually reference, plus the variables read at build time by CompilerExtensions (BUILD_TIME_ENV_VARIABLES, currently PHPSTAN_FNSR). The full environment stays in the container parameters, so %env.*% resolution and the compiled container are unchanged; only the cache key is narrowed.

Enumerating the referenced variables

References are collected from the config as parsed by PHPStan's own NeonAdapter (the same parser the container compiler uses), rather than from a raw text scan:

  • comments are ignored, so a %env.X% mentioned only in a comment no longer forces a recompile;
  • service entities become Statements, so a %env.X% in a parameter value, a service argument or a factory is found the same way it is at compile time;
  • the %env.NAME% grammar mirrors Nette's parameter-name grammar (%([\w.-]*)%), so dashed names like %env.MY-VAR% are handled.

A bare %env% reference (the whole environment), or a config that cannot be parsed here, conservatively keeps the full environment in the key, so the change can never reuse a stale container.

On keeping env static

env stays a static parameter. An earlier revision of this PR made it a dynamic parameter; that breaks %env.X%.neon includes (include paths are resolved against static params at loadConfiguration, which fails e2e/bug-12606), and it is unclear how nette/di reconciles referencing a dynamic parameter from a static one. Keeping env static means the container build is byte-identical to today, so the nette/di E2E tests are unaffected; the only observable change is that unrelated environment changes no longer force a recompile.

@SanderMuller
SanderMuller force-pushed the container-cache-key-relevant-env branch from 8a335c8 to c373f1f Compare June 24, 2026 16:06
private function relevantEnvVariableNamesForCacheKey(): ?array
{
$names = self::BUILD_TIME_ENV_VARIABLES;
foreach ($this->allConfigFiles as $file) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This isn't a great way to do this. I think the tracking of used %env... should be somewhere deeper in the DIC compiler. I'm not entirely sure how to implement it. This would have to be researched where Nette DI actually expands this parameters and whether we can efficiently tap into it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ondrejmirtes shall I apply the suggestion in this comment below, or do you have another direction you want me to explore?

@SanderMuller

Copy link
Copy Markdown
Contributor Author

Agreed, scanning config text for %env.*% is the wrong layer (the dashed-name fragility was a symptom of it).

I tried tapping into Nette's expansion directly first, and it's awkward. Helpers::expand resolves a %env.X% ref by walking the array with is_array() && array_key_exists(), so an access-recording ArrayAccess around env fails the is_array check and, not being a DynamicParameter either, makes expand throw Missing parameter (Helpers.php:80). There's no object hook to record through, and DependencyChecker tracks files and classes, not parameters, so there's nothing there to reuse either.

What does work cleanly is to stop baking env into the container at all: move env from a static parameter to a dynamic one (addDynamicParameters). Then %env.X% takes Nette's own DynamicParameter path (Helpers.php:77) and resolves at runtime, the compiled container no longer depends on env values, and env drops out of the cache key with no scanning. I tested it: toggling an unrelated env var reuses the container, and %env.X% (including tmpDir: %env.X%) still resolves.

The one case it doesn't cover is env read at build time via getenv() instead of %env.*%. PHPStan does that in exactly one place: FnsrExtension::beforeCompile() reads getenv('PHPSTAN_FNSR') and rewires the container, so that variable has to stay in the key explicitly (a one-entry allowlist), otherwise toggling it reuses a stale container. Everything else drops.

That removes the regex entirely. I can push it to this PR if the direction looks right.

@SanderMuller
SanderMuller force-pushed the container-cache-key-relevant-env branch from c373f1f to 9581033 Compare July 3, 2026 19:55
@SanderMuller SanderMuller changed the title Keep only the relevant environment variables in the container cache key Make the environment a dynamic container parameter Jul 3, 2026
@SanderMuller

Copy link
Copy Markdown
Contributor Author

@ondrejmirtes Reworked this per your note that the tracking belongs deeper in the DIC compiler rather than scanning config text.

Instead of scanning for %env.*% and narrowing the key, env is now passed via addDynamicParameters() rather than as a static parameter. Only the parameter name goes into the container cache key, never the values, so an unrelated env var can no longer flip it. %env.*% still works: it resolves at runtime, so a config that references it is reused, and picks up the current value, instead of being recompiled. No config scanning remains.

The one build-time exception is env read outside %env.*%: FnsrExtension reads getenv('PHPSTAN_FNSR') in beforeCompile() and rewires NodeScopeResolver, so that value stays in the key via Configurator::BUILD_TIME_ENV_VARIABLES, guarded by ContainerCacheKeyEnvGuardTest.

One behavior change worth calling out: because %env.X% resolves at runtime now, a reference to an unset env var resolves to null (with a PHP warning) where before it threw Missing parameter 'env.X' at build time. Preserving that build-time error would require knowing which vars are referenced, i.e. the scanning this removes, so they do not combine.

This is the direction I went with, but I am happy to take it a different way if you would prefer to keep the build-time error or have another approach in mind.

@SanderMuller
SanderMuller marked this pull request as draft July 3, 2026 20:28
@ondrejmirtes

Copy link
Copy Markdown
Member

I really don't know if it would work or not. In phpstan-src we're locked to nette/di 3.1.5 (

phpstan-src/composer.json

Lines 175 to 247 in 709654e

"package": {
"name": "nette/di",
"version": "v3.1.5",
"source": {
"type": "git",
"url": "https://github.com/nette/di.git",
"reference": "00ea0afa643b3b4383a5cd1a322656c989ade498"
},
"dist": {
"type": "zip",
"url": "https://api.github.com/repos/nette/di/zipball/00ea0afa643b3b4383a5cd1a322656c989ade498",
"reference": "00ea0afa643b3b4383a5cd1a322656c989ade498",
"shasum": ""
},
"require": {
"ext-tokenizer": "*",
"nette/neon": "^3.3 || ^4.0",
"nette/php-generator": "^3.5.4 || ^4.0",
"nette/robot-loader": "^3.2 || ~4.0.0",
"nette/schema": "^1.2",
"nette/utils": "^3.2.5 || ~4.0.0",
"php": "7.2 - 8.3"
},
"require-dev": {
"nette/tester": "^2.4",
"phpstan/phpstan": "^1.0",
"tracy/tracy": "^2.9"
},
"type": "library",
"extra": {
"branch-alias": {
"dev-master": "3.1-dev"
}
},
"autoload": {
"classmap": [
"src/"
]
},
"notification-url": "https://packagist.org/downloads/",
"license": [
"BSD-3-Clause",
"GPL-2.0-only",
"GPL-3.0-only"
],
"authors": [
{
"name": "David Grudl",
"homepage": "https://davidgrudl.com"
},
{
"name": "Nette Community",
"homepage": "https://nette.org/contributors"
}
],
"description": "💎 Nette Dependency Injection Container: Flexible, compiled and full-featured DIC with perfectly usable autowiring and support for all new PHP features.",
"homepage": "https://nette.org",
"keywords": [
"compiled",
"di",
"dic",
"factory",
"ioc",
"nette",
"static"
],
"support": {
"issues": "https://github.com/nette/di/issues",
"source": "https://github.com/nette/di/tree/v3.1.5"
},
"time": "2023-10-02T19:58:38+00:00"
}
}
) - an unpublished release mind you, because newer published releases break stuff we depend on. These things are covered by E2E tests, like 980ab5b, that break when we update to a newer version. They might also break when changing %env% to a dynamic parameter because I don't know how the library would reconcile referencing dynamic parameter from a static parameter.

@SanderMuller
SanderMuller force-pushed the container-cache-key-relevant-env branch from 9581033 to e758aec Compare July 3, 2026 23:47
@SanderMuller SanderMuller changed the title Make the environment a dynamic container parameter Narrow the container cache key to the referenced %env.* variables Jul 3, 2026
@SanderMuller

Copy link
Copy Markdown
Contributor Author

Thanks, you're right to be wary. I've dropped the dynamic-parameter direction. env stays a static parameter, so the compiled container is byte-identical and the nette/di E2E tests you mentioned are unaffected. The only change is that the container cache key now includes just the %env.*% variables the config actually references, instead of the whole environment, so unrelated CI/shell env changes no longer force a full recompile.

To find the referenced variables without a raw text scan, each config is parsed with PHPStan's own NeonAdapter (the same parser the container compiler uses), and the %env.NAME% references are collected from the parsed values using Nette's parameter-name grammar. Because it parses rather than scans text, comments are ignored and service entities are handled as Statements, so a reference in a parameter value, a service argument or a factory is found exactly as it is at compile time. A bare %env% reference, or a config that can't be parsed here, conservatively keeps the whole environment in the key.

This runs against the locked nette/di 3.1.5, since NeonAdapter is already what phpstan-src uses to load configs. I force-pushed the branch to replace the dynamic-parameter commit; the updated description has the full details. Happy to move the enumeration deeper than the Configurator if you'd prefer it live elsewhere.

@SanderMuller
SanderMuller force-pushed the container-cache-key-relevant-env branch 2 times, most recently from bba3c00 to 74bb5f6 Compare July 27, 2026 15:22
@staabm

staabm commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

I really don't know if it would work or not. In phpstan-src we're locked to nette/di 3.1.5

this todo was resolved recently with cfd6811

PHPStan puts the whole environment into the DI container cache key, so any
unrelated env change (CI, shell session, etc.) forces a full container
recompile (phpstan/phpstan#14072).

Keep env a static parameter and narrow only the cache key to the %env.*%
variables the loaded configs actually reference, plus the env vars read at
build time by CompilerExtensions (BUILD_TIME_ENV_VARIABLES). The full
environment stays in the container parameters, so %env.*% resolution and the
compiled container are unchanged.

References are enumerated from the config as parsed by PHPStan's own
NeonAdapter - the same parser the container compiler uses - not from raw config
text: comments are ignored, and service entities become Statements so a
%env.NAME% in a parameter value, a service argument or a factory is found the
same way it is at compile time. The %env.NAME% grammar mirrors Nette's
parameter-name grammar, so dashed names like %env.MY-VAR% are handled. A bare
%env% reference, or a config that can't be parsed here, keeps the full
environment in the key.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@SanderMuller
SanderMuller force-pushed the container-cache-key-relevant-env branch from 74bb5f6 to 1827182 Compare July 27, 2026 15:37
@SanderMuller

Copy link
Copy Markdown
Contributor Author

Good point, thanks. That pin was exactly why I backed out of the dynamic-parameter approach earlier (making env a dynamic parameter, so only the variable names, never the values, end up in the container cache key). With 3.1.10 that constraint is gone, so that route is open again.

The version currently on the PR went the other way: env stays a static parameter and only the cache key is narrowed to the %env.*% variables the configs actually reference. The compiled container is byte-identical and it doesn't depend on the nette/di version at all. It's rebased onto the post-3.1.10 base now and green on the PR-specific checks.

So there are two viable directions, and I'd rather not guess which you'd prefer:

  1. Keep the current static narrowing (byte-identical container; the reference enumeration lives in Configurator).
  2. Go back to the dynamic-parameter approach now that the pin is lifted (simpler cache key, but it changes one behavior: an unset %env.X% resolves to null with a warning instead of throwing Missing parameter at build time).

Which should I focus on?

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.

Performance - optimize run with result cache

3 participants