Re: fsfs-improvements branch complete

[Philip Martin <ph...@wandisco.com>]

Stefan Fuhrmann <st...@wandisco.com> writes:

> After two weeks now, I finally completed the fsfs format 6 refactoring
> and improvement work on said branch. Please review. See also
> http://svn.haxx.se/dev/archive-2013-07/0385.shtml for the "split up
> fs_fs.c" part of it.
>
> If there are no objections, I will merge the code in the week of Aug 26th.

I'm wondering whether this:

+/** Take the #svn_fs_dirent_t structures in @a entries as returned by
+ * #svn_fs_dir_entries for @a root and determine an optimized ordering
+ * in which data access would most likely be efficient.  Set @a *ordered_p
+ * to a newly allocated APR array of pointers to these #svn_fs_dirent_t
+ * structures.  Allocate the array (but not its contents) in @a pool.
+ *
+ * @since New in 1.9.
+ */
+svn_error_t *
+svn_fs_dir_optimal_order(apr_array_header_t **ordered_p,
+                         svn_fs_root_t *root,
+                         apr_hash_t *entries,
+                         apr_pool_t *pool);

should have two pools?  I can see that in lots of cases the optimal
ordering is something the backend produces with little effort, that's
the case for the current implementations, and so a scratch pool is not
necessary.  Lots of FS functions are single pool because they were
defined before we started using two pools.  What should new functions
do?

-- 
Philip Martin | Subversion Committer
WANdisco | Non-Stop Data

Re: fsfs-improvements branch complete

[Stefan Fuhrmann <st...@wandisco.com>]

On Thu, Aug 8, 2013 at 12:42 PM, Philip Martin
<ph...@wandisco.com>wrote:

> Stefan Fuhrmann <st...@wandisco.com> writes:
>
> > After two weeks now, I finally completed the fsfs format 6 refactoring
> > and improvement work on said branch. Please review. See also
> > http://svn.haxx.se/dev/archive-2013-07/0385.shtml for the "split up
> > fs_fs.c" part of it.
> >
> > If there are no objections, I will merge the code in the week of Aug
> 26th.
>
> I'm wondering whether this:
>
> +/** Take the #svn_fs_dirent_t structures in @a entries as returned by
> + * #svn_fs_dir_entries for @a root and determine an optimized ordering
> + * in which data access would most likely be efficient.  Set @a *ordered_p
> + * to a newly allocated APR array of pointers to these #svn_fs_dirent_t
> + * structures.  Allocate the array (but not its contents) in @a pool.
> + *
> + * @since New in 1.9.
> + */
> +svn_error_t *
> +svn_fs_dir_optimal_order(apr_array_header_t **ordered_p,
> +                         svn_fs_root_t *root,
> +                         apr_hash_t *entries,
> +                         apr_pool_t *pool);
>
> should have two pools?  I can see that in lots of cases the optimal
> ordering is something the backend produces with little effort, that's
> the case for the current implementations, and so a scratch pool is not
> necessary.  Lots of FS functions are single pool because they were
> defined before we started using two pools.  What should new functions
> do?
>

Yes, we could use 2 pools for new functions and
I feel more or less indifferent about it. However, I
would suggest the following guideline to maximize
symmetry:

* keep all vtable entries in SVN_FS singly pooled,
  including new ones
* make all functions in SVN_FS2 follow the new
  two-pool pattern

-- Stefan^2.