Don't defeat session.lazy_write in the php session driver (#9885) - #10248
Merged
Conversation
Member
|
Please, remove the changelog changes. |
…9885) The php session driver updated $_SESSION['__MTIME'] on every request. The variable is not read anywhere, but mutating session data on each request prevents PHP's session.lazy_write optimization from skipping storage writes for unchanged sessions - every request caused a full session write. With a network-backed session.save_handler (e.g. Redis) this is significant unnecessary traffic; the original reporter measured about half of their Redis traffic caused by it. Remove the __MTIME update. Session keep-alive does not need it: with lazy_write, PHP calls the save handler's updateTimestamp() for unchanged sessions, which refreshes the TTL (phpredis) or mtime (files). The __IP variable is kept, it is required for ip_check and is idempotent, so it does not prevent lazy_write. Verified with a counting save handler over two simulated requests: with __MTIME PHP calls write() on both requests; without it, the second request triggers updateTimestamp() only. Implements what was agreed with the maintainer in PR roundcube#9885. Co-authored-by: Thomas P <[email protected]> Co-Authored-By: Claude Fable 5 <[email protected]>
MiMoHo
force-pushed
the
session-php-lazy-write
branch
from
July 19, 2026 02:26
32940cf to
2946ace
Compare
Contributor
Author
|
Done — dropped the Changelog entry. |
alecpl
pushed a commit
that referenced
this pull request
Jul 19, 2026
…0248) The php session driver updated $_SESSION['__MTIME'] on every request. The variable is not read anywhere, but mutating session data on each request prevents PHP's session.lazy_write optimization from skipping storage writes for unchanged sessions - every request caused a full session write. With a network-backed session.save_handler (e.g. Redis) this is significant unnecessary traffic; the original reporter measured about half of their Redis traffic caused by it. Remove the __MTIME update. Session keep-alive does not need it: with lazy_write, PHP calls the save handler's updateTimestamp() for unchanged sessions, which refreshes the TTL (phpredis) or mtime (files). The __IP variable is kept, it is required for ip_check and is idempotent, so it does not prevent lazy_write. Verified with a counting save handler over two simulated requests: with __MTIME PHP calls write() on both requests; without it, the second request triggers updateTimestamp() only. Implements what was agreed with the maintainer in PR #9885. Co-authored-by: Thomas P <[email protected]> Co-authored-by: Claude Fable 5 <[email protected]>
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
2 participants
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.
Supersedes #9885, implementing what was agreed there between @tpayen and @alecpl about a year ago (the PR was not updated since); original analysis and measurement by @tpayen, credited as co-author.
Problem
rcube_session_php::write_close()sets$_SESSION['__MTIME'] = time()on every request. The variable is not read anywhere in the codebase (verified via grep — the$changedproperty alecpl mentioned in #9885 has meanwhile been removed entirely by theexpires_atrework). Its only effect is that session data changes on every request, which defeats PHP'ssession.lazy_writeoptimization: PHP writes the full session to storage on every request instead of skipping unchanged sessions. With a network-backedsession.save_handler(e.g. phpredis, as in #9885), that is significant unnecessary traffic — the original reporter measured ~50% of their Redis session traffic caused by this.Change
Remove the
__MTIMEupdate (as agreed in #9885, rather than the throttling originally proposed there).lazy_write, PHP calls the save handler'supdateTimestamp()for unchanged sessions, which refreshes the TTL (phpredis issuesEXPIRE) or the file mtime (files handler).session.gc_maxlifetimeis already set fromsession_lifetimeinrcube::session_init().__IPis kept — it is read back instart()and required forip_check— and it is idempotent from the second request on, so it does not preventlazy_write.phpdriver is affected; the db/redis/memcache drivers register their own save handler and manage expiry viaexpires_atthemselves.Testing
SessionHandlerInterface+SessionUpdateTimestampHandlerInterfaceover two simulated requests (fixed session id,lazy_write=1, second request changes nothing):__MTIME(current master):write, write__MTIME(this PR):write, updateTimestampRcubeTest::test_exec, BSD vs GNUdateon macOS, unrelated).php-cs-fixerclean on the changed file.This contribution was prepared with AI assistance (Claude Code); the analysis and test results above were independently verified as described.
🤖 Generated with Claude Code