You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ivan Zhakov <iv...@visualsvn.com> on 2015/04/13 17:01:09 UTC
Re: svn commit: r1673202 - /subversion/trunk/subversion/libsvn_fs_fs/dag.c
On 13 April 2015 at 17:22, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Mon Apr 13 14:22:33 2015
> New Revision: 1673202
>
> URL: http://svn.apache.org/r1673202
> Log:
> Following up on r1673197, use another optimization suggested by stefan2.
>
> * subversion/libsvn_fs_fs/dag.c
> (svn_fs_fs__dag_has_props): Use expanded size to determine if the
> representation stores properties vs just an empty list.
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_fs/dag.c
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/dag.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c?rev=1673202&r1=1673201&r2=1673202&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/dag.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/dag.c Mon Apr 13 14:22:33 2015
> @@ -525,15 +525,9 @@ svn_fs_fs__dag_has_props(svn_boolean_t *
> }
> else
> {
> - apr_hash_t *proplist;
> -
> - /* ### Optimize further.
> - Stefan2 suggested: prop_rep exists and is longer than 4 bytes
> - */
> - SVN_ERR(svn_fs_fs__get_proplist(&proplist, node->fs,
> - noderev, scratch_pool));
> -
> - *has_props = proplist ? (0 < apr_hash_count(proplist)) : FALSE;
> + /* Properties are stored as a standard hash stream,
> + always ending with "END\n" (4 bytes) */
> + *has_props = (noderev->prop_rep->expanded_size > 4);
I'm not sure, but strictly speaking prop representation greater than
four bytes doesn't mean that there is no props: property
representation may contain delta resulting empty property list.
--
Ivan Zhakov
Re: svn commit: r1673202 - /subversion/trunk/subversion/libsvn_fs_fs/dag.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Apr 13, 2015 at 7:29 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On 13 April 2015 at 20:13, Stefan Fuhrmann <st...@wandisco.com>
> wrote:
> > On Mon, Apr 13, 2015 at 5:01 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>
> > expanded_size is the size of the undeltified (full-text) representation.
> > For properties lists, it can only be 0 for PLAIN representations where
> > the on-disk size is then the full-text size.
> >
> OK. That's make sense. Thanks for explanation.
>
> > On a higher level, the fulltext only contains the serialized "positive"
> > contents. An incremental representation like for directory reps in
> > transactions is not supported by svn_hash_read2. The 'structure'
> > doc does not hint on any support for incremental prop lists nor did
> > it ever work in a previous release.
> >
> What about rep-sharing scenario when prop representation becomes equal
> to result of some text file? Is not prop representation will be
> 'linked' to some delta chain?
>
Positively possible. Deltification as in "txdelta and chains of
txdelta reps" is not a problem - see first paragraph. Shared
reps as well as reps deltified against shared reps look just
the same as "unique" reps. So, if we were to share a file rep
with a prop rep, then the file rep needs to contain at least that
terminating "END\n". Do you have some problematic
constellation that you have in mind?
Deltification as in "incremental hash storage" allows you
to add "removal" entries to serialized hashes, i.e. you can
end up with quite a long textual rep that is effectively an
empty hash. That would be a problem. However, we only
allow that for directory hashes within txns.
-- Stefan^2.
Re: svn commit: r1673202 - /subversion/trunk/subversion/libsvn_fs_fs/dag.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 13 April 2015 at 20:13, Stefan Fuhrmann <st...@wandisco.com> wrote:
> On Mon, Apr 13, 2015 at 5:01 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>
>> On 13 April 2015 at 17:22, <rh...@apache.org> wrote:
>> > Author: rhuijben
>> > Date: Mon Apr 13 14:22:33 2015
>> > New Revision: 1673202
>> >
>> > URL: http://svn.apache.org/r1673202
>> > Log:
>> > Following up on r1673197, use another optimization suggested by stefan2.
>> >
>> > * subversion/libsvn_fs_fs/dag.c
>> > (svn_fs_fs__dag_has_props): Use expanded size to determine if the
>> > representation stores properties vs just an empty list.
>> >
>> > Modified:
>> > subversion/trunk/subversion/libsvn_fs_fs/dag.c
>> >
>> > Modified: subversion/trunk/subversion/libsvn_fs_fs/dag.c
>> > URL:
>> > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c?rev=1673202&r1=1673201&r2=1673202&view=diff
>> >
>> > ==============================================================================
>> > --- subversion/trunk/subversion/libsvn_fs_fs/dag.c (original)
>> > +++ subversion/trunk/subversion/libsvn_fs_fs/dag.c Mon Apr 13 14:22:33
>> > 2015
>> > @@ -525,15 +525,9 @@ svn_fs_fs__dag_has_props(svn_boolean_t *
>> > }
>> > else
>> > {
>> > - apr_hash_t *proplist;
>> > -
>> > - /* ### Optimize further.
>> > - Stefan2 suggested: prop_rep exists and is longer than 4 bytes
>> > - */
>> > - SVN_ERR(svn_fs_fs__get_proplist(&proplist, node->fs,
>> > - noderev, scratch_pool));
>> > -
>> > - *has_props = proplist ? (0 < apr_hash_count(proplist)) : FALSE;
>> > + /* Properties are stored as a standard hash stream,
>> > + always ending with "END\n" (4 bytes) */
>> > + *has_props = (noderev->prop_rep->expanded_size > 4);
>> I'm not sure, but strictly speaking prop representation greater than
>> four bytes doesn't mean that there is no props: property
>> representation may contain delta resulting empty property list.
>
>
> expanded_size is the size of the undeltified (full-text) representation.
> For properties lists, it can only be 0 for PLAIN representations where
> the on-disk size is then the full-text size.
>
OK. That's make sense. Thanks for explanation.
> On a higher level, the fulltext only contains the serialized "positive"
> contents. An incremental representation like for directory reps in
> transactions is not supported by svn_hash_read2. The 'structure'
> doc does not hint on any support for incremental prop lists nor did
> it ever work in a previous release.
>
What about rep-sharing scenario when prop representation becomes equal
to result of some text file? Is not prop representation will be
'linked' to some delta chain?
--
Ivan Zhakov
Re: svn commit: r1673202 - /subversion/trunk/subversion/libsvn_fs_fs/dag.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Apr 13, 2015 at 5:01 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On 13 April 2015 at 17:22, <rh...@apache.org> wrote:
> > Author: rhuijben
> > Date: Mon Apr 13 14:22:33 2015
> > New Revision: 1673202
> >
> > URL: http://svn.apache.org/r1673202
> > Log:
> > Following up on r1673197, use another optimization suggested by stefan2.
> >
> > * subversion/libsvn_fs_fs/dag.c
> > (svn_fs_fs__dag_has_props): Use expanded size to determine if the
> > representation stores properties vs just an empty list.
> >
> > Modified:
> > subversion/trunk/subversion/libsvn_fs_fs/dag.c
> >
> > Modified: subversion/trunk/subversion/libsvn_fs_fs/dag.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c?rev=1673202&r1=1673201&r2=1673202&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/libsvn_fs_fs/dag.c (original)
> > +++ subversion/trunk/subversion/libsvn_fs_fs/dag.c Mon Apr 13 14:22:33
> 2015
> > @@ -525,15 +525,9 @@ svn_fs_fs__dag_has_props(svn_boolean_t *
> > }
> > else
> > {
> > - apr_hash_t *proplist;
> > -
> > - /* ### Optimize further.
> > - Stefan2 suggested: prop_rep exists and is longer than 4 bytes
> > - */
> > - SVN_ERR(svn_fs_fs__get_proplist(&proplist, node->fs,
> > - noderev, scratch_pool));
> > -
> > - *has_props = proplist ? (0 < apr_hash_count(proplist)) : FALSE;
> > + /* Properties are stored as a standard hash stream,
> > + always ending with "END\n" (4 bytes) */
> > + *has_props = (noderev->prop_rep->expanded_size > 4);
> I'm not sure, but strictly speaking prop representation greater than
> four bytes doesn't mean that there is no props: property
> representation may contain delta resulting empty property list.
>
expanded_size is the size of the undeltified (full-text) representation.
For properties lists, it can only be 0 for PLAIN representations where
the on-disk size is then the full-text size.
On a higher level, the fulltext only contains the serialized "positive"
contents. An incremental representation like for directory reps in
transactions is not supported by svn_hash_read2. The 'structure'
doc does not hint on any support for incremental prop lists nor did
it ever work in a previous release.
-- Stefan^2.