Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:89517
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain daveyshafik.com from 209.85.160.172 cause and error)
MIME-Version: 1.0
Sender: me@daveyshafik.com
In-Reply-To: <CANaX5ncTGhgqe4sge4pHxQxv039aq6Tv_baNkWuiNAGn86nO=w@mail.gmail.com>
References: <CANaX5ney7b1ecTQPApziSXTE5LZh-1ehOxrC7=3HMUy6vZHmaA@mail.gmail.com>
	<CANaX5nd=ipbS_mi2MAJjBnucfNmsCobHwotLDTi8mzMDQs=W-A@mail.gmail.com>
	<CANaX5ncTGhgqe4sge4pHxQxv039aq6Tv_baNkWuiNAGn86nO=w@mail.gmail.com>
Date: Tue, 1 Dec 2015 15:42:40 -0500
Message-ID: <CANaX5ncAZZp233d3Y7p=Sd9xxWyx1_0FfGW1FaP_98m8-EPSig@mail.gmail.com>
To: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11473d2ce3800b0525dc321f
Subject: Re: [RFC] HTTP/2 Server Push support in ext/curl
From: davey@php.net (Davey Shafik)

--001a11473d2ce3800b0525dc321f
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

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.

On Sun, Nov 22, 2015 at 12:14 PM, Davey Shafik <davey@php.net> wrote:

> On Wed, Nov 18, 2015 at 6:34 PM, Davey Shafik <davey@php.net> wrote:
>
>> 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
>>
>> On Wed, Nov 18, 2015 at 5:31 PM, Davey Shafik <davey@php.net> wrote:
>>
>>> 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 bugfi=
x)
>>> 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 man=
aged
>>> 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 numb=
er
> of headers, this was a holdover from the C API and unnecessary in PHP, ju=
st
> 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 the=
m
> =E2=80=94 again, any help would be appreciated.
>
> Thanks,
>

--001a11473d2ce3800b0525dc321f--