[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Double cache lookup in svn_fs_fs__rep_contents_dir_entry()

From: Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
Date: Wed, 27 May 2015 11:49:25 +0200

On Wed, May 27, 2015 at 11:28 AM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:

> It seems directory cache checked twice in function
> svn_fs_fs__rep_contents_dir_entry:
> [[[
> svn_error_t *
> svn_fs_fs__rep_contents_dir_entry(svn_fs_dirent_t **dirent,
> svn_fs_t *fs,
> node_revision_t *noderev,
> const char *name,
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool)
> {
> svn_boolean_t found = FALSE;
>
> /* find the cache we may use */
> pair_cache_key_t pair_key = { 0 };
> const void *key;
> svn_cache__t *cache = locate_dir_cache(fs, &key, &pair_key, noderev,
> scratch_pool);
> if (cache)
> {
> [...]
> SVN_ERR(svn_cache__get_partial((void **)dirent,
> &found,
> cache,
> key,
> svn_fs_fs__extract_dir_entry,
> &baton,
> result_pool));
> }
>
> /* fetch data from disk if we did not find it in the cache */
> if (! found)
> {
> [...]
>
> /* read the dir from the file system. It will probably be put it
> into the cache for faster lookup in future calls. */
> SVN_ERR(svn_fs_fs__rep_contents_dir(&entries, fs, noderev,
> scratch_pool, scratch_pool));
>
> [...]
> }
>
> return SVN_NO_ERROR;
> }
> ]]]
>
> And svn_fs_fs__rep_contents_dir() functions checks the dir cache again.
>
> Is my analysis correct or I missed something important?
>

Your analysis is correct and the code is slightly less efficient
that it could be. Feel free to add e.g. a "bypass_cache_lookup"
flag to the svn_fs_fs__rep_contents_dir() signature.

However, the actual gains from this should be minimal because
the failed lookup is easily dwarfed by the directory parsing time.
Do you have a specific workload where the double lookup becomes
more noticeable?

-- Stefan^2.
Received on 2015-05-27 11:50:20 CEST

This is an archived mail posted to the Subversion Dev mailing list.