qapi: change QmpOutputVisitor to QSLIST

Commit Message

Paolo Bonzini July 6, 2016, 12:43 p.m. UTC

This saves a little memory compared to the doubly-linked QTAILQ.

Signed-off-by: Paolo Bonzini <[e-mail]>
---
 qapi/qmp-output-visitor.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

Comments

Eric Blake July 6, 2016, 3:32 p.m. UTC | #1

On 07/06/2016 06:43 AM, Paolo Bonzini wrote:
> This saves a little memory compared to the doubly-linked QTAILQ.
> 
> Signed-off-by: Paolo Bonzini <[e-mail]>
> ---
>  qapi/qmp-output-visitor.c | 25 +++++++++----------------
>  1 file changed, 9 insertions(+), 16 deletions(-)
> 

>  struct QmpOutputVisitor
>  {
>      Visitor visitor;
> -    QStack stack; /* Stack of containers that haven't yet been finished */
> +    QSLIST_HEAD(, QStackEntry) stack; /* Stack of containers that haven't yet been finished */

Long line now, a simple reword would avoid a need to wrap:

    QSLIST_HEAD(...); /* Stack of unfinished containers */

Maintainer could fix it, so:
Reviewed-by: Eric Blake <[e-mail]>

Markus Armbruster July 7, 2016, 8:02 a.m. UTC | #2

Paolo Bonzini <[e-mail]> writes:

> This saves a little memory compared to the doubly-linked QTAILQ.
>
> Signed-off-by: Paolo Bonzini <[e-mail]>

The memory savings are trivial: we go from 160 + 24*d to 160 + 16*d,
where d is the depth of the tree we visit, almost always in the single
digits.  But I like the simplicity of a singly linked list.

Reviewed-by: Markus Armbruster <[e-mail]>

Patch

diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 4d3cf78..676df1f 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -22,15 +22,13 @@ 
 typedef struct QStackEntry
 {
     QObject *value;
-    QTAILQ_ENTRY(QStackEntry) node;
+    QSLIST_ENTRY(QStackEntry) node;
 } QStackEntry;
 
-typedef QTAILQ_HEAD(QStack, QStackEntry) QStack;
-
 struct QmpOutputVisitor
 {
     Visitor visitor;
-    QStack stack; /* Stack of containers that haven't yet been finished */
+    QSLIST_HEAD(, QStackEntry) stack; /* Stack of containers that haven't yet been finished */
     QObject *root; /* Root of the output visit */
 };
 
@@ -51,17 +49,17 @@  static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value)
     assert(qov->root);
     assert(value);
     e->value = value;
-    QTAILQ_INSERT_HEAD(&qov->stack, e, node);
+    QSLIST_INSERT_HEAD(&qov->stack, e, node);
 }
 
 /* Pop a value off the stack of QObjects being built, and return it. */
 static QObject *qmp_output_pop(QmpOutputVisitor *qov)
 {
-    QStackEntry *e = QTAILQ_FIRST(&qov->stack);
+    QStackEntry *e = QSLIST_FIRST(&qov->stack);
     QObject *value;
 
     assert(e);
-    QTAILQ_REMOVE(&qov->stack, e, node);
+    QSLIST_REMOVE_HEAD(&qov->stack, node);
     value = e->value;
     assert(value);
     g_free(e);
@@ -74,7 +72,7 @@  static QObject *qmp_output_pop(QmpOutputVisitor *qov)
 static void qmp_output_add_obj(QmpOutputVisitor *qov, const char *name,
                                QObject *value)
 {
-    QStackEntry *e = QTAILQ_FIRST(&qov->stack);
+    QStackEntry *e = QSLIST_FIRST(&qov->stack);
     QObject *cur = e ? e->value : NULL;
 
     if (!cur) {
@@ -198,7 +196,7 @@  static void qmp_output_type_null(Visitor *v, const char *name, Error **errp)
 QObject *qmp_output_get_qobject(QmpOutputVisitor *qov)
 {
     /* A visit must have occurred, with each start paired with end.  */
-    assert(qov->root && QTAILQ_EMPTY(&qov->stack));
+    assert(qov->root && QSLIST_EMPTY(&qov->stack));
 
     qobject_incref(qov->root);
     return qov->root;
@@ -211,11 +209,8 @@  Visitor *qmp_output_get_visitor(QmpOutputVisitor *v)
 
 void qmp_output_visitor_cleanup(QmpOutputVisitor *v)
 {
-    QStackEntry *e, *tmp;
-
-    QTAILQ_FOREACH_SAFE(e, &v->stack, node, tmp) {
-        QTAILQ_REMOVE(&v->stack, e, node);
-        g_free(e);
+    while (!QSLIST_EMPTY(&v->stack)) {
+        qmp_output_pop(v);
     }
 
     qobject_decref(v->root);
@@ -242,7 +237,5 @@  QmpOutputVisitor *qmp_output_visitor_new(void)
     v->visitor.type_any = qmp_output_type_any;
     v->visitor.type_null = qmp_output_type_null;
 
-    QTAILQ_INIT(&v->stack);
-
     return v;
 }