Patchwork [06/11] qobject: add QBool type

login
register
mail settings
Submitter Luiz Capitulino
Date Oct. 18, 2009, 9:50 p.m.
Message ID <20091018195007.4dbaa599@doriath>
Download mbox | patch
Permalink /patch/36344/
State New
Headers show

Comments

Luiz Capitulino - Oct. 18, 2009, 9:50 p.m.
On Sat, 17 Oct 2009 08:36:06 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> We currently model as json bool as an int.  This works fine on the server side
> but it means we cannot send back proper bools to the client.  Introducing a
> proper QBool type fixes that.

 As we talked earlier today, I think it would be simpler to have QTrue,
QFalse and QNull as static objects.

 We could define that static objects get reference counting disabled, I don't
see issues in doing that and we get a very simple model.

 The big patch bellow does it, only compiled tested though.

 If you don't agree with this model, we'll have to allocate QNull objects.
Actually I would suggest a destroy() method which always resets the
refcount to 1, but then it will have the concurrency problems on threaded
applications you've mentioned.
Anthony Liguori - Oct. 19, 2009, 2:17 p.m.
Luiz Capitulino wrote:
> On Sat, 17 Oct 2009 08:36:06 -0500
> Anthony Liguori <aliguori@us.ibm.com> wrote:
>
>   
>> We currently model as json bool as an int.  This works fine on the server side
>> but it means we cannot send back proper bools to the client.  Introducing a
>> proper QBool type fixes that.
>>     
>
>  As we talked earlier today, I think it would be simpler to have QTrue,
> QFalse and QNull as static objects.
>
>  We could define that static objects get reference counting disabled, I don't
> see issues in doing that and we get a very simple model.
>
>  The big patch bellow does it, only compiled tested though.
>
>  If you don't agree with this model, we'll have to allocate QNull objects.
> Actually I would suggest a destroy() method which always resets the
> refcount to 1, but then it will have the concurrency problems on threaded
> applications you've mentioned.
>   

A lot of object systems have dealt with this in various ways.  The 
trouble with reference counting is usually thread safety.  CLR goes to 
great lengths to make their objects reference counting thread safe and 
it comes at a cost.  Python doesn't do anything explicit and it's one of 
the reasons that they're stuck with a big lock.

Since I'd like to expose QObject to client libraries, having a single 
static instance would be pretty unfortunate as it would imply a weird 
locking semantics.  Disabling reference counts for QNull/QBool seems 
unfortunate because it suggests that different semantics for certain 
objects.

I think this is a premature optimization.  I don't think we need to try 
and save the memory here.

Regards,

Anthony Liguori
Luiz Capitulino - Oct. 19, 2009, 2:21 p.m.
On Mon, 19 Oct 2009 09:17:38 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Luiz Capitulino wrote:
> > On Sat, 17 Oct 2009 08:36:06 -0500
> > Anthony Liguori <aliguori@us.ibm.com> wrote:
> >
> >   
> >> We currently model as json bool as an int.  This works fine on the server side
> >> but it means we cannot send back proper bools to the client.  Introducing a
> >> proper QBool type fixes that.
> >>     
> >
> >  As we talked earlier today, I think it would be simpler to have QTrue,
> > QFalse and QNull as static objects.
> >
> >  We could define that static objects get reference counting disabled, I don't
> > see issues in doing that and we get a very simple model.
> >
> >  The big patch bellow does it, only compiled tested though.
> >
> >  If you don't agree with this model, we'll have to allocate QNull objects.
> > Actually I would suggest a destroy() method which always resets the
> > refcount to 1, but then it will have the concurrency problems on threaded
> > applications you've mentioned.
> >   
> 
> A lot of object systems have dealt with this in various ways.  The 
> trouble with reference counting is usually thread safety.  CLR goes to 
> great lengths to make their objects reference counting thread safe and 
> it comes at a cost.  Python doesn't do anything explicit and it's one of 
> the reasons that they're stuck with a big lock.
> 
> Since I'd like to expose QObject to client libraries, having a single 
> static instance would be pretty unfortunate as it would imply a weird 
> locking semantics.  Disabling reference counts for QNull/QBool seems 
> unfortunate because it suggests that different semantics for certain 
> objects.
> 
> I think this is a premature optimization.  I don't think we need to try 
> and save the memory here.

 You can add QNull in your series then.

