Skip to content

Implement RFC: Introduce session_start() options - read_only, unsafe_lock, lazy_write and lazy_destroy #1016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jan 29, 2015

Conversation

yohgaki
Copy link
Contributor

@yohgaki yohgaki commented Jan 22, 2015

Implement RFC
https://wiki.php.net/rfc/session-lock-ini
"read_only" and "lazy_lock" are accepted. PR is made for other people to review.

Except comments, changes are minimal, but includes a few bug fixes that tests equality of PS(session_status) against "php_session_none". The comparison must be "PS(session_status) != php_session_active" as it has php_session_disabled.

Comments are appreciated.

@smalyshev smalyshev added the RFC label Jan 23, 2015
Fix a test.
Use proper types.
@yohgaki yohgaki mentioned this pull request Jan 25, 2015
@yohgaki
Copy link
Contributor Author

yohgaki commented Jan 25, 2015

Test passes for non ZTS build. ZTS build failure seem to happen w/o this PR.
I'll merge this after I write up the UPGRADING. If anyone has comments, please do now.

@yohgaki
Copy link
Contributor Author

yohgaki commented Jan 25, 2015

Document is updated. Please review them. Thank you.

. session_start() accepts all INI settings as array. e.g. ['cache_limiter'=>'private']
sets session.cache_limiter=private. It also supports 'read_and_close' which closes
session data immediately after read data.
. Save handlers accpets validate_sid(), update_timestamp() which validates session
Copy link
Contributor

Choose a reason for hiding this comment

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

typo - accepts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Fixed.

@yohgaki
Copy link
Contributor Author

yohgaki commented Jan 28, 2015

I've tried to get some benchmarks. It seems current system is too fast to get obvious performance difference.

Test command: ab -c 7 -n 500000 http://localhost:8888/session.php
Test script:

<?php
ini_set('session.save_path', '/home/tmp');
ini_set('session.lazy_write', 1); // Change mode here
ini_set('session.use_strict_mode', 0);

session_id('testid');
session_start(['read_and_close'=>0]); // Change mode here
//$_SESSION['test'] = ++$_SESSION['test'];
$_SESSION['a'] = str_repeat('a', 102400);
echo '<pre>';
var_dump(session_id(), $_SESSION['test']); 

Old behavior was around 15000 reqs/sec.
"read_and_close" improved it to about 20000 reqs/sec. i.e 33% faster.
"lazy_write" did not improve # of reqs, but per process httpd disk writes was reduced from 100 MB/s to 5 MB/s. i.e. There were 12 httpd processes, 1200 MB/s writes was reduced to 60 MB/s writes.

I think this would be good enough benchmark for merging.

@php-pulls php-pulls merged commit 7a97eaf into php:master Jan 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants