[Qemu-devel] [PATCH v12 28/36] qapi: Simplify QObject

[Eric Blake <eblake@redhat.com>]

The QObject hierarchy is small enough, and unlikely to grow further
(since we only use it to map to JSON and already cover all JSON
types), that we can simplify things by not tracking a separate
vtable, but just inline all two elements of the vtable QType
directly into QObject.  We can drop qnull_destroy_obj() in the
process.

This also has the nice benefit of moving the typename 'QType'
out of the way, so that the next patch can repurpose it for a
nicer name for 'qtype_code'.

The various objects are now 8 bytes larger on 64-bit platforms, but
hopefully this is in the noise.  If size is absolutely critical due
to cache line speed effects, we could do some bit packing to
restore the previous size (qtype_code currently only requires three
distinct bits, so we could assert an assumption that architecture
ABIs will give function pointers at least an 8-byte alignment, and
that the three low-order bits of destroy are always zero; or we
could use a bitfield for refcnt, with a maximum of SIZE_MAX/8, and
given that a QObject already occupies at least 8 bytes, that won't
artificially limit us). Or we could expose the currently static
destroy methods and instead create a table in qobject.h that maps
QType to destructor.  But let's save any size compression for a
followup patch, if at all.

On the other hand, by avoiding a separate vtable, there is less
indirection, so use of QObjects may be slightly more efficient.

However, as we don't have evidence pointing at either QObject size
or access speed as being a hot spot, I didn't think it was worth
benchmarking.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>

---
v12: new patch, split off from 21/28
---
 include/qapi/qmp/qobject.h | 38 +++++++++++++++++++++-----------------
 qobject/qbool.c            |  7 +------
 qobject/qdict.c            |  7 +------
 qobject/qfloat.c           |  7 +------
 qobject/qint.c             |  7 +------
 qobject/qlist.c            |  7 +------
 qobject/qnull.c            | 13 ++-----------
 qobject/qstring.c          |  7 +------
 8 files changed, 29 insertions(+), 64 deletions(-)

diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index 4b96ed5..83ed70b 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -47,21 +47,21 @@ typedef enum {
     QTYPE_MAX,
 } qtype_code;

-struct QObject;
+typedef struct QObject QObject;

-typedef struct QType {
-    qtype_code code;
-    void (*destroy)(struct QObject *);
-} QType;
-
-typedef struct QObject {
-    const QType *type;
+struct QObject {
+    qtype_code type;
     size_t refcnt;
-} QObject;
+    void (*destroy)(QObject *);
+};

 /* Get the 'base' part of an object */
 #define QOBJECT(obj) (&(obj)->base)

+/* High-level interface for qobject_init() */
+#define QOBJECT_INIT(obj, type, destroy) \
+    qobject_init(QOBJECT(obj), type, destroy)
+
 /* High-level interface for qobject_incref() */
 #define QINCREF(obj)      \
     qobject_incref(QOBJECT(obj))
