svn commit: r256132 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

[Andriy Gapon]

on 08/10/2013 04:38 Xin LI said the following:
> Author: delphij
> Date: Tue Oct  8 01:38:24 2013
> New Revision: 256132
> URL: http://svnweb.freebsd.org/changeset/base/256132
> 
> Log:
>   Improve lzjb decompress performance by reorganizing the code
>   to tighten the copy loop.
>   
>   Submitted by:	Denis Ahrens <denis h3q com>
>   MFC after:	2 weeks
>   Approved by:	re (gjb)
> 
> Modified:
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/lzjb.c
> 
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/lzjb.c
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/lzjb.c	Mon Oct  7 22:30:03 2013	(r256131)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/lzjb.c	Tue Oct  8 01:38:24 2013	(r256132)
> @@ -117,7 +117,9 @@ lzjb_decompress(void *s_start, void *d_s
>  			src += 2;
>  			if ((cpy = dst - offset) < (uchar_t *)d_start)
>  				return (-1);
> -			while (--mlen >= 0 && dst < d_end)
> +			if (mlen > (d_end - dst))
> +				mlen = d_end - dst;
> +			while (--mlen >= 0)
>  				*dst++ = *cpy++;
>  		} else {
>  			*dst++ = *src++;
> 

Intuitively it might seem that this change is indeed an improvement.
But given how "not dumb" (not sure if that always amounts to smart) the modern
compilers are, has anyone actually measured if this change indeed improves the
performance?

Judging from the conversations on the ZFS mailing lists this change event hurt
performance in some environments.
Looks like the upstream is not going to take this change.

So does it make any sense for us to make the code more obscure and different
from upstream?  Unless performance benefits could indeed be demonstrated.

-- 
Andriy Gapon