GNOME Bugzilla – Bug 437457
saving relocations for GstElementDetails
Last modified: 2007-06-22 10:14:01 UTC
replacing this code: static const GstElementDetails gst_identity_details = GST_ELEMENT_DETAILS ("Identity", "Generic", "Pass data without modification", "Erik Walthinsen <omega@cse.ogi.edu>"); gst_element_class_set_details (gstelement_class, &gst_identity_details); with gst_element_class_set_details_simple (gstelement_class, "Queue", "Generic", "Simple data queue", "Erik Walthinsen <omega@cse.ogi.edu>"); and having this in e.g gstelement.c void gst_element_class_set_details_simple (GstElementClass * klass, gchar *longname, gchar *classification, gchar *description, gchar *author) { const GstElementDetails details = GST_ELEMENT_DETAILS (longname, classification, description, author); g_return_if_fail (GST_IS_ELEMENT_CLASS (klass)); __gst_element_details_copy (&klass->details, &details); } saves 4 relocations per GstElementDetails. You can check as below: readelf -d /usr/lib/gstreamer-0.10/libgstcoreelements.so | grep REL For some background please read: http://bugzilla.gnome.org/show_bug.cgi?id=337058
It'd be nice to have a patch to try out.
Created attachment 88579 [details] [review] save relocs for GTypeInfo This imho is save to apply.
Created attachment 88580 [details] [review] save relocs caused by GstElementDetails there is a '#if 0' in there to switch before/after for easy testing. I am not 100% sure about this one. It lowers the number of relocs (as explained in detail on the -devel maling-list), but makes the binary grow also.
I'm in favor of this change because it makes the code (IMO) easier to read. I've always been a bit uncomfortable with using static constructor for something that is easily constructed with a function call. FWIW, I feel the same way about static pad templates and static caps, too.
Because it adds API, I'd like to punt it until after the release, then apply it immediately so we have time to let it settle.
agreed. I marked the patches as commit-after-freeze.
I think this can be committed now, no?
2007-06-21 Stefan Kost <ensonic@users.sf.net> * gst/gstelement.c: (gst_element_class_set_details_simple): * gst/gstelement.h: * gst/gstutils.c: (gst_type_register_static_full): * gst/gstutils.h: * plugins/elements/gstcapsfilter.c: (gst_capsfilter_base_init): * plugins/elements/gstfakesink.c: (gst_fake_sink_base_init): * plugins/elements/gstfakesrc.c: (gst_fake_src_base_init): * plugins/elements/gstfdsink.c: (gst_fd_sink_base_init): * plugins/elements/gstfdsrc.c: (gst_fd_src_base_init): * plugins/elements/gstfilesink.c: (gst_file_sink_base_init): * plugins/elements/gstfilesrc.c: (gst_file_src_base_init): * plugins/elements/gstidentity.c: (gst_identity_base_init): * plugins/elements/gstmultiqueue.c: (gst_multi_queue_base_init): * plugins/elements/gstqueue.c: (gst_queue_base_init), (apply_buffer), (gst_queue_chain): * plugins/elements/gsttee.c: (gst_tee_base_init): * plugins/elements/gsttypefindelement.c: (gst_type_find_element_base_init), (gst_type_find_element_class_init): Saving relocations for GTypeInfo and GstElementDetails. Fixes #437457.
Sorry, I know you just committed stuff and closed the bug, but I have to ask, because I don't really understand this new API: Instead of static const GTypeInfo, we now call a function which pushes the contents of a GTypeInfo onto the stack as arguments, then pulls it off to put into a stack allocated GTypeInfo and passes that to g_type_register_static. Wouldn't removing 'static' from the existing GTypeInfo do the same thing?
@Jan, its quick tricky: Here is what happens: libgstaudiodelay is a plugin with one element, that had the global static const GstElementDetails (before). Next I moved it as a local var into _base_init(). Then I removed static and finally I used the gst_element_class_set_details_simple(). These are the binary sizes: 51029 libgstaudiodelay.before.so* 51035 libgstaudiodelay.local-nonstatic.so* 51035 libgstaudiodelay.local.so* 50972 libgstaudiodelay.simple.so* > readelf -d libgstaudiodelay.before.so | grep REL 0x00000002 (PLTRELSZ) 320 (bytes) 0x00000012 (RELSZ) 200 (bytes) 0x6ffffffa (RELCOUNT) 21 > readelf -d libgstaudiodelay.local.so | grep REL 0x00000002 (PLTRELSZ) 320 (bytes) 0x00000012 (RELSZ) 200 (bytes) 0x6ffffffa (RELCOUNT) 21 > readelf -d libgstaudiodelay.local-nonstatic.so | grep REL 0x00000002 (PLTRELSZ) 320 (bytes) 0x00000012 (RELSZ) 200 (bytes) 0x6ffffffa (RELCOUNT) 21 > readelf -d libgstaudiodelay.simple.so | grep REL 0x00000002 (PLTRELSZ) 320 (bytes) 0x00000012 (RELSZ) 168 (bytes) 0x6ffffffa (RELCOUNT) 17 Some of the savings in file-size are because now there is no code to initialize the struct and only a little more code for the function call. The savings of reloc-entries is because the struct contains pointers to strings which need to be relocated. It the struct would only contain constant numbers (like a bitrate table) it would be fine.
Nicely summarised :) OK, that explains it well, thanks!