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
Download (untitled) / with headers
text/plain 291b
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
Download bla20.pl
text/x-perl 1.4k
#!/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
Download (untitled) / with headers
text/plain 541b
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).
Download (untitled) / with headers
text/plain 541b
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>
Download (untitled) / with headers
text/plain 1.4k
"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
Download (untitled) / with headers
text/plain 1.6k
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.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.