use HTTP::Headers::Fast

[miyagawa]

Can someone profile/benchmark these?

cc @tokuhirom @kazeburo @avar

[avar]

HTTP::Headers is a minor bottleneck in our infrastructure, I'll try to get to benchmarking this next week in our live systems.

[miyagawa]

@avar 👍

[coveralls]

Coverage increased (+0.04%) to 81.88% when pulling 7db582b on http-headers-fast into db4148f on master.

[miyagawa]

#!/usr/bin/env perl
use strict;
use HTTP::Message::PSGI qw(req_to_psgi);
use HTTP::Request::Common qw(GET);
use Plack::Request;
use Benchmark qw(timethis);

my $req = GET "http://localhost/?foo=bar", "User-Agent" => "Mozilla/5.0";
my $env = req_to_psgi($req);

timethis 100000, sub { Plack::Request->new($env)->header("User-Agent") };

I've done a pretty un-scientific benchmark with the simple code, and the performance increase was about 2%. Probably negligible, but it's not slower either.

Might be more beneficial when there are more header variables and more operations to get the header value though.

[avar]

Just as an update I now have this running in prod on one of our roles, it doesn't do a lot of headers, and only sets a few so no idea how significant this'll be, but we do spend 3% of our time there in HTTP::Headers::_header.

[miyagawa]

Do you see any improvements in terms of performance? I'm happy to pull this in as long as it doesn't degrade the perf, though. as far as I can see the "Fast" bit is the header rearrangement, and accessing the header doesn't make a big difference.

[avar]

It is faster, but I'd revert this for now due to incompatibilities.

I've been running this on 2x boxes, one with the old code and one without, both are using statprofiler to get statistical sampling of performance time.

The total time spent in HTTP::Headers is 3.26% for my app, v.s. 1.94% for ::Fast, so a clear benefit.

However if you clone https://github.com/libwww-perl/http-message.git and search/replace HTTP::Headers->new with HTTP::Headers::Fast->new in t/headers.t you get quite a few failures. Including scary stuff like this (there are more failures than the ones I'm quoting here):

$h = HTTP::Headers::Fast->new;
$h->header(abc_abc   => "foo");
$h->header("abc-abc" => "bar");

ok($h->header("ABC_ABC"), "foo"); # line 401
ok($h->header("ABC-ABC"),"bar");
ok($h->remove_header("Abc_Abc"));
ok(!defined($h->header("abc_abc")));
ok($h->header("ABC-ABC"), "bar"); # line 405

Failing with:

# Test 151 got: "bar" (t/headers.t at line 401)
#     Expected: "foo"
#  t/headers.t line 401 is:     ok($h->header("ABC_ABC"), "foo");
# Test 155 got: <UNDEF> (t/headers.t at line 405)
#     Expected: "bar"
#  t/headers.t line 405 is:     ok($h->header("ABC-ABC"), "bar");

And:

Can't locate object method "flatten" via package
"HTTP::Headers::Fast" at t/headers.t line 467.

I've filed tokuhirom/HTTP-Headers-Fast#3 against
HTTP::Header::Fast, but since Plack has documented returning a
HTTP::Header object for a long time I'd suggest either reverting this
or holding off on doing a release until these bugs are fixed.

[miyagawa]

Thanks. I reverted it.

[miyagawa]

As commented on tokuhirom/HTTP-Headers-Fast#3 these test failures are either minor illegality check, newly introduced method that's not available on CPAN yet, and a false failure because of subtle package variable change.

So I think this is already safe, and these are not bugs in HTTP::Headers::Fast at this point.

Re-merged with a cherry-pick on 9023699

[avar]

Yeah, makes sense, \o/.