* Wei Yang (richardw.y...@linux.intel.com) wrote:
> The encoding process could be described below:
>
> for [content]
> get length of a run
> encode this run
>
> By refactoring the code with this logic, it maybe a little easier to
> understand.
>
> Signed-off-by: Wei Yang <richardw.y...@linux.intel.com>
> ---
> migration/xbzrle.c | 153 +++++++++++++++++++++------------------------
> 1 file changed, 73 insertions(+), 80 deletions(-)
>
> diff --git a/migration/xbzrle.c b/migration/xbzrle.c
> index 1ba482ded9..25c69708ec 100644
> --- a/migration/xbzrle.c
> +++ b/migration/xbzrle.c
> @@ -14,6 +14,59 @@
> #include "qemu/cutils.h"
> #include "xbzrle.h"
>
> +static int next_run(uint8_t *old_buf, uint8_t *new_buf, int off, int slen,
> + bool zrun)
> +{
> + uint32_t len = 0;
> + long res;
> +
> + res = (slen - off) % sizeof(long);
> +
> + /* first unaligned bytes */
> + while (res) {
> + uint8_t xor = old_buf[off + len] ^ new_buf[off + len];
> +
> + if (!(zrun ^ !!xor)) {
> + break;
> + }
> + len++;
> + res--;
> + }
> +
> + if (res) {
> + return len;
> + }
> +
> + /* word at a time for speed, use of 32-bit long okay */
> + while (off + len < slen) {
> + /* truncation to 32-bit long okay */
> + unsigned long mask = (unsigned long)0x0101010101010101ULL;
> + long xor = (*(long *)(old_buf + off + len)) ^
> + (*(long *)(new_buf + off + len));
> +
> + if (zrun && !(zrun ^ !!xor)) {
Are lines like this really making it easier to read?
Juan: Opinion?
Dave
> + break;
> + } else if (!zrun && ((xor - mask) & ~xor & (mask << 7))) {
> + break;
> + }
> +
> + len += sizeof(long);
> + }
> +
> + /* go over the rest */
> + while (off + len < slen) {
> + uint8_t xor = old_buf[off + len] ^ new_buf[off + len];
> +
> + if (!(zrun ^ !!xor)) {
> + break;
> + }
> +
> + len++;
> + }
> +
> + return len;
> +}
> +
> /*
> page = zrun nzrun
> | zrun nzrun page
> @@ -27,103 +80,43 @@
> int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen,
> uint8_t *dst, int dlen)
> {
> - uint32_t zrun_len = 0, nzrun_len = 0;
> - int d = 0, i = 0;
> - long res;
> - uint8_t *nzrun_start = NULL;
> + bool zrun = true;
> + int len, src_off = 0, dst_off = 0;
>
> g_assert(!(((uintptr_t)old_buf | (uintptr_t)new_buf | slen) %
> sizeof(long)));
>
> - while (i < slen) {
> + for (; src_off < slen; src_off += len, zrun = !zrun) {
> /* overflow */
> - if (d + 2 > dlen) {
> + if (dst_off + 2 > dlen) {
> return -1;
> }
>
> - /* not aligned to sizeof(long) */
> - res = (slen - i) % sizeof(long);
> - while (res && old_buf[i] == new_buf[i]) {
> - zrun_len++;
> - i++;
> - res--;
> - }
> + len = next_run(old_buf, new_buf, src_off, slen, zrun);
>
> - /* 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);
> + if (zrun) {
> + /* buffer unchanged */
> + if (len == slen) {
> + return 0;
> }
> -
> - /* go over the rest */
> - while (i < slen && old_buf[i] == new_buf[i]) {
> - zrun_len++;
> - i++;
> + /* skip last zero run */
> + if (src_off + len == slen) {
> + return dst_off;
> }
> }
>
> - /* buffer unchanged */
> - if (zrun_len == slen) {
> - return 0;
> - }
> -
> - /* skip last zero run */
> - if (i == slen) {
> - return d;
> - }
> -
> - d += uleb128_encode_small(dst + d, zrun_len);
> -
> - zrun_len = 0;
> - nzrun_start = new_buf + i;
> -
> - /* overflow */
> - if (d + 2 > dlen) {
> - return -1;
> - }
> - /* not aligned to sizeof(long) */
> - res = (slen - i) % sizeof(long);
> - while (res && old_buf[i] != new_buf[i]) {
> - i++;
> - nzrun_len++;
> - res--;
> - }
> -
> - /* word at a time for speed, use of 32-bit long okay */
> - if (!res) {
> - /* truncation to 32-bit long okay */
> - unsigned long mask = (unsigned long)0x0101010101010101ULL;
> - while (i < slen) {
> - unsigned long xor;
> - xor = *(unsigned long *)(old_buf + i)
> - ^ *(unsigned long *)(new_buf + i);
> - if ((xor - mask) & ~xor & (mask << 7)) {
> - /* found the end of an nzrun within the current long */
> - while (old_buf[i] != new_buf[i]) {
> - nzrun_len++;
> - i++;
> - }
> - break;
> - } else {
> - i += sizeof(long);
> - nzrun_len += sizeof(long);
> - }
> + dst_off += uleb128_encode_small(dst + dst_off, len);
> + if (!zrun) {
> + /* overflow */
> + if (dst_off + len > dlen) {
> + return -1;
> }
> + memcpy(dst + dst_off, new_buf + src_off, len);
> + dst_off += len;
> }
> -
> - d += uleb128_encode_small(dst + d, nzrun_len);
> - /* overflow */
> - if (d + nzrun_len > dlen) {
> - return -1;
> - }
> - memcpy(dst + d, nzrun_start, nzrun_len);
> - d += nzrun_len;
> - nzrun_len = 0;
> }
>
> - return d;
> + return dst_off;
> }
>
> int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen)
> --
> 2.19.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
