You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2014/09/27 12:44:13 UTC
svn commit: r1627946 - /subversion/trunk/subversion/libsvn_fs_x/cached_data.c
Author: stefan2
Date: Sat Sep 27 10:44:13 2014
New Revision: 1627946
URL: http://svn.apache.org/r1627946
Log:
* subversion/libsvn_fs_x/cached_data.c
(svn_fs_x__rep_chain_length): The sub-pool is actually an iteration pool
used in a non-standard way. Rename and
document the usage.
Suggested by: rhuijben
Modified:
subversion/trunk/subversion/libsvn_fs_x/cached_data.c
Modified: subversion/trunk/subversion/libsvn_fs_x/cached_data.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/cached_data.c?rev=1627946&r1=1627945&r2=1627946&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/cached_data.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/cached_data.c Sat Sep 27 10:44:13 2014
@@ -772,13 +772,18 @@ svn_fs_x__rep_chain_length(int *chain_le
svn_revnum_t shard_size = ffd->max_files_per_dir
? ffd->max_files_per_dir
: 1;
- apr_pool_t *subpool = svn_pool_create(pool);
svn_boolean_t is_delta = FALSE;
int count = 0;
int shards = 1;
svn_revnum_t revision = svn_fs_x__get_revnum(rep->id.change_set);
svn_revnum_t last_shard = revision / shard_size;
-
+
+ /* Note that this iteration pool will be used in a non-standard way.
+ * To reuse open file handles between iterations (e.g. while within the
+ * same pack file), we only clear this pool once in a while instead of
+ * at the start of each iteration. */
+ apr_pool_t *iterpool = svn_pool_create(pool);
+
/* Check whether the length of the deltification chain is acceptable.
* Otherwise, shared reps may form a non-skipping delta chain in
* extreme cases. */
@@ -806,7 +811,7 @@ svn_fs_x__rep_chain_length(int *chain_le
&file_hint,
&base_rep,
fs,
- subpool));
+ iterpool));
base_rep.id.change_set
= svn_fs_x__change_set_by_rev(header->base_revision);
@@ -814,18 +819,21 @@ svn_fs_x__rep_chain_length(int *chain_le
base_rep.size = header->base_length;
is_delta = header->type == svn_fs_x__rep_delta;
+ /* Clear it the ITERPOOL once in a while. Doing it too frequently
+ * renders the FILE_HINT ineffective. Doing too infrequently, may
+ * leave us with too many open file handles. */
++count;
if (count % 16 == 0)
{
file_hint = NULL;
- svn_pool_clear(subpool);
+ svn_pool_clear(iterpool);
}
}
while (is_delta && base_rep.id.change_set);
*chain_length = count;
*shard_count = shards;
- svn_pool_destroy(subpool);
+ svn_pool_destroy(iterpool);
return SVN_NO_ERROR;
}
Re: svn commit: r1627946 - /subversion/trunk/subversion/libsvn_fs_x/cached_data.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Sun, Sep 28, 2014 at 12:41 AM, Branko Čibej <br...@wandisco.com> wrote:
> On 27.09.2014 12:44, stefan2@apache.org wrote:
> > Author: stefan2
> > Date: Sat Sep 27 10:44:13 2014
> > New Revision: 1627946
>
> > + /* Clear it the ITERPOOL once in a while. Doing it too frequently
> > + * renders the FILE_HINT ineffective. Doing too infrequently, may
> > + * leave us with too many open file handles. */
>
>
> This makes me wonder: there's no substantial difference between clearing
> the pool (and closing file) every 16 iterations, or doing that on every
> iteration. So, what's magical about 16, do we actually know how many
> files we keep open, does the code try to gracefully handle running out
> of file handles, and if not, does the whole pack operation go south when
> we do run out?
>
No, there is nothing magical about that number; it's just my
value of "once in a while". r1628083 should make that clear now.
-- Stefan^2.
Re: svn commit: r1627946 - /subversion/trunk/subversion/libsvn_fs_x/cached_data.c
Posted by Branko Čibej <br...@wandisco.com>.
On 27.09.2014 12:44, stefan2@apache.org wrote:
> Author: stefan2
> Date: Sat Sep 27 10:44:13 2014
> New Revision: 1627946
> + /* Clear it the ITERPOOL once in a while. Doing it too frequently
> + * renders the FILE_HINT ineffective. Doing too infrequently, may
> + * leave us with too many open file handles. */
This makes me wonder: there's no substantial difference between clearing
the pool (and closing file) every 16 iterations, or doing that on every
iteration. So, what's magical about 16, do we actually know how many
files we keep open, does the code try to gracefully handle running out
of file handles, and if not, does the whole pack operation go south when
we do run out?
-- Brane