Re: [Qemu-devel] [PATCH v5 08/10] xbzrle: check 8 bytes at a time after an concurrency scene

2014-04-04 Thread Dr. David Alan Gilbert
* arei.gong...@huawei.com (arei.gong...@huawei.com) wrote:
 From: ChenLiang chenlian...@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 chenlian...@huawei.com
 Signed-off-by: Gonglei arei.gong...@huawei.com
 ---
  xbzrle.c | 28 ++--
  1 file changed, 18 insertions(+), 10 deletions(-)
 
 diff --git a/xbzrle.c b/xbzrle.c
 index 92cccd7..9d67309 100644
 --- a/xbzrle.c
 +++ b/xbzrle.c
 @@ -51,16 +51,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;

Is it not possible to make this code the same as the other loop, you could xor 
in the same
way just change the comparison?
(What do other people think - I was thinking that would just be better since it 
would
be symmetric?)

Dave
 +}
 +}
  }
  }
  
 -- 
 1.7.12.4
 
 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH v5 08/10] xbzrle: check 8 bytes at a time after an concurrency scene

2014-04-04 Thread 陈梁

 * arei.gong...@huawei.com (arei.gong...@huawei.com) wrote:
 From: ChenLiang chenlian...@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 chenlian...@huawei.com
 Signed-off-by: Gonglei arei.gong...@huawei.com
 ---
 xbzrle.c | 28 ++--
 1 file changed, 18 insertions(+), 10 deletions(-)
 
 diff --git a/xbzrle.c b/xbzrle.c
 index 92cccd7..9d67309 100644
 --- a/xbzrle.c
 +++ b/xbzrle.c
 @@ -51,16 +51,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;
 
 Is it not possible to make this code the same as the other loop, you could 
 xor in the same
 way just change the comparison?
 (What do other people think - I was thinking that would just be better since 
 it would
 be symmetric?)
 
 Dave
Hi,
yeah, It will be symmetric. They are equivalence like this:
(*(long *)(old_buf + i)) == (*(long *)(new_buf + i))

!(xor = (*(long *)(old_buf + i)) ^ (*(long *)(new_buf + i)))

But the second is not efficient, IMHO. Because xbzrle assumes that most of 
bytes in the dirty
page is not modified. The result of xor will be not useful mostly. ps: It's 
just my idea.

Best regards
Chen Liang
 +}
 +}
 }
 }
 
 -- 
 1.7.12.4
 
 
 --
 Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK