Re: svn commit: r1586947 - in /subversion/trunk/subversion/libsvn_fs_fs: dag.c dag.h tree.c

Bert Huijben wrote:
>>  Author: stefan2
>>  URL: http://svn.apache.org/r1586947
>>  Log:
>>  Improve MT scalability of the FSFS DAG traversal code.

It seems the main problem here is simply that this log message summary line gives a false impression about the magnitude of this particular change.

>>  Error objects are a very expensive way to control the control flow
>>  as they carry their own pools, created from a thread-safe root pool.
>>
>>  dag_open should not return an error to open_path if the dirent
>>  cannot be found, pass a NULL node back instead.  This eliminates
>>  about 50% of all transitional error objects during log-y operations.

I totally support this particular kind of change. It's simply good interface design.

p.s. A minor commenting nit...

>>  Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
>>  ==========================================================
>>  @@ -1047,17 +1046,15 @@ open_path(parent_path_t **parent_path_p,
>>             if (cached_node)
>>               child = cached_node;
>>             else
>>  -            err = svn_fs_fs__dag_open(&child, here, entry, pool, iterpool);
>>  +            SVN_ERR(svn_fs_fs__dag_open(&child, here, entry, pool, iterpool));
>>
>>             /* "file not found" requires special handling.  */

That comment there is now redundant ...

>>  -          if (err && err->apr_err == SVN_ERR_FS_NOT_FOUND)
>>  +          if (child == NULL)
>>               {
>>  @@ -1073,9 +1070,6 @@ open_path(parent_path_t **parent_path_p,
>>                   }
>>               }
>>
>>  -          /* Other errors we return normally.  */

... as this one has now gone away.

>>  -          SVN_ERR(err);
>>  -

- Julian
Received on 2014-04-14 10:30:38 CEST