Dave,
I've used the new caching infrastructure to implement a
revision->pack-file-offset cache on the fsfs-pack branch. I've *think* that
I've implemented everything correctly, but for some reason it isn't working as
expected. Commenting out the call to svn_cache__get() removes the problems.
(You can see this effect by running fs-pack-test on the branch.)
On the very likely chance that the bug is something obvious, and is just my
(ab)use of the caching APIs, could you take a look at the attached patch let me
know if there are any glaring deficiencies?
Thanks,
-Hyrum
[[[
On the fsfs-pack branch:
Cache revision offsets in the pack file, which prevents repeated opening and
reading of the corresponding manifest file. I believe this change may also
create performance parity with non-packed repositories, in terms of number
of files opened and seeks performed, in the common case.
* subversion/libsvn_fs_fs/fs_fs.c
(get_packed_offset): Set and get the cache of revision->offset mappings.
* subversion/libsvn_fs_fs/fs.h
(fs_fs_data_t): Add the packed offset cache.
* subversio/libsvn_fs_fs/caching.c
(offset_serialize, offset_deserialize, dup_pack_offset): New.
(svn_fs_fs__initialize_caches): Initialize the offset cache.
]]]
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 34464)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -1608,8 +1608,15 @@ get_packed_offset(apr_off_t *rev_offset,
apr_file_t *manifest_file;
char buf[PACK_MANIFEST_ENTRY_LEN + 1];
apr_off_t rev_index_offset;
+ svn_boolean_t is_cached;
apr_size_t len;
+ SVN_ERR(svn_cache__get((void **) &rev_offset, &is_cached,
+ ffd->packed_offset_cache, &rev, pool));
+
+ if (is_cached)
+ return SVN_NO_ERROR;
+
/* Open the manifest file. */
SVN_ERR(svn_io_file_open(&manifest_file, path_rev_packed(fs, rev, "manifest",
pool),
@@ -1628,7 +1635,7 @@ get_packed_offset(apr_off_t *rev_offset,
*rev_offset = apr_atoi64(buf);
- return SVN_NO_ERROR;
+ return svn_cache__set(ffd->packed_offset_cache, &rev, rev_offset, pool);
}
/* Open the revision file for revision REV in filesystem FS and store
Index: subversion/libsvn_fs_fs/fs.h
===================================================================
--- subversion/libsvn_fs_fs/fs.h (revision 34458)
+++ subversion/libsvn_fs_fs/fs.h (working copy)
@@ -223,6 +223,10 @@ typedef struct
rep key to svn_string_t. */
svn_cache__t *fulltext_cache;
+ /* Pack manifest cache; maps revision numbers to offsets in their respective
+ pack files. */
+ svn_cache__t *packed_offset_cache;
+
/* Data shared between all svn_fs_t objects for a given filesystem. */
fs_fs_shared_data_t *shared;
Index: subversion/libsvn_fs_fs/caching.c
===================================================================
--- subversion/libsvn_fs_fs/caching.c (revision 34449)
+++ subversion/libsvn_fs_fs/caching.c (working copy)
@@ -104,6 +104,45 @@ dup_dir_listing(void **out,
return SVN_NO_ERROR;
}
+
+/** Caching packed rev offsets. **/
+/* Implements svn_cache__serialize_func_t */
+static svn_error_t *
+offset_serialize(char **data,
+ apr_size_t *data_len,
+ void *in,
+ apr_pool_t *pool)
+{
+ *data = apr_off_t_toa(pool, *((apr_off_t *) in));
+ *data_len = strlen(*data);
+ return SVN_NO_ERROR;
+}
+
+/* Implements svn_cache__deserialize_func_t */
+static svn_error_t *
+offset_deserialize(void **out,
+ const char *data,
+ apr_size_t data_len,
+ apr_pool_t *pool)
+{
+ *out = apr_palloc(pool, sizeof (apr_off_t));
+ apr_strtoff((apr_off_t *)*out, data, NULL, 0);
+
+ return SVN_NO_ERROR;
+}
+
+/* Implements svn_cache__dup_func_t */
+static svn_error_t *
+dup_pack_offset(void **out,
+ void *in,
+ apr_pool_t *pool)
+{
+ *out = apr_palloc(pool, sizeof(apr_off_t));
+ **((apr_off_t **) out) = *((apr_off_t *) in);
+ return SVN_NO_ERROR;
+}
+
+
/* Return a memcache in *MEMCACHE_P for FS if it's configured to use
memcached, or NULL otherwise. Also, sets *FAIL_STOP to a boolean
indicating whether cache errors should be returned to the caller or
@@ -218,7 +257,26 @@ svn_fs_fs__initialize_caches(svn_fs_t *fs,
SVN_ERR(svn_cache__set_error_handler(ffd->dir_cache,
warn_on_cache_errors, fs, pool));
+ /* Only 16 bytes per entry (a revision number + the corresponding offset). */
if (memcache)
+ SVN_ERR(svn_cache__create_memcache(&(ffd->packed_offset_cache),
+ memcache,
+ offset_serialize,
+ offset_deserialize,
+ sizeof(svn_revnum_t),
+ apr_pstrcat(pool, prefix, "PACK-OFFSET",
+ NULL),
+ fs->pool));
+ else
+ SVN_ERR(svn_cache__create_inprocess(&(ffd->packed_offset_cache),
+ dup_pack_offset, sizeof(svn_revnum_t),
+ 16, 1024, FALSE, fs->pool));
+
+ if (! no_handler)
+ SVN_ERR(svn_cache__set_error_handler(ffd->packed_offset_cache,
+ warn_on_cache_errors, fs, pool));
+
+ if (memcache)
{
SVN_ERR(svn_cache__create_memcache(&(ffd->fulltext_cache),
memcache,
Received on 2008-11-29 05:04:29 CET