Re: [PATCH] port of performance improvements from HTTP::Headers::Fast
Gisle Aas <[email protected]>
| Newsgroups | gmane.comp.lang.perl.modules.lwp |
|---|---|
| Message-ID | <[email protected]> |
I'm not comfortable with this patch. Some reasons:
- it's one big patch (actually there is 2)
- it does not follow the layout style in use, e.g. introduces cuddled elses
- it does various whitespace reformats
- introduces ways that are not optimizations
(like replacing $op eq "GET" with $op == $OP_GET)
- introduces unused functions, e.g. _header_push_no_return
I do like to see performance improvements, so I would not mind smaller
patches with demonstrated good effects. Having HTTP::Headers::Fast as
a benchmark to beat it good. I don't consider all the benchmarks in
[1] valid. I don't think the speed of pushing thousands of values
onto a header important. The speed of getting and setting single
values are.
I'm also slightly annoyed by the HTTP::Headers::Fast author for
copying my code and then claiming[2] he wrote it.
[1] http://github.com/markstos/p5-http-headers-fast/blob/master/tools/benchmark.pl
[2] http://search.cpan.org/dist/HTTP-Headers-Fast/lib/HTTP/Headers/Fast.pm#AUTHOR
--Gisle
On Tue, Jan 26, 2010 at 17:38, Mark Stosberg <[email protected]> wrote:
>
> I've now published some patches in "git" that port the performance
> improvements of HTTP::Headers::Fast back to HTTP::Headers:
>
> http://github.com/markstos/libwww-perl/commits/http-headers-fast
>
> The changes benchmark to be 10 to 20% faster on average and pass all of
> the HTTP::Headers regression tests.
>
> As I just mentioned in a previous post, it also adds a new method to
> generate the headers in an unsorted order, for better performance. The
> behavior of "as_string()" is not changed.
>
> Mark