From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:34566)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1XyemO-0001fK-1G
	for qemu-devel@nongnu.org; Wed, 10 Dec 2014 05:46:05 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1XyemJ-0004RQ-4A
	for qemu-devel@nongnu.org; Wed, 10 Dec 2014 05:45:59 -0500
Received: from mx1.redhat.com ([209.132.183.28]:43678)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1XyemI-0004RI-Nm
	for qemu-devel@nongnu.org; Wed, 10 Dec 2014 05:45:55 -0500
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <1416830152-524-5-git-send-email-arei.gonglei@huawei.com> (arei
	gonglei's message of "Mon, 24 Nov 2014 19:55:50 +0800")
References: <1416830152-524-1-git-send-email-arei.gonglei@huawei.com>
	<1416830152-524-5-git-send-email-arei.gonglei@huawei.com>
Date: Wed, 10 Dec 2014 11:05:17 +0100
Message-ID: <87r3w7akgy.fsf@elfo.elfo>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH RESEND for 2.3 4/6] xbzrle: check 8 bytes
	at a time after an concurrency scene
Reply-To: quintela@redhat.com
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: arei.gonglei@huawei.com
Cc: ChenLiang <chenliang88@huawei.com>, weidong.huang@huawei.com, qemu-devel@nongnu.org, dgilbert@redhat.com, amit.shah@redhat.com, pbonzini@redhat.com, peter.huangpeng@huawei.com

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> The logic of old code is correct. But Checking byte by byte will
> consume time after an concurrency scene.
>
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  xbzrle.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/xbzrle.c b/xbzrle.c
> index d27a140..0477367 100644
> --- a/xbzrle.c
> +++ b/xbzrle.c
> @@ -50,16 +50,24 @@ int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen,
>  
>          /* word at a time for speed */
>          if (!res) {
> -            while (i < slen &&
> -                   (*(long *)(old_buf + i)) == (*(long *)(new_buf + i))) {
> -                i += sizeof(long);
> -                zrun_len += sizeof(long);
> -            }
> -
> -            /* go over the rest */
> -            while (i < slen && old_buf[i] == new_buf[i]) {
> -                zrun_len++;
> -                i++;
> +            while (i < slen) {
> +                if ((*(long *)(old_buf + i)) == (*(long *)(new_buf + i))) {
> +                    i += sizeof(long);
> +                    zrun_len += sizeof(long);
> +                } else {
> +                    /* go over the rest */
> +                    for (j = 0; j < sizeof(long); j++) {
> +                        if (old_buf[i] == new_buf[i]) {
> +                            i++;
> +                            zrun_len++;
> +                        } else {
> +                            break;
> +                        }
> +                    }
> +                    if (j != sizeof(long)) {
> +                        break;
> +                    }
> +                }
>              }
>          }

This still does misaligned reads.  If we want to do aligned stuff,
something like that looks much better, no?  Notice that where I put
"break", I mean we have finished, but you get the idea.  Or I am missing something?


while(i % sizeof(long) != 0) {
	if (old_buf[i] == new_buf[i]) {
		i++;
		zrun_len++;
	} else {
		break;
	}
}

while (i < slen) {
	if ((*(long *)(old_buf + i)) == (*(long *)(new_buf + i))) {
		i += sizeof(long);
		zrun_len += sizeof(long);
	} else {
		break;
	}
}

for (j = 0; j < sizeof(long); j++) {
	if (old_buf[i] == new_buf[i]) {
		i++;
		zrun_len++;
	} else {
		break;
	}
}