Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:78757
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain devilix.net designates 209.85.214.180 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAH-PCH6jjnoLRQs8xeONimuvcHzgL707modxOqxV1DFQvX7M4g@mail.gmail.com>
References: <CAGAGxbY+2G8zOWkHW7t_QhvFSFM=QLY7tSWJpGc8tAgpraLm-w@mail.gmail.com>
	<E336496D-4FEB-4B99-ABA6-AFF0F42E5339@gmail.com>
	<CAPwsocFVSBqju+jCSjOQeL3wFGbr=mLD6xC7jYh2u_RgQbOZeA@mail.gmail.com>
	<CAGAGxbbWU_=EFmMCq6YWdoB4P+OMtBS=xfL4W1-WFfyZxvKYAA@mail.gmail.com>
	<CAGAGxbbjSR0LKXKHKirzF=grEFRZD93get9+c8BAbK=fZ3VWzw@mail.gmail.com>
	<424081508.1350448.1415209726279.JavaMail.zimbra@escapemg.com>
	<801152711.1350497.1415209768109.JavaMail.zimbra@escapemg.com>
	<CAH-PCH6jjnoLRQs8xeONimuvcHzgL707modxOqxV1DFQvX7M4g@mail.gmail.com>
Date: Thu, 6 Nov 2014 01:23:49 +0200
Message-ID: <CAPhkiZzA7dgLGoOfWaj-gJzeSk17EKbMaJ9dV22JLxREPVJtRg@mail.gmail.com>
To: Ferenc Kovacs <tyra3l@gmail.com>
Cc: Mark Caudill <mark.caudill@grooveshark.com>, Yasuo Ohgaki <yohgaki@php.net>, 
	julien pauli <jpauli@php.net>, Damian Wadley <php@requinix.net>, 
	PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] Lazy writing in the session, session-lock-ini, and bug #68331
From: narf@devilix.net (Andrey Andreev)

Hi all,

On Wed, Nov 5, 2014 at 9:52 PM, Ferenc Kovacs <tyra3l@gmail.com> wrote:
>
> Hi,
>
> thanks for the report.
> first let me clarify a couple of things:
>
> 1, neither of https://wiki.php.net/rfc/session-lock-ini or
> https://wiki.php.net/rfc/session-read_only-lazy_write made into 5.6 in their
> proposed forms
> 2, the changes about the write short circuit (and session_gc() and
> session_serializer_name()) and the lack of consensus went under the radar
> originally, as it was commited to master before 5.6.
>
> sometimes later the write short circuit one was spotted, and Yasuo was asked
> to make a proper rfc about it, which lead to the creation of
> session-lock-ini rfc which didn't got much attention originally, but the
> option which passed was basically the same thing which was committed before,
> plus an optional argument for session_start which controlled the behavior.
> this rfc was not followed up on though, and this was spotted by Andrey who
> pointed out the discrepancies about the session related changes and asked
> the list to reconsider the inclusion of those:
> http://grokbase.com/t/php/php-internals/143brgjp9d/revert-session-serializer-name-session-gc
> http://grokbase.com/t/php/php-internals/143fagz0a7/rfc-revert-extend-postpone-original-rfc-about-read-only-lazy-write-sessions
> http://grokbase.com/t/php/php-internals/143r244qtr/rfc-session-start-read-only-lazy-write-take-2
>
> a lengthy discussion broke out and Yasuo reverted the changes about
> session_gc() and session_serializer_name() which was introduced without
> prior discussion, and there were some more discussion about the committed
> write short circuit and the accepted but not yet merged lazy_write, which
> also triggered the creation of
> https://wiki.php.net/rfc/session-read_only-lazy_write but in the end,
> nothing was done, neither of reverting the original short circuit nor
> properly exposing this to the userland session handler.
>
> We also had a last minute blocker issue because of the write short
> circuit(https://bugs.php.net/bug.php?id=67694) but I guess we were too
> focused on shipping 5.6 at that point to take a step back and reconsider
> reverting/refining this small performance improvement before the 5.6.0
> final.
>
> I cc'ed Julien(the other RM for 5.6), Yasuo (the original author of the
> change in question) and Andrey(who was the most vocal and active person
> discussing the lazy write changes with Yasuo and the author of the
> https://wiki.php.net/rfc/session-read_only-lazy_write rfc), and I would like
> to hear your opinion on the situation at hand.
> Personally I think that we can have 3 options:
>
> 1. keep everything as is, including the perf gain for those using the default
> session handler and risk the potential problems for others (for example some
> cache/memcache based handlers could be expecting the session write handler
> to be called to not let active but unmodified sessions to be expired/purged
> from the cache). (we keep a potential BC break and if you need to work
> around you have to do some acrobatics)
> 2. introduce a better interface for custom handlers which allows them to write
> custom logic to decide if they want to write out the session or not. (this
> would still be a potential BC break for some people, but a better upgrade
> path, but it would also cause introducing new features in a minor version,
> which would cause for apps/libs to either depend on a minor version or
> support both versions in their custom session handler).
> 3. revert the write short circuit. (this would cause a small perf loss for most
> users - to the same level as it was before 5.6 -, but it would eliminate the
> BC break when migrating to 5.6, and it would allow us to take our time to
> have time to properly figure out the introduction of this feature including
> the interaction with the custom session handlers).
>
> I would vote for 3,

Depending on the context, I'd vote for multiple of those options ...

Short-term fix for 5.6 is obviously to revert the commit. I was vocal
mostly because of principle at the time, but this issue is an example
why.

A long-term "fix" for 5.6 would be what I've done in userland recently
- continue to call SessionHandler::write() at all times, but _inside
of it_ call touch() instead of fwrite() if there's no new data. That
way custom session handlers wouldn't be affected, but the default one
would still provide a performance boost. This is of course if we do
want to keep the feature (I do), but considering that it was voted in
a slightly different form ... I'm just being egoistic with this
suggestion.

For PHP7 though, I definately want a redesign of the session APIs and
it's on my TODO list for when I have some spare time to work on it.

Cheers,
Andrey.