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 2013/06/21 00:36:37 UTC

Re: svn commit: r1495209 - /subversion/trunk/subversion/libsvn_fs_fs/tree.c

On Fri, Jun 21, 2013 at 2:11 AM,  <st...@apache.org> wrote:
> Author: stefan2
> Date: Thu Jun 20 22:11:31 2013
> New Revision: 1495209
>
> URL: http://svn.apache.org/r1495209
> Log:
> Make the hash function used by the FSFS DAG node cache
> platform-independent. That should help us reproducing issues
> detected on "exotic" platforms.
>
> * subversion/libsvn_fs_fs/tree.c
>   (cache_lookup): normalize chunked calculation to big endian
>
> Patch by: stsp
>
> Modified:
>     subversion/trunk/subversion/libsvn_fs_fs/tree.c
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1495209&r1=1495208&r2=1495209&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Thu Jun 20 22:11:31 2013
> @@ -355,14 +355,15 @@ cache_lookup( fs_fs_dag_cache_t *cache
>       (HASH_VALUE has been initialized to REVISION). */
>    for (i = 0; i + 4 <= path_len; i += 4)
>  #if SVN_UNALIGNED_ACCESS_IS_OK
> -    hash_value = hash_value * 0xd1f3da69 + *(const apr_uint32_t*)(path + i);
> +    hash_value = hash_value * 0xd1f3da69
> +               + ntohl(*(const apr_uint32_t*)(path + i));
ntohl() is real function, not inline function or macro on Windows [1].
So most likely your change  make hash calculation significantly slower
on Windows due function call for each four bytes.

[1] http://msdn.microsoft.com/en-us/library/windows/desktop/ms740069%28v=vs.85%29.aspx


-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com

Re: svn commit: r1495209 - /subversion/trunk/subversion/libsvn_fs_fs/tree.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Fri, Jun 21, 2013 at 12:36 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:

> On Fri, Jun 21, 2013 at 2:11 AM,  <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Thu Jun 20 22:11:31 2013
> > New Revision: 1495209
> >
> > URL: http://svn.apache.org/r1495209
> > Log:
> > Make the hash function used by the FSFS DAG node cache
> > platform-independent. That should help us reproducing issues
> > detected on "exotic" platforms.
> >
> > * subversion/libsvn_fs_fs/tree.c
> >   (cache_lookup): normalize chunked calculation to big endian
> >
> > Patch by: stsp
> >
> > Modified:
> >     subversion/trunk/subversion/libsvn_fs_fs/tree.c
> >
> > Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1495209&r1=1495208&r2=1495209&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
> > +++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Thu Jun 20 22:11:31
> 2013
> > @@ -355,14 +355,15 @@ cache_lookup( fs_fs_dag_cache_t *cache
> >       (HASH_VALUE has been initialized to REVISION). */
> >    for (i = 0; i + 4 <= path_len; i += 4)
> >  #if SVN_UNALIGNED_ACCESS_IS_OK
> > -    hash_value = hash_value * 0xd1f3da69 + *(const apr_uint32_t*)(path
> + i);
> > +    hash_value = hash_value * 0xd1f3da69
> > +               + ntohl(*(const apr_uint32_t*)(path + i));
> ntohl() is real function, not inline function or macro on Windows [1].
> So most likely your change  make hash calculation significantly slower
> on Windows due function call for each four bytes.
>
> [1]
> http://msdn.microsoft.com/en-us/library/windows/desktop/ms740069%28v=vs.85%29.aspx
>
>
The offending revisions have been removed from STATUS.
After some discussion on IRC, we realized that going for
a platform-independent hash function is simply not worth
the hassle.

So, the /trunk code already looks different. Not sure how
much of this should be backported (tons of back-and-forth
changes).

-- Stefan^2.