Hi all,
On Mon, Mar 17, 2014 at 7:47 PM, Andrey Andreev <[email protected]> wrote:
> >> > If save handler (regardless of user or C) does not implement its own
> >> > update(),
> >> > then update() will not be called, instead write() is called.
> >> > To do that, address of the dummy function is compared.
> >>
> >> Ok, so why is 'lazy_write' an option at all then? I don't see a reason
> >> why it shouldn't be the default and even mandatory behavior.
> >
> >
> > I would like to make 'lazy_write' default to TRUE, since there would not
> be
> > 'unsafe_lock'.
> > I wouldn't try to add 'unsafe_lock' anymore. It's safe to make
> 'lazy_write'
> > default to TRUE.
> >
> > 'lazy_write' needs a little memory, but it's session. Session data would
> be
> > small enough
> > for almost all apps and memory is cheap these days.
> > (Note: I've implemented 'lazy_write' by using MD5 hash at first, but
> > benchmark revealed
> > simple store and string compare is much faster. It's saving loaded
> session
> > data and
> > compare to check modification with new patch.)
> >
> > Any objection/comment make this default to TRUE or automatically apply
> > 'lazy_write'?
> > I think automatically applying 'lazy_write' is good choice.
>
> My quesion was rather why does it need to be optional at all
> (regardless of the default value)? If it is somehow handled at all
> times, it becomes just a performance improvement. I don't see a reason
> why performance improvements should be optional.
>
I agree.
If nobody objects, I'll remove the option. Please send mail ASAP!
>
> >> >> Btw, this should probably be called updateTimestamp() instead of
> >> >> update(), for userland at least. write() vs. update() can be somewhat
> >> >> confusing.
> >> >
> >> >
> >> > This could be changed. Better name is always good.
> >>
> >> Well, there you go - updateTimestamp() is a better name. ;)
> >
> >
> > I thought of similar name. I choose update() since it seemed it is
> matching
> > name for read()/write(), but descriptive name may be better.
> >
> > Any comments for renaming?
>
> I'm all for it (naturally, since I suggested it), it could be
> updateTimestamp(), updateTs(), updateTime(), etc. Anything in that
> fashion would be more descriptive and less confusing.
I agree this, too.
I would choose "updateTimestamp".
Please send mail ASAP, if anyone has comment.
When is the beta due? I made PR so that RMs can merge it.
I don't need git commit log. It's just a personal memo. Please feel free to
merge
with squash if RMs are would like to merge it now. It's good enough for
beta.
https://github.com/php/php-src/pull/628
Regards,
--
Yasuo Ohgaki
[email protected]