Re: [PATCH 1/2] net/colo-compare.c: Optimize compare order for performance

[Jason Wang]

On Mon, Dec 20, 2021 at 9:16 AM Zhang Chen <chen.zhang@intel.com> wrote:
>
> COLO-compare use the glib function g_queue_find_custom to dump
> another VM's networking packet to compare. But this function always
> start find from the queue->head(here is the newest packet), It will
> reduce the success rate of comparison. So this patch reversed
> the order of the queues for performance.
>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> Reported-by: leirao <lei.rao@intel.com>
> ---
>  net/colo-compare.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)

Applied.

Thanks

>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index b966e7e514..216de5a12b 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -197,7 +197,7 @@ static void 
> colo_compare_inconsistency_notify(CompareState *s)
>  /* Use restricted to colo_insert_packet() */
>  static gint seq_sorter(Packet *a, Packet *b, gpointer data)
>  {
> -    return a->tcp_seq - b->tcp_seq;
> +    return b->tcp_seq - a->tcp_seq;
>  }
>
>  static void fill_pkt_tcp_info(void *data, uint32_t *max_ack)
> @@ -421,13 +421,13 @@ pri:
>      if (g_queue_is_empty(&conn->primary_list)) {
>          return;
>      }
> -    ppkt = g_queue_pop_head(&conn->primary_list);
> +    ppkt = g_queue_pop_tail(&conn->primary_list);
>  sec:
>      if (g_queue_is_empty(&conn->secondary_list)) {
> -        g_queue_push_head(&conn->primary_list, ppkt);
> +        g_queue_push_tail(&conn->primary_list, ppkt);
>          return;
>      }
> -    spkt = g_queue_pop_head(&conn->secondary_list);
> +    spkt = g_queue_pop_tail(&conn->secondary_list);
>
>      if (ppkt->tcp_seq == ppkt->seq_end) {
>          colo_release_primary_pkt(s, ppkt);
> @@ -458,7 +458,7 @@ sec:
>              }
>          }
>          if (!ppkt) {
> -            g_queue_push_head(&conn->secondary_list, spkt);
> +            g_queue_push_tail(&conn->secondary_list, spkt);
>              goto pri;
>          }
>      }
> @@ -477,7 +477,7 @@ sec:
>          if (mark == COLO_COMPARE_FREE_PRIMARY) {
>              conn->compare_seq = ppkt->seq_end;
>              colo_release_primary_pkt(s, ppkt);
> -            g_queue_push_head(&conn->secondary_list, spkt);
> +            g_queue_push_tail(&conn->secondary_list, spkt);
>              goto pri;
>          } else if (mark == COLO_COMPARE_FREE_SECONDARY) {
>              conn->compare_seq = spkt->seq_end;
> @@ -490,8 +490,8 @@ sec:
>              goto pri;
>          }
>      } else {
> -        g_queue_push_head(&conn->primary_list, ppkt);
> -        g_queue_push_head(&conn->secondary_list, spkt);
> +        g_queue_push_tail(&conn->primary_list, ppkt);
> +        g_queue_push_tail(&conn->secondary_list, spkt);
>
>  #ifdef DEBUG_COLO_PACKETS
>          qemu_hexdump(stderr, "colo-compare ppkt", ppkt->data, ppkt->size);
> @@ -673,7 +673,7 @@ static void colo_compare_packet(CompareState *s, 
> Connection *conn,
>
>      while (!g_queue_is_empty(&conn->primary_list) &&
>             !g_queue_is_empty(&conn->secondary_list)) {
> -        pkt = g_queue_pop_head(&conn->primary_list);
> +        pkt = g_queue_pop_tail(&conn->primary_list);
>          result = g_queue_find_custom(&conn->secondary_list,
>                   pkt, (GCompareFunc)HandlePacket);
>
> @@ -689,7 +689,7 @@ static void colo_compare_packet(CompareState *s, 
> Connection *conn,
>               * timeout, it will trigger a checkpoint request.
>               */
>              trace_colo_compare_main("packet different");
> -            g_queue_push_head(&conn->primary_list, pkt);
> +            g_queue_push_tail(&conn->primary_list, pkt);
>
>              colo_compare_inconsistency_notify(s);
>              break;
> @@ -819,7 +819,7 @@ static int compare_chr_send(CompareState *s,
>          entry->buf = g_malloc(size);
>          memcpy(entry->buf, buf, size);
>      }
> -    g_queue_push_head(&sendco->send_list, entry);
> +    g_queue_push_tail(&sendco->send_list, entry);
>
>      if (sendco->done) {
>          sendco->co = qemu_coroutine_create(_compare_chr_send, sendco);
> @@ -1347,7 +1347,7 @@ static void colo_flush_packets(void *opaque, void 
> *user_data)
>      Packet *pkt = NULL;
>
>      while (!g_queue_is_empty(&conn->primary_list)) {
> -        pkt = g_queue_pop_head(&conn->primary_list);
> +        pkt = g_queue_pop_tail(&conn->primary_list);
>          compare_chr_send(s,
>                           pkt->data,
>                           pkt->size,
> @@ -1357,7 +1357,7 @@ static void colo_flush_packets(void *opaque, void 
> *user_data)
>          packet_destroy_partial(pkt, NULL);
>      }
>      while (!g_queue_is_empty(&conn->secondary_list)) {
> -        pkt = g_queue_pop_head(&conn->secondary_list);
> +        pkt = g_queue_pop_tail(&conn->secondary_list);
>          packet_destroy(pkt, NULL);
>      }
>  }
> --
> 2.25.1
>