Re: [Qemu-devel] [RfC PATCH 07/10] io: add qio_buffer tracing

[Peter Lieven]

Am 24.09.2015 um 10:42 schrieb Gerd Hoffmann:
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  io/buffer.c  | 10 ++++++++++
>  trace-events |  6 ++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/io/buffer.c b/io/buffer.c
> index 96077d3..05425c2 100644
> --- a/io/buffer.c
> +++ b/io/buffer.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include "io/buffer.h"
> +#include "trace.h"
>  
>  #define QIO_BUFFER_MIN_INIT_SIZE 4096
>  
> @@ -37,6 +38,7 @@ void qio_buffer_reserve(QIOBuffer *buffer, size_t len)
>          buffer->capacity = pow2ceil(buffer->offset + len);
>          buffer->capacity = MAX(buffer->capacity, QIO_BUFFER_MIN_INIT_SIZE);
>          buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
> +        trace_qio_buffer_resize(buffer->name ?: "unnamed", buffer->capacity);
>      }
>  }
>  
> @@ -57,6 +59,7 @@ void qio_buffer_reset(QIOBuffer *buffer)
>  
>  void qio_buffer_free(QIOBuffer *buffer)
>  {
> +    trace_qio_buffer_free(buffer->name ?: "unnamed");
>      g_free(buffer->buffer);
>      g_free(buffer->name);
>      buffer->offset = 0;
> @@ -80,6 +83,9 @@ void qio_buffer_advance(QIOBuffer *buffer, size_t len)
>  
>  void qio_buffer_move_empty(QIOBuffer *to, QIOBuffer *from)
>  {
> +    trace_qio_buffer_move_empty(to->name ?: "unnamed",
> +                                from->offset,
> +                                from->name ?: "unnamed");
>      assert(to->offset == 0);
>  
>      g_free(to->buffer);
> @@ -99,6 +105,10 @@ void qio_buffer_move(QIOBuffer *to, QIOBuffer *from)
>          return;
>      }
>  
> +    trace_qio_buffer_move(to->name ?: "unnamed",
> +                          from->offset,
> +                          from->name ?: "unnamed");
> +
>      qio_buffer_reserve(to, from->offset);
>      qio_buffer_append(to, from->buffer, from->offset);
>  
> diff --git a/trace-events b/trace-events
> index 88a2f14..0f06b64 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1376,6 +1376,12 @@ spapr_iommu_new_table(uint64_t liobn, void *tcet, void 
> *table, int fd) "liobn=%"
>  # hw/ppc/ppc.c
>  ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) 
> "adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"
>  
> +# io/buffer.c
> +qio_buffer_resize(const char *buf, size_t len) "%s: len %zd"
> +qio_buffer_move_empty(const char *buf, size_t len, const char *from) "%s: 
> %zd bytes from %s"
> +qio_buffer_move(const char *buf, size_t len, const char *from) "%s: %zd 
> bytes from %s"
> +qio_buffer_free(const char *buf) "%s"
> +
>  # util/hbitmap.c
>  hbitmap_iter_skip_words(const void *hb, void *hbi, uint64_t pos, unsigned 
> long cur) "hb %p hbi %p pos %"PRId64" cur 0x%lx"
>  hbitmap_reset(void *hb, uint64_t start, uint64_t count, uint64_t sbit, 
> uint64_t ebit) "hb %p items %"PRIu64",%"PRIu64" bits %"PRIu64"..%"PRIu64

It might be a good idea to add the allocation size (aka capacity) when the 
buffer is freed:

diff --git a/io/buffer.c b/io/buffer.c
index 05425c2..60a96b0 100644
--- a/io/buffer.c
+++ b/io/buffer.c
@@ -59,7 +59,7 @@ void qio_buffer_reset(QIOBuffer *buffer)
 
 void qio_buffer_free(QIOBuffer *buffer)
 {
-    trace_qio_buffer_free(buffer->name ?: "unnamed");
+    trace_qio_buffer_free(buffer->name ?: "unnamed", buffer->capacity);
     g_free(buffer->buffer);
     g_free(buffer->name);
     buffer->offset = 0;
diff --git a/trace-events b/trace-events
index 0f06b64..53e096b 100644
--- a/trace-events
+++ b/trace-events
@@ -1380,7 +1380,7 @@ ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t 
diff, int64_t seconds) "ad
 qio_buffer_resize(const char *buf, size_t len) "%s: len %zd"
 qio_buffer_move_empty(const char *buf, size_t len, const char *from) "%s: %zd 
bytes from %s"
 qio_buffer_move(const char *buf, size_t len, const char *from) "%s: %zd bytes 
from %s"
-qio_buffer_free(const char *buf) "%s"
+qio_buffer_free(const char *buf, size_t len) "%s: capacity %zd"
 
 # util/hbitmap.c
 hbitmap_iter_skip_words(const void *hb, void *hbi, uint64_t pos, unsigned long 
cur) "hb %p hbi %p pos %"PRId64" cur 0x%lx"


This reveals that the buffers still increase to a reasonable size so it might 
still be an option to have some sort of shrinking logic:

address@hidden:qio_buffer_free vnc-input/12: capacity 4096
address@hidden:qio_buffer_free vnc-output/12: capacity 8388608
address@hidden:qio_buffer_free vnc-ws_input/12: capacity 0
address@hidden:qio_buffer_free vnc-ws_output/12: capacity 0
address@hidden:qio_buffer_free vnc-zlib/12: capacity 0
address@hidden:qio_buffer_free vnc-tight/12: capacity 16777216
address@hidden:qio_buffer_free vnc-tight-zlib/12: capacity 32768
address@hidden:qio_buffer_free vnc-tight-gradient/12: capacity 0
address@hidden:qio_buffer_free vnc-tight-png/12: capacity 0
address@hidden:qio_buffer_free vnc-zrle/12: capacity 0
address@hidden:qio_buffer_free vnc-zrle-fb/12: capacity 0
address@hidden:qio_buffer_free vnc-zrle-zlib/12: capacity 0
address@hidden:qio_buffer_free vnc-jobs_buffer/12: capacity 0

Peter