Re: [Qemu-devel] [PATCH] qom: Make object_unref() free the object's memory when refcount goes to 0.

2012-02-26 Thread Alexander Barabash

On 02/24/2012 05:11 PM, Anthony Liguori wrote:

On 02/23/2012 10:21 AM, Alexander Barabash wrote:

On 02/22/2012 09:12 PM, Anthony Liguori wrote:

On 02/22/2012 12:00 PM, alexander_barab...@mentor.com wrote:

From: Alexander Barabashalexander_barab...@mentor.com

Why do you want to have a delete notifier list, rather than just a 
delete callback.


Because a notifier list allows for third parties to receive the event 
(think GObject signal/slots).
This is a valid point, but wouldn't it logical to issue an event before 
running the destructor?

Along the lines:

void object_finalize(void *data)
{
Object *obj = data;
TypeImpl *ti = obj-class-type;

object_deinit(obj, ti);
object_property_del_all(obj);

g_assert(obj-ref == 0);

object_finalized_notification(obj);
}

...

void object_unref(Object *obj)
{
g_assert(obj-ref  0);
if (obj-ref == 1) {
object_is_about_to_be_finalized_notification(obj);
}
 obj-ref--;

/* parent always holds a reference to its children */
if (obj-ref == 0) {
object_finalize(obj);
}
}

Here, there is a notification while the object is still alive (in the 
sense that it has not been finalized).

Then, if the object is actually finalized, there is notification about that.

By the way, using weak references would spare us the notification list.
Object's memory will not be freed as long as a weak reference to it exists.
Access through a weak reference to a dead object will remove that weak 
reference.
This way, we shall also avoid problems with circular references between 
objects.


Regards,
Alex




At the point where refcount == 0, the destructor has been called 
already,

so there is not much to be done, except for reclaim the memory.


Right, but the memory is not allocated by the core of Object.  This is 
important in order to allow in-place object creation.  You could 
special case this and have a flag to indicate whether the object has 
allocated it's own memory or not but I think the two approaches end up 
having equal complexity whereas the NotifierList gives you a lot more 
flexibility.


It makes it possible to use a small object allocator for Objects which 
could be useful one day if we use objects in a fast path (like using 
Objects to allocate packets in the network layer or requests in the 
block layer).


Regards,

Anthony Liguori






Re: [Qemu-devel] [PATCH] qom: Make object_unref() free the object's memory when refcount goes to 0.

2012-02-24 Thread Anthony Liguori

On 02/23/2012 10:21 AM, Alexander Barabash wrote:

On 02/22/2012 09:12 PM, Anthony Liguori wrote:

On 02/22/2012 12:00 PM, alexander_barab...@mentor.com wrote:

From: Alexander Barabashalexander_barab...@mentor.com

In the existing implementation, object_delete()
calls object_unref(), then frees the object's storage.
Running object_delete() on an object with reference count
different from 1 causes program failure.

In the existing implementation, object_unref()
finalizes the object when its reference count becomes 0.

In the new implementation, object_unref()
finalizes and frees the object's storage when the reference count becomes 0.

In the new implementation, object_delete()
just calls object_unref().
Running object_delete() on an object with reference count
different from 1 still causes program failure.


This isn't correct. QOM objects don't necessarily have heap allocated objects.

I've been thinking about this general problem and I think the right way to
solve it is to have a delete notifier list. That way, object_new() can
register a delete notifier that calls g_free() whenever refcount=0. That way
an explicit object_delete() isn't needed anymore.


Why do you want to have a delete notifier list, rather than just a delete 
callback.


Because a notifier list allows for third parties to receive the event (think 
GObject signal/slots).



At the point where refcount == 0, the destructor has been called already,
so there is not much to be done, except for reclaim the memory.


Right, but the memory is not allocated by the core of Object.  This is important 
in order to allow in-place object creation.  You could special case this and 
have a flag to indicate whether the object has allocated it's own memory or not 
but I think the two approaches end up having equal complexity whereas the 
NotifierList gives you a lot more flexibility.


It makes it possible to use a small object allocator for Objects which could be 
useful one day if we use objects in a fast path (like using Objects to allocate 
packets in the network layer or requests in the block layer).


Regards,

Anthony Liguori



Regards,
Alex



Although I think we should keep the call around as it's convenient for
replacing occurrences of qdev_free() where you really want the assert.

Regards,

Anthony Liguori



Signed-off-by: Alexander Barabashalexander_barab...@mentor.com
---
qom/object.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index e6591e1..8d36a9c 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -373,9 +373,8 @@ Object *object_new(const char *typename)

void object_delete(Object *obj)
{
+ g_assert(obj-ref == 1);
object_unref(obj);
- g_assert(obj-ref == 0);
- g_free(obj);
}

static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
@@ -585,6 +584,7 @@ void object_unref(Object *obj)
/* parent always holds a reference to its children */
if (obj-ref == 0) {
object_finalize(obj);
+ g_free(obj);
}
}










Re: [Qemu-devel] [PATCH] qom: Make object_unref() free the object's memory when refcount goes to 0.

