The type check is of course a good thing to have to catch programmer errors. How about having the type check be something like:

 gtype == GST_TYPE_BUFFER || g_type_check_instance_is_a (miniobjecttype)

Doing a quick check for the most common type and fall back to the slower check. The slow check would only happen for buffer subclasses.

you would indeed need a non-checking miniobject_unref and do the check in the gst_buffer_unref() method before you call the non-checking one, or something.

> The type check is of course a good thing to have to catch programmer errors.

Yes, this is all assuming that we think this type check is important enough that we want to keep it by default even if it costs us some performance (I do think so, fwiw).

The idea of having dedicated gst_{buffer,event,query}_{ref,unref} is that we can then use  GST_IS_BUFFER() etc. which takes the cheaper code path by default. Then we can either copy'n'paste the body of gst_mini_object_{ref,unref} or chain up to an _unchecked() variant.

> How about having the type check be something like:
> 
>  gtype == GST_TYPE_BUFFER || g_type_check_instance_is_a (miniobjecttype)
> 
> Doing a quick check for the most common type and fall back to the slower check.
> The slow check would only happen for buffer subclasses.

This would be where then? in gst_mini_object_*() or gst_buffer_*()?
 
> you would indeed need a non-checking miniobject_unref and do the check in the
> gst_buffer_unref() method before you call the non-checking one, or something.

Alternatively of course we could just add the type == GST_TYPE_BUFFER || type == GST_TYPE_EVENT || GST_IS_MINI_OBJECT (mo) to gst_mini_object_*(), which wouldn't be pretty, but surely we'd find ways to numb our sense of disgust.

in gst_buffer_unref() (which you presumably only can call with something that is a buffer) you would do:

gst_buffer_unref(buffer)
{
  return_if_fail (buffer->type == GST_TYPE_BUFFER || g_type_check_instance_is_a (miniobjecttype, buffer))

  gst_mini_object_unref_unchecked (buffer);
}

Same for events and queries (do a quick check for the exact type but fallback to the heavy type check in case someone subclassed).

In the miniobject2 branch I've been experimenting with that (it helps that miniobject can't be subclassed so that there is only one and only one simple int check (and the buffer type is in a globally exported variable to avoid the method call))

Wim's proposal sounds like a good plan, this doesn't speed up gst_mini_object_unref() but it is not that ugly and only requires few changes.

If you care about performance you turn those checks off (--enable-gobject-cast-checks=no) anyway, right? Not sure if its worth to speed it up that way.

> If you care about performance you turn those checks off
> (--enable-gobject-cast-checks=no) anyway, right? Not sure if its worth to speed
> it up that way.

We do care about performance on the desktop as well, but also want to keep those checks in place, which are extremely helpful for debugging purposes.

I think these changes are very much worth it: they're dead simple and should result in performance improvements. I can't see any major downsides.

Created attachment 209041 [details] [review]
mini object refcounting overhead micro-optimisations

In case someone feels like benchmarking.. (didn't like to put g_return_val() etc. into inline functions in header, so did it like this)

I can't be bothered to benchmark this, nor can anyone else apparently, so let's just WONTFIX this, since this shouldn't be an issue any more in 0.11.