The basic pattern of the code we are dealing with is:
my $path = cache_location(...);
if (valid_cache($path)) {
...;
}
With cache_location possibly doing a stat() of $path and valid_cache
doing at least one stat() as well. Rewrite it so that cache_location
always stats and checks the cache (since we always need it). This way
we can now guarantee this flow requires at *most* 3 stats rather than
5.
The replacement for the above code becomes:
my ($path, $is_valid) = cache_location(...);
if ($is_valid) {
...;
}
Signed-off-by: Niels Thykier <niels@thykier.net>
$DEBUG = 0 unless defined $DEBUG;
@EXPORT = ();
$DEBUG = 0 unless defined $DEBUG;
@EXPORT = ();
- %EXPORT_TAGS = (libravatar => [qw(cache_valid retrieve_libravatar cache_location)]
+ %EXPORT_TAGS = (libravatar => [qw(retrieve_libravatar cache_location)]
);
@EXPORT_OK = ();
Exporter::export_ok_tags(keys %EXPORT_TAGS);
$EXPORT_TAGS{all} = [@EXPORT_OK];
}
);
@EXPORT_OK = ();
Exporter::export_ok_tags(keys %EXPORT_TAGS);
$EXPORT_TAGS{all} = [@EXPORT_OK];
}
-sub cache_valid{
- my ($cache_location) = @_;
- if (-e $cache_location) {
- if (time - (stat($cache_location))[9] < 60*60) {
- return 1;
- }
- }
- return 0;
-}
} else {
# figure out if the cache is now valid; if it is, return the
# cache location
} else {
# figure out if the cache is now valid; if it is, return the
# cache location
- my $temp_location = cache_location(email => $param{email});
- if (cache_valid($temp_location)) {
+ my ($temp_location, $is_valid) = cache_location(email => $param{email});
+ if ($is_valid) {
return $temp_location;
}
}
return $temp_location;
}
}
+# Returns ($path, $is_valid)
+# - For blocked images, $path will be undef
+# - If $is_valid is false (and $path is not undef), the image should
+# be re-fetched.
sub cache_location {
my %param = @_;
sub cache_location {
my %param = @_;
if (exists $param{md5sum}) {
$md5sum = $param{md5sum};
}elsif (exists $param{email}) {
if (exists $param{md5sum}) {
$md5sum = $param{md5sum};
}elsif (exists $param{email}) {
} else {
croak("cache_location must be called with one of md5sum or email");
}
} else {
croak("cache_location must be called with one of md5sum or email");
}
- return undef if blocked_libravatar($param{email},$md5sum);
- for my $ext (qw(.png .jpg)) {
- if (-e $config{libravatar_cache_dir}.'/'.$md5sum.$ext) {
- return $config{libravatar_cache_dir}.'/'.$md5sum.$ext;
+ return (undef, 0) if blocked_libravatar($param{email},$md5sum);
+ $stem = $config{libravatar_cache_dir}.'/'.$md5sum;
+ for my $ext ('.png', '.jpg', '') {
+ my $path = $stem.$ext;
+ if (-e $path) {
+ my $is_valid = (time - (stat(_))[9] < 60*60) ? 1 : 0;
+ return ($path, $is_valid);
- return $config{libravatar_cache_dir}.'/'.$md5sum;
}
## the following is mod_perl specific
}
## the following is mod_perl specific
return Apache2::Const::DECLINED();
}
# figure out what the md5sum of the e-mail is.
return Apache2::Const::DECLINED();
}
# figure out what the md5sum of the e-mail is.
- my $cache_location = cache_location(email => $email);
+ my ($cache_location, $is_valid) = cache_location(email => $email);
# if we've got it, and it's less than one hour old, return it.
# if we've got it, and it's less than one hour old, return it.
- if (cache_valid($cache_location)) {
serve_cache_mod_perl($cache_location,$r);
return Apache2::Const::DECLINED();
}
serve_cache_mod_perl($cache_location,$r);
return Apache2::Const::DECLINED();
}
# figure out what the md5sum of the e-mail is.
my $email_md5sum = md5_hex(lc($param{email}));
# figure out what the md5sum of the e-mail is.
my $email_md5sum = md5_hex(lc($param{email}));
-my $cache_location = cache_location(email => lc($param{email}));
+my ($cache_location, $is_valid) = cache_location(email => lc($param{email}));
# if we've got it, and it's less than one hour old, return it.
# if we've got it, and it's less than one hour old, return it.
-if (cache_valid($cache_location)) {
serve_cache($cache_location,$q);
exit 0;
}
serve_cache($cache_location,$q);
exit 0;
}