Skip Menu |

This queue is for tickets about the libwww-perl CPAN distribution.

Report information
The Basics
Id: 39611
Status: resolved
Priority: 0/
Queue: libwww-perl

People
Owner: Nobody in particular
Requestors: SREZIC [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Wishlist
Broken in: 5.814
Fixed in: (no value)



Subject: Use Storable::dclone for HTTP::Headers::clone
It seems that the HTTP::Response::clone method is slower than Storable::dclone. So my suggestion is to use Storable::dclone instead of HTTP::Response::clone if Storable is available or a global variable instructs HTTP::Response to do so. Attached is a benchmark script. Regards, Slaven
Subject: bla20.pl
#!/usr/bin/perl use strict; use warnings; no warnings 'redefine'; use Benchmark qw(cmpthese); use HTTP::Response; use HTTP::Headers; use Storable qw(dclone); use Test::More 'no_plan'; use YAML::Syck; my $http_headers = YAML::Syck::Load(<<EOF); --- !!perl/hash:HTTP::Headers accept-ranges: bytes client-date: Thu, 25 Sep 2008 13:10:21 GMT client-peer: 172.17.30.1:84 client-response-num: 10 content-length: 4122 content-type: image/jpeg date: Thu, 25 Sep 2008 13:10:21 GMT etag: '"1746eb-101a-b09afc40"' last-modified: Thu, 18 Sep 2008 13:37:45 GMT server: Apache/2.2.3 (Debian) PHP/5.2.5-3 with Suhosin-Patch mod_perl/2.0.3 Perl/v5.8.8 EOF # cmpthese(-1, { # using_http_response_clone => sub { # HTTP::Response->new(undef, undef, $http_headers); # }, # using_storable_clone => sub { # local *HTTP::Headers::clone = sub { dclone shift }; # HTTP::Response->new(undef, undef, $http_headers); # }, # } # ); my $res1; my $res2; cmpthese(-1, { http_response_clone => sub { $res1 = $http_headers->clone; }, using_storable_clone => sub { local *HTTP::Headers::clone = sub { dclone shift }; $res2 = dclone $http_headers; }, } ); is_deeply($res2, $res1); __END__ Rate http_response_clone using_storable_clone http_response_clone 5440/s -- -64% using_storable_clone 15175/s 179% -- ok 1 1..1
What's your use case for fast cloning and why can't you then just use dclone() directly? I see from the implementation that we intentionally don't clone the response linked via the 'previous' attribute, so this would be a difference from invoking dclone. I don't remember why it was decided to do it this way. Other ad-hoc attributes will also not be copied by the clone method which would be another difference. I'm not set on not changing this, I just want to understand why you ask for this (besides from faster is always better).
What's your use case for fast cloning and why can't you then just use dclone() directly? I see from the implementation that we intentionally don't clone the response linked via the 'previous' attribute, so this would be a difference from invoking dclone. I don't remember why it was decided to do it this way. Other ad-hoc attributes will also not be copied by the clone method which would be another difference. I'm not set on not changing this, I just want to understand why you ask for this (besides from faster is always better).
CC: SREZIC [...] cpan.org
Subject: Re: [rt.cpan.org #39611] Use Storable::dclone for HTTP::Headers::clone
Date: 25 Sep 2008 21:26:16 +0200
To: bug-libwww-perl [...] rt.cpan.org
From: Slaven Rezic <slaven [...] rezic.de>
"Gisle_Aas via RT" <bug-libwww-perl@rt.cpan.org> writes: Show quoted text
> <URL: http://rt.cpan.org/Ticket/Display.html?id=39611 > > > What's your use case for fast cloning and why can't you then just use dclone() directly? > > I see from the implementation that we intentionally don't clone the response linked via the > 'previous' attribute, so this would be a difference from invoking dclone. I don't remember why > it was decided to do it this way.
I think this is only for HTTP::Response::clone, but I was refering specifically to HTTP::Headers::clone. Using scan() there seems to slow things down. Show quoted text
> > Other ad-hoc attributes will also not be copied by the clone method which would be another > difference. > > I'm not set on not changing this, I just want to understand why you ask for this (besides from > faster is always better). >
In my application I have HTTP::Header objects of a previous HTTP response stored to disk. To use the freshness_lifetime() and is_fresh() methods, I have to create a new HTTP::Response object which is populated with the stored HTTP::Header (something like HTTP::Response->new(undef, undef, $http_headers)). When profiling the application with the NYT profiler I found that quite a lot time was lost in the clone() method. (Another optimization candidate would be the _date_header() method, but this is another story) Regards, Slaven -- Slaven Rezic - slaven <at> rezic <dot> de Berlin Perl Mongers - http://berlin.pm.org
On Thu Sep 25 15:29:23 2008, slaven@rezic.de wrote: Show quoted text
> "Gisle_Aas via RT" <bug-libwww-perl@rt.cpan.org> writes: >
> > <URL: http://rt.cpan.org/Ticket/Display.html?id=39611 > > > > > What's your use case for fast cloning and why can't you then just
> use dclone() directly?
> > > > I see from the implementation that we intentionally don't clone the
> response linked via the
> > 'previous' attribute, so this would be a difference from invoking
> dclone. I don't remember why
> > it was decided to do it this way.
> > I think this is only for HTTP::Response::clone, but I was refering > specifically to HTTP::Headers::clone. Using scan() there seems to slow > things down. >
> > > > Other ad-hoc attributes will also not be copied by the clone method
> which would be another
> > difference. > > > > I'm not set on not changing this, I just want to understand why you
> ask for this (besides from
> > faster is always better). > >
> > In my application I have HTTP::Header objects of a previous HTTP > response stored to disk. To use the freshness_lifetime() and > is_fresh() methods, I have to create a new HTTP::Response object which > is populated with the stored HTTP::Header (something like > HTTP::Response->new(undef, undef, $http_headers)). > > When profiling the application with the NYT profiler I found that > quite a lot time was lost in the clone() method. (Another optimization > candidate would be the _date_header() method, but this is another > story) > > Regards, > Slaven >
Here's a sample implementation for this: git://bbbike.dyndns.org/libwww-perl Note that this machine may not be up all the time. Regards, Slaven
Thanks. I've applied your patch now and then I removed the $USE_STORABLE_DCLONE configuration.
Thanks. I've applied your patch now and then I removed the $USE_STORABLE_DCLONE configuration.