@@ -71,9 +71,14 @@ typedef struct QObject {
     qobject_decref(obj ? QOBJECT(obj) : NULL)

 /* Initialize an object to default values */
-#define QOBJECT_INIT(obj, qtype_type)   \
-    obj->base.refcnt = 1;               \
-    obj->base.type   = qtype_type
+static inline void qobject_init(QObject *obj, qtype_code type,
+                                void (*destroy)(struct QObject *))
+{
+    assert(type);
+    obj->refcnt = 1;
+    obj->type = type;
+    obj->destroy = destroy;
+}

 /**
  * qobject_incref(): Increment QObject's reference count
@@ -92,9 +97,8 @@ static inline void qobject_decref(QObject *obj)
 {
     assert(!obj || obj->refcnt);
     if (obj && --obj->refcnt == 0) {
-        assert(obj->type != NULL);
-        assert(obj->type->destroy != NULL);
-        obj->type->destroy(obj);
+        assert(obj->destroy);
+        obj->destroy(obj);
     }
 }

@@ -103,8 +107,8 @@ static inline void qobject_decref(QObject *obj)
  */
 static inline qtype_code qobject_type(const QObject *obj)
 {
-    assert(obj->type != NULL);
-    return obj->type->code;
+    assert(obj->type);
+    return obj->type;
 }

 extern QObject qnull_;
diff --git a/qobject/qbool.c b/qobject/qbool.c
index bc6535f..9f46e67 100644
--- a/qobject/qbool.c
+++ b/qobject/qbool.c
@@ -17,11 +17,6 @@

 static void qbool_destroy_obj(QObject *obj);

-static const QType qbool_type = {
-    .code = QTYPE_QBOOL,
-    .destroy = qbool_destroy_obj,
-};
-
 /**
  * qbool_from_bool(): Create a new QBool from a bool
  *
@@ -33,7 +28,7 @@ QBool *qbool_from_bool(bool value)

     qb = g_malloc(sizeof(*qb));
     qb->value = value;
-    QOBJECT_INIT(qb, &qbool_type);
+    QOBJECT_INIT(qb, QTYPE_QBOOL, qbool_destroy_obj);

     return qb;
 }
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 2d67bf1..cf62269 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -21,11 +21,6 @@

 static void qdict_destroy_obj(QObject *obj);

-static const QType qdict_type = {
-    .code = QTYPE_QDICT,
-    .destroy = qdict_destroy_obj,
-};
-
 /**
  * qdict_new(): Create a new QDict
  *
@@ -36,7 +31,7 @@ QDict *qdict_new(void)
     QDict *qdict;

     qdict = g_malloc0(sizeof(*qdict));
-    QOBJECT_INIT(qdict, &qdict_type);
+    QOBJECT_INIT(qdict, QTYPE_QDICT, qdict_destroy_obj);

     return qdict;
 }
diff --git a/qobject/qfloat.c b/qobject/qfloat.c
index c865163..2472189 100644
--- a/qobject/qfloat.c
+++ b/qobject/qfloat.c
@@ -17,11 +17,6 @@

 static void qfloat_destroy_obj(QObject *obj);

-static const QType qfloat_type = {
-    .code = QTYPE_QFLOAT,
-    .destroy = qfloat_destroy_obj,
-};
-
 /**
  * qfloat_from_int(): Create a new QFloat from a float
  *
@@ -33,7 +28,7 @@ QFloat *qfloat_from_double(double value)

     qf = g_malloc(sizeof(*qf));
     qf->value = value;
-    QOBJECT_INIT(qf, &qfloat_type);
+    QOBJECT_INIT(qf, QTYPE_QFLOAT, qfloat_destroy_obj);

     return qf;
 }
diff --git a/qobject/qint.c b/qobject/qint.c
index 999688e..765307f 100644
--- a/qobject/qint.c
+++ b/qobject/qint.c
@@ -16,11 +16,6 @@

 static void qint_destroy_obj(QObject *obj);

-static const QType qint_type = {
-    .code = QTYPE_QINT,
-    .destroy = qint_destroy_obj,
-};
-
 /**
  * qint_from_int(): Create a new QInt from an int64_t
  *
@@ -32,7 +27,7 @@ QInt *qint_from_int(int64_t value)

     qi = g_malloc(sizeof(*qi));
     qi->value = value;
-    QOBJECT_INIT(qi, &qint_type);
+    QOBJECT_INIT(qi, QTYPE_QINT, qint_destroy_obj);

     return qi;
 }
diff --git a/qobject/qlist.c b/qobject/qlist.c
index 298003a..1e2098a 100644
--- a/qobject/qlist.c
+++ b/qobject/qlist.c
@@ -17,11 +17,6 @@

 static void qlist_destroy_obj(QObject *obj);

-static const QType qlist_type = {
-    .code = QTYPE_QLIST,
-    .destroy = qlist_destroy_obj,
-};
- 
 /**
  * qlist_new(): Create a new QList
  *
@@ -33,7 +28,7 @@ QList *qlist_new(void)

     qlist = g_malloc(sizeof(*qlist));
     QTAILQ_INIT(&qlist->head);
-    QOBJECT_INIT(qlist, &qlist_type);
+    QOBJECT_INIT(qlist, QTYPE_QLIST, qlist_destroy_obj);

     return qlist;
 }
diff --git a/qobject/qnull.c b/qobject/qnull.c
index 9873e26..22d59e9 100644
--- a/qobject/qnull.c
+++ b/qobject/qnull.c
@@ -13,17 +13,8 @@
 #include "qemu-common.h"
 #include "qapi/qmp/qobject.h"

-static void qnull_destroy_obj(QObject *obj)
-{
-    assert(0);
-}
-
-static const QType qnull_type = {
-    .code = QTYPE_QNULL,
-    .destroy = qnull_destroy_obj,
-};
-
 QObject qnull_ = {
-    .type = &qnull_type,
+    .type = QTYPE_QNULL,
     .refcnt = 1,
+    .destroy = NULL,
 };
diff --git a/qobject/qstring.c b/qobject/qstring.c
index cb72dfb..ba59d00 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -16,11 +16,6 @@

 static void qstring_destroy_obj(QObject *obj);

-static const QType qstring_type = {
-    .code = QTYPE_QSTRING,
-    .destroy = qstring_destroy_obj,
-};
-
 /**
  * qstring_new(): Create a new empty QString
  *
@@ -57,7 +52,7 @@ QString *qstring_from_substr(const char *str, int start, int end)
     memcpy(qstring->string, str + start, qstring->length);
     qstring->string[qstring->length] = 0;

-    QOBJECT_INIT(qstring, &qstring_type);
+    QOBJECT_INIT(qstring, QTYPE_QSTRING, qstring_destroy_obj);

     return qstring;
 }
-- 
2.4.3