Skip to content

fix(hot-reload): default opcache.revalidate_freq to 0 - #2680

Merged
henderkes merged 1 commit into
mainfrom
fix/hot-reload-opcache
Oct 11, 2026
Merged

henderkes merged 1 commit into
mainfrom
fix/hot-reload-opcache

Conversation

@dunglas

@dunglas dunglas commented Sep 30, 2026

Copy link
Copy Markdown
Member

With the default opcache.revalidate_freq=2, OPcache keeps serving the old version of a changed script for up to 2 seconds. The hot reload client fetches the page right after the Mercure update, so it often receives stale HTML: the morph is a no-op, or the full reload displays the old content. In an end-to-end test with headless Chrome and frankenphp-hot-reload, every PHP change was missed with the default settings.

When hot_reload is configured, opcache.revalidate_freq now defaults to 0. The value is set through the ini_defaults SAPI hook (the one the CLI SAPI uses for display_errors), which runs before the INI files are parsed: a value set in php.ini, in a file of PHP_INI_SCAN_DIR, or with the php_ini directive takes precedence.

Worker mode with watch isn't affected, as it reboots PHP on every change.

Comment thread frankenphp.c
Comment on lines +1490 to +1500
/* php.ini files and php_ini directives override these defaults. */
static void frankenphp_ini_defaults(HashTable *configuration_hash) {
if (go_is_hot_reload_enabled()) {
/* Otherwise, a page reloaded right after a change may run the old code. */
zval tmp;
ZVAL_NEW_STR(&tmp, zend_string_init("0", sizeof("0") - 1, 1));
zend_hash_str_update(configuration_hash, "opcache.revalidate_freq",
sizeof("opcache.revalidate_freq") - 1, &tmp);
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of this. Can we not just do this for tests?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure to follow. It's not to fix our tests but making hot reload reliable.

@henderkes henderkes Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, I thought this was only needed for tests. Have you benchmarked the impacts of this? Especially on Windows this could be a severe performance degradation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's only in dev, and looks like the only way to prevent executing stale code when using hot reload.

@dunglas

dunglas commented Oct 10, 2026

Copy link
Copy Markdown
Member Author

@henderkes does it work for you if I merge this?

@henderkes
henderkes enabled auto-merge (squash) October 10, 2026 18:09
@henderkes

Copy link
Copy Markdown
Contributor

Yes, go for it after conflict.

OPcache served the old version of a script for up to 2 seconds after a change,
so the page fetched by the hot reload client could be stale. The value is set
through the SAPI INI defaults: php.ini files and php_ini directives still take
precedence.
@dunglas
dunglas force-pushed the fix/hot-reload-opcache branch from 9ab5b3b to 5b7fb0a Compare October 11, 2026 15:06
@henderkes
henderkes merged commit 2db0a2d into main Oct 11, 2026
22 checks passed
@henderkes
henderkes deleted the fix/hot-reload-opcache branch October 11, 2026 15:06
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.

2 participants