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=1327703view=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
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=1327703view=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
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: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=1327703view=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
On 04/18/2012 03:45 PM, Greg Stein wrote: On Wed, Apr 18, 2012 at 18:42, Blair Zajacbl...@orcaware.com wrote: On 04/18/2012 03:35 PM, Greg Stein wrote: On Wed, Apr 18, 2012 at 18:11,bl...@apache.orgwrote: Author: blair Date: Wed Apr 18 22:11:43 2012 New Revision: 1327703 URL: http://svn.apache.org/viewvc?rev=1327703view=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
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
