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 <m...@summersault.com> 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

Reply via email to