stefan2_at_apache.org wrote on Sat, Mar 28, 2015 at 10:58:13 -0000:
> Author: stefan2
> Date: Sat Mar 28 10:58:13 2015
> New Revision: 1669743
>
> URL: http://svn.apache.org/r1669743
> Log:
> Follow-up to r1654934:
> Relax the size check in the FSFS representation sharing code to make it as
> effective as in earlier releases.
>
> The size check has been to strict and would not allow two matching reps to
> have a different base representation, e.g. mod on /trunk and add-without-
> history on a branch. Mainly depending on the merge policy (catch-up vs.
> cherry-pick), this resulted in detecting and eliding fewer instances of
> redundant representations. Hence, larger repositories.
>
> * subversion/libsvn_fs_fs/transaction.c
> (get_shared_rep): Care about the on-disk size only if the fulltext /
> expanded size is not known - which would be the case
> for e.g. PLAIN representations.
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_fs/transaction.c
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/transaction.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/transaction.c?rev=1669743&r1=1669742&r2=1669743&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/transaction.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/transaction.c Sat Mar 28 10:58:13 2015
> @@ -2214,10 +2214,11 @@ get_shared_rep(representation_t **old_re
> return SVN_NO_ERROR;
>
> /* We don't want 0-length PLAIN representations to replace non-0-length
> - ones (see issue #4554). Also, this doubles as a simple guard against
> - general rep-cache induced corruption. */
> + ones (see issue #4554). Take into account that EXPANDED_SIZE may be
> + 0 in which case we have to check the on-disk SIZE. Also, this doubles
> + as a simple guard against general rep-cache induced corruption. */
> if ( ((*old_rep)->expanded_size != rep->expanded_size)
> - || ((*old_rep)->size != rep->size))
> + || (rep->expanded_size && ((*old_rep)->size != rep->size)))
If two reps have EXPANDED_SIZE == 0 and different SIZE, the condition
will be FALSE. Shouldn't it be TRUE instead?
When comparing SIZE, shouldn't it also compare the rep kind (PLAIN or
DELTA) at the same time? Otherwise, two reps that have the same
EXPANDED_SIZE, different SIZE, and different kinds wouldn't be shared,
even though they could be.
Daniel
> {
> *old_rep = NULL;
> }
>
>
Received on 2015-03-29 09:45:14 CEST