It turns out that on trunk, we are using apr_int64_t which Daniel
tells me is still incorrect, so r1103665 applies your patch there, and
I've nominated it for 1.6.x (with Daniel's vote) in r1103668.
Thanks!
-Hyrum
On Mon, May 16, 2011 at 10:39 AM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> +1 from Hyrum and I; we'll apply to trunk and nominate a backport.
>
> Roderich Schupp wrote on Sun, May 15, 2011 at 19:50:53 +0200:
>> Hi,
>>
>> while looking at strace's of Subversion operations on a repository
>> using packing
>> I noticed the following behavior:
>>
>> 20829 open(".../db/revs/2.pack/pack", O_RDONLY|O_CLOEXEC) = 13
>> 20829 open(".../db/revs/2.pack/manifest", O_RDONLY|O_CLOEXEC) = 14
>> 20829 open(".../db/revs/2.pack/pack", O_RDONLY|O_CLOEXEC) = 14
>> 20829 open(".../db/revs/2.pack/manifest", O_RDONLY|O_CLOEXEC) = 15
>> 20829 open(".../db/revs/2.pack/pack", O_RDONLY|O_CLOEXEC) = 12
>> 20829 open(".../db/revs/2.pack/manifest", O_RDONLY|O_CLOEXEC) = 13
>>
>> i.e. every open for a pack file is paired with an open for the corresponding
>> manifest file. I would have expected that a specific manifest file is only
>> read once and then its contents are cached in the packed_offset_cache field
>> of the fs_fs_data_t struct. I saw this on Linux on amd64, i.e. where
>>
>> sizeof(int) < sizeof(rev_num_t)
>>
>> The packed_offset_cache never finds anything because of the following mismatch:
>> - in subversion/libsvn_fs_fs/caching.c packed_offset_cache is created
>> with a key length of
>> sizeof(svn_revnum_t)
>> - in subversion/libsvn_fs_fs/fs_fs.c in function get_packed_offset the
>> shard number is passed
>> to svn_cache__{get,set} as (the address of) an int
>>
>> Patch below. The problem is still present in branches/1.6.x, while trunk has
>> rewritten packed_offset_cache() and shard is now an apr_int64_t (which
>> will work,
>> though still not literally consistent with the creation of the cache).
>>
>> Cheers, Roderich
>>
>>
>> --- subversion-1.6.16.origsubversion/libsvn_fs_fs/fs_fs.c 2011-02-14
>> 21:54:43.000000000 +0100
>> +++ subversion-1.6.16/subversion/libsvn_fs_fs/fs_fs.c 2011-05-14
>> 22:11:39.127424984 +0200
>> @@ -1800,7 +1800,7 @@
>> fs_fs_data_t *ffd = fs->fsap_data;
>> svn_stream_t *manifest_stream;
>> svn_boolean_t is_cached;
>> - int shard;
>> + svn_revnum_t shard;
>> apr_array_header_t *manifest;
>> apr_pool_t *iterpool;
>
Received on 2011-05-16 12:06:22 CEST