You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2012/04/19 00:35:38 UTC
Re: svn commit: r1327703 - in /subversion/trunk/subversion:
include/private/svn_subr_private.h libsvn_fs_base/util/fs_skels.c libsvn_subr/checksum.c
On Wed, Apr 18, 2012 at 18:11, <bl...@apache.org> wrote:
> Author: blair
> Date: Wed Apr 18 22:11:43 2012
> New Revision: 1327703
>
> URL: http://svn.apache.org/viewvc?rev=1327703&view=rev
> Log:
> Add and use svn_checksum__from_digest_sha1().
>
> This replaces svn_checksum__from_digest() taking a svn_checksum_sha1
> argument with a new svn_checksum_t constructor function.
>
> * subversion/include/private/svn_subr_private.h,
> * subversion/libsvn_subr/checksum.c
> (svn_checksum__from_digest_sha1):
> New private function.
>
> * subversion/libsvn_fs_base/util/fs_skels.c
> (svn_fs_base__parse_representation_skel):
> Replace
> svn_checksum__from_digest(digest, svn_checksum_sha1, pool);
> with
> svn_checksum__from_digest_sha1(digest, pool);
Shouldn't we be using svn_checksum_serialize() and _deserialize()
rather than raw digests?
(iow, we "shouldn't" need from_digest_sha1)
>...
Cheers,
-g
Re: svn commit: r1327703 - in /subversion/trunk/subversion:
include/private/svn_subr_private.h libsvn_fs_base/util/fs_skels.c libsvn_subr/checksum.c
Posted by Greg Stein <gs...@gmail.com>.
On Wed, Apr 18, 2012 at 19:15, Blair Zajac <bl...@orcaware.com> wrote:
> On 04/18/2012 03:45 PM, Greg Stein wrote:
>...
>> Yup, I understood. Sorry that I wasn't clear: I meant in our
>> serialization code, shouldn't we use the "proper" functions rather
>> than raw sha1 digests? Is there any way to switch to them at a
>> higher/semantic level?
>>
>> I haven't looked at that stuff, but I'm going to guess repositories
>> now exist with raw sha1 digests. Is there a format type in there? Can
>> we start writing "csum" and svn_checksum_serialize() into the skel?
>> And then read raw md5, raw sha1, or a serialized checksum?
>
> I'm not familiar with this part of the code either (not having looked at the
> svn_skel.h before) but that make sense. The code could look for a raw
> digest or a $name$ and then use that.
>
> It doesn't look like it would be too hard. Because the skel has a len, if
> it's equal to APR_SHA1_DIGESTSIZE then you would use the raw digest,
> otherwise use svn_checksum_deserialize().
>
> Sounds like a 1.8 repository upgrade though, since once you wrote a $sha1$
> style string, older Subversion's wouldn't parse it directly.
Oh. Good point!
> I won't be taking this on, I've still got a memory lifetime issue I'm
> debugging and then a svn commit thread pool to write to support 4
> commits/sec from remote clients that are consuming all threads from our RPC
> thread pool and killing readers.
hehe... that'll take you what... an hour? ;-)
Cheers,
-g
Re: svn commit: r1327703 - in /subversion/trunk/subversion: include/private/svn_subr_private.h
libsvn_fs_base/util/fs_skels.c libsvn_subr/checksum.c
Posted by Blair Zajac <bl...@orcaware.com>.
On 04/18/2012 03:45 PM, Greg Stein wrote:
> On Wed, Apr 18, 2012 at 18:42, Blair Zajac<bl...@orcaware.com> wrote:
>> On 04/18/2012 03:35 PM, Greg Stein wrote:
>>> On Wed, Apr 18, 2012 at 18:11,<bl...@apache.org> wrote:
>>>>
>>>> Author: blair
>>>> Date: Wed Apr 18 22:11:43 2012
>>>> New Revision: 1327703
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1327703&view=rev
>>>> Log:
>>>> Add and use svn_checksum__from_digest_sha1().
>>>>
>>>> This replaces svn_checksum__from_digest() taking a svn_checksum_sha1
>>>> argument with a new svn_checksum_t constructor function.
>>>>
>>>> * subversion/include/private/svn_subr_private.h,
>>>> * subversion/libsvn_subr/checksum.c
>>>> (svn_checksum__from_digest_sha1):
>>>> New private function.
>>>>
>>>> * subversion/libsvn_fs_base/util/fs_skels.c
>>>> (svn_fs_base__parse_representation_skel):
>>>> Replace
>>>> svn_checksum__from_digest(digest, svn_checksum_sha1, pool);
>>>> with
>>>> svn_checksum__from_digest_sha1(digest, pool);
>>>
>>>
>>> Shouldn't we be using svn_checksum_serialize() and _deserialize()
>>> rather than raw digests?
>>>
>>> (iow, we "shouldn't" need from_digest_sha1)
>>
>>
>> They don't contain the same bytes. I wasn't aware of
>> svn_checksum_serialize() till now, but those prepend a checksum type string
>> to the hex representation, as these are raw digest bytes.
>>
>> This work was to get rid of svn_checksum__from_digest(), which is now almost
>> complete, per the other thread.
>
> Yup, I understood. Sorry that I wasn't clear: I meant in our
> serialization code, shouldn't we use the "proper" functions rather
> than raw sha1 digests? Is there any way to switch to them at a
> higher/semantic level?
>
> I haven't looked at that stuff, but I'm going to guess repositories
> now exist with raw sha1 digests. Is there a format type in there? Can
> we start writing "csum" and svn_checksum_serialize() into the skel?
> And then read raw md5, raw sha1, or a serialized checksum?
I'm not familiar with this part of the code either (not having looked at
the svn_skel.h before) but that make sense. The code could look for a
raw digest or a $name$ and then use that.
It doesn't look like it would be too hard. Because the skel has a len,
if it's equal to APR_SHA1_DIGESTSIZE then you would use the raw digest,
otherwise use svn_checksum_deserialize().
Sounds like a 1.8 repository upgrade though, since once you wrote a
$sha1$ style string, older Subversion's wouldn't parse it directly.
I won't be taking this on, I've still got a memory lifetime issue I'm
debugging and then a svn commit thread pool to write to support 4
commits/sec from remote clients that are consuming all threads from our
RPC thread pool and killing readers.
Blair
Re: svn commit: r1327703 - in /subversion/trunk/subversion:
include/private/svn_subr_private.h libsvn_fs_base/util/fs_skels.c libsvn_subr/checksum.c
Posted by Greg Stein <gs...@gmail.com>.
On Wed, Apr 18, 2012 at 18:42, Blair Zajac <bl...@orcaware.com> wrote:
> On 04/18/2012 03:35 PM, Greg Stein wrote:
>> On Wed, Apr 18, 2012 at 18:11,<bl...@apache.org> wrote:
>>>
>>> Author: blair
>>> Date: Wed Apr 18 22:11:43 2012
>>> New Revision: 1327703
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1327703&view=rev
>>> Log:
>>> Add and use svn_checksum__from_digest_sha1().
>>>
>>> This replaces svn_checksum__from_digest() taking a svn_checksum_sha1
>>> argument with a new svn_checksum_t constructor function.
>>>
>>> * subversion/include/private/svn_subr_private.h,
>>> * subversion/libsvn_subr/checksum.c
>>> (svn_checksum__from_digest_sha1):
>>> New private function.
>>>
>>> * subversion/libsvn_fs_base/util/fs_skels.c
>>> (svn_fs_base__parse_representation_skel):
>>> Replace
>>> svn_checksum__from_digest(digest, svn_checksum_sha1, pool);
>>> with
>>> svn_checksum__from_digest_sha1(digest, pool);
>>
>>
>> Shouldn't we be using svn_checksum_serialize() and _deserialize()
>> rather than raw digests?
>>
>> (iow, we "shouldn't" need from_digest_sha1)
>
>
> They don't contain the same bytes. I wasn't aware of
> svn_checksum_serialize() till now, but those prepend a checksum type string
> to the hex representation, as these are raw digest bytes.
>
> This work was to get rid of svn_checksum__from_digest(), which is now almost
> complete, per the other thread.
Yup, I understood. Sorry that I wasn't clear: I meant in our
serialization code, shouldn't we use the "proper" functions rather
than raw sha1 digests? Is there any way to switch to them at a
higher/semantic level?
I haven't looked at that stuff, but I'm going to guess repositories
now exist with raw sha1 digests. Is there a format type in there? Can
we start writing "csum" and svn_checksum_serialize() into the skel?
And then read raw md5, raw sha1, or a serialized checksum?
Cheers,
-g
Re: svn commit: r1327703 - in /subversion/trunk/subversion: include/private/svn_subr_private.h
libsvn_fs_base/util/fs_skels.c libsvn_subr/checksum.c
Posted by Blair Zajac <bl...@orcaware.com>.
On 04/18/2012 03:35 PM, Greg Stein wrote:
> On Wed, Apr 18, 2012 at 18:11,<bl...@apache.org> wrote:
>> Author: blair
>> Date: Wed Apr 18 22:11:43 2012
>> New Revision: 1327703
>>
>> URL: http://svn.apache.org/viewvc?rev=1327703&view=rev
>> Log:
>> Add and use svn_checksum__from_digest_sha1().
>>
>> This replaces svn_checksum__from_digest() taking a svn_checksum_sha1
>> argument with a new svn_checksum_t constructor function.
>>
>> * subversion/include/private/svn_subr_private.h,
>> * subversion/libsvn_subr/checksum.c
>> (svn_checksum__from_digest_sha1):
>> New private function.
>>
>> * subversion/libsvn_fs_base/util/fs_skels.c
>> (svn_fs_base__parse_representation_skel):
>> Replace
>> svn_checksum__from_digest(digest, svn_checksum_sha1, pool);
>> with
>> svn_checksum__from_digest_sha1(digest, pool);
>
> Shouldn't we be using svn_checksum_serialize() and _deserialize()
> rather than raw digests?
>
> (iow, we "shouldn't" need from_digest_sha1)
They don't contain the same bytes. I wasn't aware of
svn_checksum_serialize() till now, but those prepend a checksum type
string to the hex representation, as these are raw digest bytes.
This work was to get rid of svn_checksum__from_digest(), which is now
almost complete, per the other thread.
Blair