Skip Menu |

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

Report information
The Basics
Id: 25308
Status: resolved
Priority: 0/
Queue: Cache-FastMmap

People
Owner: Nobody in particular
Requestors: sgifford [...] suspectclass.com
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 1.14
Fixed in: (no value)



Subject: get method seems to fail when called recursively from within read_cb
The get method seems to fail when called recursively from within the read_cb sub. In my application, I'm dealing with heirarchical data, where each child node contains some information from its parent node. When the read_cb sub for the child tries to call a Cache::FastMmap object's get method, it seems that the parent gets cached, but not the child. This is in Perl 5.8.4 with Cache::Mmap 1.14 on Fedora Core 6 Linux. Here's a fairly minimal example which calculates fibonacci numbers. If I use fastfib, results aren't cached correctly; with slowfib everything works as expected. A slightly modified version of this works fine with Cache::Mmap. #!/usr/bin/perl use warnings; use strict; use Cache::FastMmap; my $cache = Cache::FastMmap->new( read_cb => sub { fastfib($_[1]); } ); my $count = 0; sub fastfib { my($n) = @_; if ($n <= 2) { my $r = 1; return \$r; } $count++; my $a = $cache->get($n-1) or die "Couldn't calculate fib(@{[$n-1]})\n"; my $b = $cache->get($n-2) or die "Couldn't calculate fib(@{[$n-2]})\n"; my $sum = $$a+$$b; return \$sum; } sub slowfib { my($n) = @_; if ($n <= 2) { my $r = 1; return \$r; } $count++; my $a = slowfib($n-1) or die "Couldn't calculate fib(@{[$n-1]})\n"; my $b = slowfib($n-2) or die "Couldn't calculate fib(@{[$n-2]})\n"; my $sum = $$a+$$b; return \$sum; } print ${$cache->get($ARGV[0])},"\n"; print "($count calculations)\n";
Here is a test case.
######################### use Test::More tests => 10; BEGIN { use_ok('Cache::FastMmap') }; use warnings; use strict; ######################### # Insert your test code below, the Test::More module is use()ed here so read # its man page ( perldoc Test::More ) for help writing this test script. my $FC = Cache::FastMmap->new( read_cb => sub { fastfib($_[1]); }, init_file => 1, ); ok( defined $FC,"Created Cache::FastMmap object" ); our $count = 0; sub fastfib { $count++; my($n) = @_; if ($n <= 2) { my $r = 1; return \$r; } my $a = $FC->get($n-1) or die "Couldn't calculate fib(@{[$n-1]})\n"; my $b = $FC->get($n-2) or die "Couldn't calculate fib(@{[$n-2]})\n"; my $sum = $$a+$$b; return \$sum; } is(${$FC->get(3)},2, "Fib(3) value, first call"); is($count,3, "Fib(3) call count, first call"); is(${$FC->get(3)},2, "Fib(3) value, second call"); is($count,3, "Fib(3) call count, second call"); is(${$FC->get(5)},5, "Fib(5) value, first call"); is($count,5, "Fib(5) call count, first call"); is(${$FC->get(5)},5, "Fib(5) value, first call"); is($count,5, "Fib(5) call count, second call");
Here is a patch. The problem was with locking. Each call to get unlocks all locked pages when it's done, so in a recursive call, the inner call unlocks pages locked by the outer call. My fix is to not keep pages locked when the read callback is called; this seems useful from a performance perspective, too, since if the callback is slow other processes could block for a long time waiting for it. It does change the semantics, though. If a get call is being processed when a second get call with the same key comes in, the old behavior would block the second process until the first was done, then return the cached value; this version will calculate the value a second time in the second process, and overwrite the value from the first call when it is done. Which behavior is better will depend on access patterns. Also, this is still not threadsafe. To make it threadsafe, unlock should unlock only the pages which it actually locked, instead of all pages. This would also enable a fix which didn't change the semantics.
diff -ur Cache-FastMmap-1.14/FastMmap.pm Cache-FastMmap-1.14-sg/FastMmap.pm --- Cache-FastMmap-1.14/FastMmap.pm 2006-10-19 21:49:47.000000000 -0400 +++ Cache-FastMmap-1.14-sg/FastMmap.pm 2007-03-06 15:06:46.000000000 -0500 @@ -516,8 +516,10 @@ # Hash value, lock page, read result my ($HashPage, $HashSlot) = $Cache->fc_hash($_[1]); + $Cache->fc_lock($HashPage); my ($Val, $Flags, $Found) = $Cache->fc_read($HashSlot, $_[1]); + $Cache->fc_unlock() unless $_[2] && $_[2]->{skip_unlock}; # Value not found, check underlying data store if (!$Found && (my $read_cb = $Self->{read_cb})) { @@ -537,9 +539,11 @@ # Get key/value len (we've got 'use bytes'), and do expunge check to # create space if needed my $KVLen = length($_[1]) + length($Val); - $Self->_expunge_page(2, 1, $KVLen); + $Cache->fc_lock($HashPage) unless $_[2] && $_[2]->{skip_unlock}; + $Self->_expunge_page(2, 1, $KVLen); $Cache->fc_write($HashSlot, $_[1], $Val, 0); + $Cache->fc_unlock() unless $_[2] && $_[2]->{skip_unlock}; } }
I've just released 1.15 which includes the new "allow_recursive" option to new() which unlocks the page during the callback.