> Index: subversion/libsvn_fs_fs/tree.c
> ===================================================================
> /* Invalidate cache entries for PATH and any of its children. */
> -static void
> +static svn_error_t *
> dag_node_cache_invalidate(svn_fs_root_t *root,
> - const char *path)
> + const char *path,
> + apr_pool_t *pool)
> {
> - fs_txn_root_data_t *frd;
> - apr_size_t len = strlen(path);
> - const char *key;
> - dag_node_cache_t *item;
> + struct fdic_baton b;
> + svn_cache_t *cache;
> + int i;
>
> + b.path = path;
> + b.pool = svn_pool_create(pool);
> + b.list = apr_array_make(b.pool, 1, sizeof(const char *));
> +
> assert(root->is_txn_root);
> + locate_cache(&cache, NULL, root, NULL, b.pool);
>
> - frd = root->fsap_data;
>
> - for (item = frd->txn_node_list.next;
> - item != &frd->txn_node_list;
> - item = item->next)
> + SVN_ERR(svn_cache_iter(NULL, cache, find_descendents_in_cache,
> + &b, b.pool));
> +
> + for (i = 0; i < b.list->nelts; i++)
> {
> - key = item->key;
> - if (strncmp(key, path, len) == 0 && (key[len] == '/' || !key[len]))
> - item->node = NULL;
> + const char *descendent = APR_ARRAY_IDX(b.list, i, const char *);
> + SVN_ERR(svn_cache_set(cache, descendent, NULL, b.pool));
Use an iterpool for this loop?
> +make_txn_root(svn_fs_root_t **root_p,
> + svn_fs_t *fs,
> const char *txn,
> svn_revnum_t base_rev,
> apr_uint32_t flags,
> @@ -3801,11 +3775,14 @@
> root->txn_flags = flags;
> root->rev = base_rev;
>
> - frd->txn_node_cache = apr_hash_make(root->pool);
> - frd->txn_node_list.prev = &frd->txn_node_list;
> - frd->txn_node_list.next = &frd->txn_node_list;
> + /* Because this cache actually tries to invalidate elements, keep
> + the number of elements per page down. */
> + SVN_ERR(svn_cache_create(&(frd->txn_node_cache),
> + svn_fs_fs__dag_dup_for_cache, APR_HASH_KEY_STRING,
> + 32, 20, FALSE, root->pool));
We don't have to go there right now, but it seems unlikely to me
that one cache size fits all; should we allow admins to tune them?
> Index: subversion/libsvn_fs_fs/fs.c
> ===================================================================
> @@ -114,6 +122,50 @@
>
> ffd->shared = ffsd;
>
> + /* Now do it all again for the caches; this key contains the path. */
> + key = apr_pstrcat(pool, SVN_FSFS_SHARED_CACHES_USERDATA_PREFIX, ffd->uuid,
> + "/", fs->path, (char *) NULL);
> + status = apr_pool_userdata_get(&val, key, common_pool);
> + if (status)
> + return svn_error_wrap_apr(status, _("Can't fetch FSFS shared caches data"));
> + ffsc = val;
> +
> + if (!ffsc)
> + {
> + ffsc = apr_pcalloc(common_pool, sizeof(*ffsc));
> +
> + /* Make the cache for revision roots. For the vast majority of
> + * commands, this is only going to contain a few entries (svnadmin
> + * dump/verify is an exception here), so to reduce overhead let's
> + * try to keep it to just one page. I estimate each entry has about
> + * 72 bytes of overhead (svn_revnum_t key, svn_fs_id_t +
> + * id_private_t + 3 strings for value, and the cache_entry); the
> + * default pool size is 8192, so about a hundred should fit
> + * comfortably. */
> + SVN_ERR(svn_cache_create(&(ffsc->rev_root_id_cache),
> + dup_ids, sizeof(svn_revnum_t),
> + 1, 100, TRUE, common_pool));
> +
> + /* Rough estimate: revision DAG nodes have size around 320 bytes, so
> + * let's put 16 on a page. */
> + SVN_ERR(svn_cache_create(&(ffsc->rev_node_cache),
> + svn_fs_fs__dag_dup_for_cache,
> + APR_HASH_KEY_STRING,
> + 1024, 16, TRUE, common_pool));
> +
> + /* Very rough estimate: 1K per directory. */
> + SVN_ERR(svn_cache_create(&(ffsc->dir_cache),
> + dup_dir_listing, APR_HASH_KEY_STRING,
> + 1024, 8, TRUE, common_pool));
As we discussed in person, allocating these things in a
process-lifetime pool (common_pool in fs-loader.c) is probably
not a problem, but worth documenting.
> +/* We have two different "shared between all svn_fs_t objects for the
> + same filesystem" structures, allocated in the common pool. Why two?
[...]
Thanks for all the great comments; they really helped.
I had a lot less to say on this round, so I may have missed
potential problems if I didn't sufficiently understand some
surrounding context of existing code. It all looked right, but
it would be nice to have another person as familiar with fsfs as
you to review this, too.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-03-26 01:08:14 CET