Skip to content

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

Open
dunglas wants to merge 1 commit into
mainfrom
fix/hot-reload-opcache
Open

dunglas wants to merge 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.

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.
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.

This branch has not been deployed

No deployments
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