Skip to content

Fixed bug #50333 Improving multi-threaded scalability by using emalloc/efree/estrdup #500

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 39 commits into from
Nov 5, 2013

Conversation

weltling
Copy link
Contributor

Things done

  • The original patch was fixed and applied to the current master fork
  • Additional fixes was developed (windows)
  • Manually tested so far with CLI TS, CLI NTS and Apache TS

DaveRandom and others added 30 commits September 18, 2013 12:18
* PHP-5.5:
  Assume the free space is correct on Travis CI.
… PHP-5.5

* 'PHP-5.5' of https://git.php.net/repository/php-src:
  Assume the free space is correct on Travis CI.
Conflicts:
	ext/openssl/xp_ssl.c
* PHP-5.5:
  Added support for TLSv1.1 and TLSv1.2

Conflicts:
	ext/openssl/xp_ssl.c
* PHP-5.5:
  TLS news
This reverts commit 62be976.
* PHP-5.5:
  Revert "TLS news"
  Revert "Added support for TLSv1.1 and TLSv1.2"
…-src

* 'updated_tls_support' of https://github.com/rdlowrey/php-src:
  Added support for TLSv1.1 and TLSv1.2

Conflicts:
	ext/openssl/xp_ssl.c
… PHP-5.5

* 'PHP-5.5' of https://git.php.net/repository/php-src:
  Revert "TLS news"
  Revert "Added support for TLSv1.1 and TLSv1.2"
  TLS news
  Added support for TLSv1.1 and TLSv1.2
* 'master' of https://git.php.net/repository/php-src:
  TLS news
  previous revert killed that file
  Revert "TLS news"
  Revert "Added support for TLSv1.1 and TLSv1.2"
  TLS news
  fix ws
  Added support for TLSv1.1 and TLSv1.2
  Added support for TLSv1.1 and TLSv1.2
@nikic
Copy link
Member

nikic commented Oct 18, 2013

The Travis build failed because tsrm_virtual_cwd.h is used in ext/phar. Log: https://s3.amazonaws.com/archive.travis-ci.org/jobs/12722496/log.txt

@weltling
Copy link
Contributor Author

Thanks, fixed.

@weltling
Copy link
Contributor Author

After Pierre's idea I'm going to convert this PR to less intrusive, without moving the virtual cwd stuff into zend and doing that much renames.

@weltling
Copy link
Contributor Author

hm, now the build succeeded but it still shows it'l failed https://api.travis-ci.org/jobs/12727144/log.txt?deansi=true

@smalyshev
Copy link
Contributor

The build still shows ton of failed unit tests as far as I can see, and memory errors:
*** glibc detected *** /home/travis/build/php/php-src/sapi/cgi/php-cgi: free(): invalid next size (fast): 0x00007f14414c3090 ***

Also, we need some benchmarks to ensure we are getting performance improvement before introducing this big a patch.

@weltling
Copy link
Contributor Author

Yep, that's what Dmitry has pointed out in this mail http://news.php.net/php.internals/69730 . So I'll follow his suggestion with do_alloca() first, to see if it makes sense at all to make the less intrusive variant.

Thanks.

memcpy((d)->cwd, (s)->cwd, (s)->cwd_length+1);

#define CWD_STATE_FREE(s) \
free((s)->cwd);
efree((s)->cwd); \
(s)->cwd = NULL;
Copy link
Member

Choose a reason for hiding this comment

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

As I understood, this new assignment makes sense only for call from virtual_cwd_deactivate(). May be it makes sense to move it there.

@weltling
Copy link
Contributor Author

weltling commented Nov 2, 2013

@weltling
Copy link
Contributor Author

weltling commented Nov 5, 2013

Looks like this one is ready to merge as Dmitry gave his OK http://news.php.net/php.internals/70009

@php-pulls php-pulls merged commit 699f07b into php:master Nov 5, 2013
@weltling
Copy link
Contributor Author

weltling commented Nov 5, 2013

merged into master

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.