Hey folks,
I'd like to introduce a new RFC for 7.1 that will add support for HTTP/2
Server Push to ext/curl.
This is exposing new features from libcurl to user land, which means they
must have the latest (in fact currently unreleased, due to a bugfix)
version of libcurl.
For those who are not aware, HTTP/2 support is growing rapidly, and is
already available in 60% of browsers, Apache 2 (via mod_h2), nginx, and
even IIS.
The RFC can be seen here: https://wiki.php.net/rfc/curl_http2_push
With the patch at:
https://github.com/php/php-src/compare/master...dshafik:curl-http2-push
The patch still needs some work, possibly a lot of work due to my
inexperience with C. Specifically, there are some memory leaks I've managed
to introduce with the headers array stuff (I'm pretty sure, lines
529-535[1]) and there is some duplicated code from interface.c (lines
454-523[2]) that I was unable to refactor out successfully to it's own
function. Plus, on the whole it seems incredibly untidy to me.
Feedback welcome!
- Davey
[1]
https://github.com/php/php-src/compare/master...dshafik:curl-http2-push#diff-ab7a9164033bd55887d45d0a6cb837eeR529
[2]
https://github.com/php/php-src/compare/master...dshafik:curl-http2-push#diff-ab7a9164033bd55887d45d0a6cb837eeR451
Forgot to mention, if you'd like to try this patch for yourself, check out
my Docker container stuff here (with instructions :):
https://github.com/dshafik/php-http2-push-example
Hey folks,
I'd like to introduce a new RFC for 7.1 that will add support for HTTP/2
Server Push to ext/curl.This is exposing new features from libcurl to user land, which means they
must have the latest (in fact currently unreleased, due to a bugfix)
version of libcurl.For those who are not aware, HTTP/2 support is growing rapidly, and is
already available in 60% of browsers, Apache 2 (via mod_h2), nginx, and
even IIS.The RFC can be seen here: https://wiki.php.net/rfc/curl_http2_push
With the patch at:
https://github.com/php/php-src/compare/master...dshafik:curl-http2-pushThe patch still needs some work, possibly a lot of work due to my
inexperience with C. Specifically, there are some memory leaks I've managed
to introduce with the headers array stuff (I'm pretty sure, lines
529-535[1]) and there is some duplicated code from interface.c (lines
454-523[2]) that I was unable to refactor out successfully to it's own
function. Plus, on the whole it seems incredibly untidy to me.Feedback welcome!
- Davey
[1]
https://github.com/php/php-src/compare/master...dshafik:curl-http2-push#diff-ab7a9164033bd55887d45d0a6cb837eeR529
[2]
https://github.com/php/php-src/compare/master...dshafik:curl-http2-push#diff-ab7a9164033bd55887d45d0a6cb837eeR451
Forgot to mention, if you'd like to try this patch for yourself, check out
my Docker container stuff here (with instructions :):
https://github.com/dshafik/php-http2-push-exampleHey folks,
I'd like to introduce a new RFC for 7.1 that will add support for HTTP/2
Server Push to ext/curl.This is exposing new features from libcurl to user land, which means they
must have the latest (in fact currently unreleased, due to a bugfix)
version of libcurl.For those who are not aware, HTTP/2 support is growing rapidly, and is
already available in 60% of browsers, Apache 2 (via mod_h2), nginx, and
even IIS.The RFC can be seen here: https://wiki.php.net/rfc/curl_http2_push
With the patch at:
https://github.com/php/php-src/compare/master...dshafik:curl-http2-pushThe patch still needs some work, possibly a lot of work due to my
inexperience with C. Specifically, there are some memory leaks I've managed
to introduce with the headers array stuff (I'm pretty sure, lines
529-535[1]) and there is some duplicated code from interface.c (lines
454-523[2]) that I was unable to refactor out successfully to it's own
function. Plus, on the whole it seems incredibly untidy to me.Feedback welcome!
- Davey
[1]
https://github.com/php/php-src/compare/master...dshafik:curl-http2-push#diff-ab7a9164033bd55887d45d0a6cb837eeR529
[2]
https://github.com/php/php-src/compare/master...dshafik:curl-http2-push#diff-ab7a9164033bd55887d45d0a6cb837eeR451
I have made some further changes to this patch (tidied it up a bit, and
refactored out the duplicate code) and RFC (no longer passing in the number
of headers, this was a holdover from the C API and unnecessary in PHP, just
use count()
on the header array).
If you tried out the Docker container, a simple rebuild (perhaps with
--no-cache) will fetch the latest patch and rebuild.
The memory leaks are still there, and I'm not much closer to plugging them
— again, any help would be appreciated.
Thanks,
Hi all,
I wanted to give another update, thanks to the input of Sean DuBois, the
patch is now pretty much final. There is one more memory leak, but I
believe it's in libcurl itself, I'll follow up over there on that.
Sara expressed a desire to get this into HHVM, so I'm going to follow up
with her specifically to get an opinion on the implementation.
Assuming she has no issues, I'd like to talk about moving forward with a
vote soon.
Forgot to mention, if you'd like to try this patch for yourself, check
out my Docker container stuff here (with instructions :):
https://github.com/dshafik/php-http2-push-exampleHey folks,
I'd like to introduce a new RFC for 7.1 that will add support for HTTP/2
Server Push to ext/curl.This is exposing new features from libcurl to user land, which means
they must have the latest (in fact currently unreleased, due to a bugfix)
version of libcurl.For those who are not aware, HTTP/2 support is growing rapidly, and is
already available in 60% of browsers, Apache 2 (via mod_h2), nginx, and
even IIS.The RFC can be seen here: https://wiki.php.net/rfc/curl_http2_push
With the patch at:
https://github.com/php/php-src/compare/master...dshafik:curl-http2-pushThe patch still needs some work, possibly a lot of work due to my
inexperience with C. Specifically, there are some memory leaks I've managed
to introduce with the headers array stuff (I'm pretty sure, lines
529-535[1]) and there is some duplicated code from interface.c (lines
454-523[2]) that I was unable to refactor out successfully to it's own
function. Plus, on the whole it seems incredibly untidy to me.Feedback welcome!
- Davey
[1]
https://github.com/php/php-src/compare/master...dshafik:curl-http2-push#diff-ab7a9164033bd55887d45d0a6cb837eeR529
[2]
https://github.com/php/php-src/compare/master...dshafik:curl-http2-push#diff-ab7a9164033bd55887d45d0a6cb837eeR451I have made some further changes to this patch (tidied it up a bit, and
refactored out the duplicate code) and RFC (no longer passing in the number
of headers, this was a holdover from the C API and unnecessary in PHP, just
usecount()
on the header array).If you tried out the Docker container, a simple rebuild (perhaps with
--no-cache) will fetch the latest patch and rebuild.The memory leaks are still there, and I'm not much closer to plugging them
— again, any help would be appreciated.Thanks,
I wanted to give another update, thanks to the input of Sean DuBois, the
patch is now pretty much final. There is one more memory leak, but I
believe it's in libcurl itself, I'll follow up over there on that.Sara expressed a desire to get this into HHVM, so I'm going to follow up
with her specifically to get an opinion on the implementation.Assuming she has no issues, I'd like to talk about moving forward with a
vote soon.
LGTM; I'll wait until it's landed before getting started on the HHVM
port since it needs a bleeding edge version of libcurl anyway.
-Sara