2012-02-23 Thread Alexander Barabash

On 02/22/2012 09:12 PM, Anthony Liguori wrote:

On 02/22/2012 12:00 PM, alexander_barab...@mentor.com wrote:

From: Alexander Barabashalexander_barab...@mentor.com

In the existing implementation, object_delete()
calls object_unref(), then frees the object's storage.
Running object_delete() on an object with reference count
different from 1 causes program failure.

In the existing implementation, object_unref()
finalizes the object when its reference count becomes 0.

In the new implementation, object_unref()
finalizes and frees the object's storage when the reference count 
becomes 0.


In the new implementation, object_delete()
just calls object_unref().
Running object_delete() on an object with reference count
different from 1 still causes program failure.


This isn't correct.  QOM objects don't necessarily have heap allocated 
objects.


I've been thinking about this general problem and I think the right 
way to solve it is to have a delete notifier list.  That way, 
object_new() can register a delete notifier that calls g_free() 
whenever refcount=0.  That way an explicit object_delete() isn't 
needed anymore.


Why do you want to have a delete notifier list, rather than just a 
delete callback.

At the point where refcount == 0, the destructor has been called already,
so there is not much to be done, except for reclaim the memory.

Regards,
Alex



Although I think we should keep the call around as it's convenient for 
replacing occurrences of qdev_free() where you really want the assert.


Regards,

Anthony Liguori



Signed-off-by: Alexander Barabashalexander_barab...@mentor.com
---
  qom/object.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index e6591e1..8d36a9c 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -373,9 +373,8 @@ Object *object_new(const char *typename)

  void object_delete(Object *obj)
  {
+g_assert(obj-ref == 1);
  object_unref(obj);
-g_assert(obj-ref == 0);
-g_free(obj);
  }

  static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
@@ -585,6 +584,7 @@ void object_unref(Object *obj)
  /* parent always holds a reference to its children */
  if (obj-ref == 0) {
  object_finalize(obj);
+g_free(obj);
  }
  }








[Qemu-devel] [PATCH] qom: Make object_unref() free the object's memory when refcount goes to 0.

2012-02-22 Thread alexander_barabash
From: Alexander Barabash alexander_barab...@mentor.com

In the existing implementation, object_delete()
calls object_unref(), then frees the object's storage.
Running object_delete() on an object with reference count
different from 1 causes program failure.

In the existing implementation, object_unref()
finalizes the object when its reference count becomes 0.

In the new implementation, object_unref()
finalizes and frees the object's storage when the reference count becomes 0.

In the new implementation, object_delete()
just calls object_unref().
Running object_delete() on an object with reference count
different from 1 still causes program failure.

Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
---
 qom/object.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index e6591e1..8d36a9c 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -373,9 +373,8 @@ Object *object_new(const char *typename)
 
 void object_delete(Object *obj)
 {
+g_assert(obj-ref == 1);
 object_unref(obj);
-g_assert(obj-ref == 0);
-g_free(obj);
 }
 
 static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
@@ -585,6 +584,7 @@ void object_unref(Object *obj)
 /* parent always holds a reference to its children */
 if (obj-ref == 0) {
 object_finalize(obj);
+g_free(obj);
 }
 }
 
-- 
1.7.5.4




Re: [Qemu-devel] [PATCH] qom: Make object_unref() free the object's memory when refcount goes to 0.

2012-02-22 Thread Anthony Liguori

On 02/22/2012 12:00 PM, alexander_barab...@mentor.com wrote:

From: Alexander Barabashalexander_barab...@mentor.com

In the existing implementation, object_delete()
calls object_unref(), then frees the object's storage.
Running object_delete() on an object with reference count
different from 1 causes program failure.

In the existing implementation, object_unref()
finalizes the object when its reference count becomes 0.

In the new implementation, object_unref()
finalizes and frees the object's storage when the reference count becomes 0.

In the new implementation, object_delete()
just calls object_unref().
Running object_delete() on an object with reference count
different from 1 still causes program failure.


This isn't correct.  QOM objects don't necessarily have heap allocated objects.

I've been thinking about this general problem and I think the right way to solve 
it is to have a delete notifier list.  That way, object_new() can register a 
delete notifier that calls g_free() whenever refcount=0.  That way an explicit 
object_delete() isn't needed anymore.


Although I think we should keep the call around as it's convenient for replacing 
occurrences of qdev_free() where you really want the assert.


Regards,

Anthony Liguori



Signed-off-by: Alexander Barabashalexander_barab...@mentor.com
---
  qom/object.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index e6591e1..8d36a9c 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -373,9 +373,8 @@ Object *object_new(const char *typename)

  void object_delete(Object *obj)
  {
+g_assert(obj-ref == 1);
  object_unref(obj);
-g_assert(obj-ref == 0);
-g_free(obj);
  }

  static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
@@ -585,6 +584,7 @@ void object_unref(Object *obj)
  /* parent always holds a reference to its children */
  if (obj-ref == 0) {
  object_finalize(obj);
+g_free(obj);
  }
  }