On Wed, May 27, 2015 at 11:28 AM, Ivan Zhakov <i...@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.