Patch

diff --git a/Makefile b/Makefile
index e78a3d0..1f985cf 100644
--- a/Makefile
+++ b/Makefile
@@ -125,7 +125,7 @@  obj-y += net.o net-queue.o
 obj-y += qemu-char.o aio.o net-checksum.o savevm.o
 obj-y += msmouse.o ps2.o
 obj-y += qdev.o qdev-properties.o
-obj-y += qint.o qstring.o qdict.o qlist.o qemu-config.o
+obj-y += qint.o qstring.o qdict.o qlist.o qmisc.o qemu-config.o
 
 obj-$(CONFIG_BRLAPI) += baum.o
 obj-$(CONFIG_WIN32) += tap-win32.o
diff --git a/qmisc.c b/qmisc.c
new file mode 100644
index 0000000..38a87dd
--- /dev/null
+++ b/qmisc.c
@@ -0,0 +1,31 @@ 
+#include "qmisc.h"
+
+/*
+ * QBool
+ */
+
+static const QType qbool_type = {
+    .code = QTYPE_QBOOL,
+};
+
+const QBool QTrue = {
+     QOBJECT_INIT_STATIC(&qbool_type),
+     .value = 1
+};
+
+const QBool QFalse = {
+     QOBJECT_INIT_STATIC(&qbool_type),
+     .value = 0
+};
+
+/*
+ * QNull
+ */
+
+static const QType qnull_type = {
+    .code = QTYPE_QNULL,
+};
+
+const qnull QNull = {
+     QOBJECT_INIT_STATIC(&qnull_type),
+};
diff --git a/qmisc.h b/qmisc.h
new file mode 100644
index 0000000..dd9d5a9
--- /dev/null
+++ b/qmisc.h
@@ -0,0 +1,38 @@ 
+#ifndef QMISC_H
+#define QMISC_H
+
+#include "qobject.h"
+
+/*
+ * QBool
+ */
+
+typedef struct QBool {
+    QObject_HEAD;
+    int value;
+} QBool;
+
+extern const QBool QTrue;
+extern const QBool QFalse;
+
+static inline int qbool_is_true(const QBool *qbool)
+{
+    return qbool->value;
+}
+
+static inline int qbool_is_false(const QBool *qbool)
+{
+    return !qbool->value;
+}
+
+/*
+ * QNull
+ */
+
+typedef struct qnull {
+    QObject_HEAD;
+} qnull;
+
+extern const qnull QNull;
+
+#endif /* QMISC_H */
diff --git a/qobject.h b/qobject.h
index 4cc9287..81e4d94 100644
--- a/qobject.h
+++ b/qobject.h
@@ -41,6 +41,8 @@  typedef enum {
     QTYPE_QSTRING,
     QTYPE_QDICT,
     QTYPE_QLIST,
+    QTYPE_QBOOL,
+    QTYPE_QNULL,
 } qtype_code;
 
 struct QObject;
@@ -75,12 +77,15 @@  typedef struct QObject {
     obj->base.refcnt = 1;               \
     obj->base.type   = qtype_type
 
+#define QOBJECT_INIT_STATIC(qtype_type) \
+    .base.type = qtype_type
+
 /**
  * qobject_incref(): Increment QObject's reference count
  */
 static inline void qobject_incref(QObject *obj)
 {
-    if (obj)
+    if (obj && obj->type->destroy)
         obj->refcnt++;
 }
 
@@ -90,9 +95,7 @@  static inline void qobject_incref(QObject *obj)
  */
 static inline void qobject_decref(QObject *obj)
 {
-    if (obj && --obj->refcnt == 0) {
-        assert(obj->type != NULL);
-        assert(obj->type->destroy != NULL);
+    if (obj && obj->type->destroy && --obj->refcnt == 0) {
         obj->type->destroy(obj);
     }
 }