Skip Menu |

This queue is for tickets about the Cache-Memcached-Fast CPAN distribution.

Report information
The Basics
Id: 81782
Status: resolved
Priority: 0/
Queue: Cache-Memcached-Fast
People
Owner: Nobody in particular
Requestors: aholland [...] ecstuning.com
Cc:
AdminCc:
Bug Information
Severity: Wishlist
Broken in: 0.19
Fixed in: (no value)

History Show all quoted text
  Fri Dec 07 14:46:58 2012 aholland [...] ecstuning.com - Ticket created
Subject: Unnecessary string eval in constructor
When creating a new cache object, as in: Cache::Memcached::Fast->new A string eval is done to pull in the Compress::Zlib module. This makes the constructor a lot slower than it needs to be, in the case where compression is not used (probably the majority use-case). I have attached a patch so that this string eval is only performed when compression is actually needed (e.g. when compress_threshold > 0).
Subject: Fast.pm.diff
--- Fast.pm 2012-12-07 14:36:36.000000000 -0500 +++ Fast.pm.old 2012-12-07 14:34:21.000000000 -0500 @@ -565,10 +565,7 @@ _check_args(\%known_params, $conf); - if (not $conf->{compress_methods} - and $conf->{compress_compress_threshold} - and $conf->{compress_compress_threshold} >= 0 - and eval "require Compress::Zlib") { + if (not $conf->{compress_methods} and eval "require Compress::Zlib") { # Note that the functions below can't return false when # operation succeed. This is because "" and "0" compress to a # longer values (because of additional format data), and
  Fri Dec 07 14:49:36 2012 aholland [...] ecstuning.com - Correspondence added
From: aholland [...] ecstuning.com
Fixing file extension on the patch so that CPAN will show it.
Subject: Fast.pm.diff.txt
--- Fast.pm 2012-12-07 14:36:36.000000000 -0500 +++ Fast.pm.old 2012-12-07 14:34:21.000000000 -0500 @@ -565,10 +565,7 @@ _check_args(\%known_params, $conf); - if (not $conf->{compress_methods} - and $conf->{compress_compress_threshold} - and $conf->{compress_compress_threshold} >= 0 - and eval "require Compress::Zlib") { + if (not $conf->{compress_methods} and eval "require Compress::Zlib") { # Note that the functions below can't return false when # operation succeed. This is because "" and "0" compress to a # longer values (because of additional format data), and
  Fri Dec 07 15:13:50 2012 KROKI [...] cpan.org - Correspondence added
On Fri Dec 07 14:49:36 2012, aholland wrote: Show quoted text
> Fixing file extension on the patch so that CPAN will show it.
I'm sorry but I do not understand your patch, neigher the problem nor how it might help. The docs say the default value for compress_threshold is -1, so we never get to eval. Could you please elaborate?
  Fri Dec 07 15:13:51 2012 The RT System itself - Status changed from 'new' to 'open'
  Fri Dec 07 15:15:09 2012 aholland [...] ecstuning.com - Correspondence added
From: aholland [...] ecstuning.com
On Fri Dec 07 15:13:50 2012, KROKI wrote: Show quoted text
> On Fri Dec 07 14:49:36 2012, aholland wrote:
> > Fixing file extension on the patch so that CPAN will show it.
> > I'm sorry but I do not understand your patch, neigher the problem nor > how it might help. The docs say the default value for > compress_threshold is -1, so we never get to eval. Could you please > elaborate?
I got the diff backwards, sorry. The compress_threshold stuff in the if statement is new.
  Fri Dec 07 15:19:41 2012 KROKI [...] cpan.org - Correspondence added
On Fri Dec 07 15:13:50 2012, KROKI wrote: Show quoted text
> On Fri Dec 07 14:49:36 2012, aholland wrote:
> > Fixing file extension on the patch so that CPAN will show it.
> > I'm sorry but I do not understand your patch, neigher the problem nor > how it might help. The docs say the default value for > compress_threshold is -1, so we never get to eval. Could you please > elaborate?
Ah, I see it now, your patch switches +++ and ---. Probably should be - if (not $conf->{compress_methods} and eval "require Compress::Zlib") { + if (not $conf->{compress_methods} + and defined $conf->{compress_threshold} + and $conf->{compress_threshold} >= 0 + and eval "require Compress::Zlib") {
  Fri Dec 07 15:58:23 2012 KROKI [...] cpan.org - Correspondence added
On Fri Dec 07 15:19:41 2012, KROKI wrote: Show quoted text
> Ah, I see it now, your patch switches +++ and ---. Probably should be
The patch will eventually get to 0.20. Still, the bootstrapping of C::M::F (constructor + TCP connect to each server on first request) should not be relayed on as being fast. To get most of C::M::F one should reuse bootstrapped module instances as long as possible.
  Wed Apr 03 12:45:10 2013 KROKI [...] cpan.org - Correspondence added
Patch applied to 0.20.
  Wed Apr 03 12:45:11 2013 KROKI [...] cpan.org - Status changed from 'open' to 'resolved'