Skip to content
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

HHVM implementation #136

Merged
merged 1 commit into from
Mar 20, 2015
Merged

HHVM implementation #136

merged 1 commit into from
Mar 20, 2015

Conversation

swalkinshaw
Copy link
Member

This is an updated and slightly simplified version of #45.

Most of the credit goes to @nathanielks since a lot of this was copied directly from that work.

This does not support magically toggling between the two implementations though. I don't think the extra complexity that adds is worth it. If anyone wants to switch between the two on an existing server, it's not much extra work to do the manual cleanup.

Note: this also removes the per-site php-fpm worker pools that existed before. It was needless complexity for the little/no benefits it added and would have made the setup diverse more from HHVM.

Todo:

@louim
Copy link
Contributor

louim commented Mar 6, 2015

Reading trough HHVM issues, I came across: facebook/hhvm#4398. I'm not currently using HHVM, but I'm looking into it for performance reason. Do you think such mechanism (reloading HHVM automatically when memory usage is high or juste in the middle of the night) should be added? Maybe just a comment in the readme or an option to enable it?

Thoughts?

@swalkinshaw
Copy link
Member Author

@louim good catch. Tough to tell how pervasive this problem is but I'm more inclined to include a nightly cron job by default since the memory leak source has been found. And obviously this won't change anytime soon since WP won't switch to anonymous functions.

@louim
Copy link
Contributor

louim commented Mar 6, 2015

👍 for the nightly cronjob.

@swalkinshaw
Copy link
Member Author

@louim added the daily restart cron job

@louim
Copy link
Contributor

louim commented Mar 13, 2015

@swalkinshaw Excellent, do you plan to merge this into the project soon?

@swalkinshaw
Copy link
Member Author

@louim yep, I'll try and do it tonight.

@swalkinshaw swalkinshaw reopened this Mar 17, 2015
swalkinshaw added a commit that referenced this pull request Mar 20, 2015
@swalkinshaw swalkinshaw merged commit 96af131 into master Mar 20, 2015
@swalkinshaw swalkinshaw deleted the hhvm branch March 20, 2015 03:46
@swalkinshaw swalkinshaw mentioned this pull request Apr 3, 2015
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.

2 participants