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

@SanderMullerSanderMuller commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Closesphpstan/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
SanderMullerforce-pushed the container-cache-key-relevant-env branch from 8a335c8 to c373f1fCompareJune 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
ContributorAuthor

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
ContributorAuthor

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
SanderMullerforce-pushed the container-cache-key-relevant-env branch from c373f1f to 9581033CompareJuly 3, 2026 19:55
@SanderMullerSanderMuller changed the title Keep only the relevant environment variables in the container cache keyMake the environment a dynamic container parameterJul 3, 2026
@SanderMuller

Copy link
Copy Markdown
ContributorAuthor

@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 (

"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
SanderMullerforce-pushed the container-cache-key-relevant-env branch from 9581033 to e758aecCompareJuly 3, 2026 23:47
@SanderMullerSanderMuller changed the title Make the environment a dynamic container parameterNarrow the container cache key to the referenced %env.* variablesJul 3, 2026
@SanderMuller

Copy link
Copy Markdown
ContributorAuthor

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
SanderMullerforce-pushed the container-cache-key-relevant-env branch 2 times, most recently from bba3c00 to 74bb5f6CompareJuly 27, 2026 15:22
@staabm

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

@SanderMuller
SanderMullerforce-pushed the container-cache-key-relevant-env branch from 74bb5f6 to 1827182CompareJuly 27, 2026 15:37
@SanderMuller

Copy link
Copy Markdown
ContributorAuthor

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?

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
SanderMullerforce-pushed the container-cache-key-relevant-env branch from 1827182 to 668e035CompareAugust 11, 2026 22:28
@SanderMuller

Copy link
Copy Markdown
ContributorAuthor

Rebased onto current 2.2.x — it had drifted into a conflict, so it was unmergeable as well as unanswered. Now green again:

  • The only conflict was the import block in Configurator.php. use PHPStan\Turbo\TurboExtensionEnabler; was pre-existing context that 2.2.x has since removed, so I did not re-add it; nothing in the file references it. The change itself replays byte-identically (same added lines as before, +547 across the same 8 files).
  • Gates on 668e03568: full suite 21307 tests green, self-analysis clean, phpcs clean, and this PR's own ContainerCacheKeyEnvTest + ContainerCacheKeyEnvGuardTest pass (7 tests).

The open question from July is still the only thing blocking, so to restate it in one line: do you want the current static narrowing, or the dynamic-parameter route now that the nette/di pin is lifted?

  1. Static narrowing (what is on the PR).env stays a static parameter; only the %env.*% names the configs actually reference go into the cache key. Compiled container byte-identical, independent of the nette/di version.
  2. Dynamic parameter. Simpler cache key, but an unset %env.X% then resolves to null with a warning instead of throwing Missing parameter at build time — a behaviour change.

I have no strong preference and either is a small amount of work from here; I just do not want to rebuild it in the wrong direction twice. If neither appeals, closing it is a fine outcome too — the underlying complaint (unrelated CI env vars forcing full container recompiles, phpstan/phpstan#14072) would just stay open.

Sign up for freeto 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

@SanderMuller@ondrejmirtes@staabm