After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 500143 - Some small improvements (limiting realloc and CPU usage )
Some small improvements (limiting realloc and CPU usage )
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: 0.10.16
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-11-28 07:55 UTC by Laurent Glayal
Modified: 2008-01-09 16:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gstcaps.c (747 bytes, patch)
2007-11-28 07:56 UTC, Laurent Glayal
rejected Details | Review
gststructure.c (1.47 KB, patch)
2007-11-28 07:56 UTC, Laurent Glayal
rejected Details | Review
gstvalue.c (2.07 KB, patch)
2007-11-28 07:56 UTC, Laurent Glayal
needs-work Details | Review

Description Laurent Glayal 2007-11-28 07:55:31 UTC
gstcaps.c ::
  - preallocate string size to avoid too many realloc
  
gststructure.c ::
  - replace some for() iterations by a while(i--) which is faster.
  - preallocate string size to avoid too many realloc

gstvalue.c ::
  - replace some for() iterations by a while(i--) which is faster.
  - replace 
for (n = 0; n < size; n++) {
       kid = gst_value_array_get_value (value, n);
       fixed &= gst_value_is_fixed (kid);
     }

by 
    for (n = 0; n < size && fixed; n++) {
       kid = gst_value_array_get_value (value, n);
       fixed &= gst_value_is_fixed (kid);
     }

to exit the loop when 'fixed' turned to be false.
Comment 1 Laurent Glayal 2007-11-28 07:56:01 UTC
Created attachment 99757 [details] [review]
gstcaps.c
Comment 2 Laurent Glayal 2007-11-28 07:56:18 UTC
Created attachment 99758 [details] [review]
gststructure.c
Comment 3 Laurent Glayal 2007-11-28 07:56:37 UTC
Created attachment 99759 [details] [review]
gstvalue.c
Comment 4 David Schleef 2007-12-07 22:49:08 UTC
I'd prefer to trust that g_string is doing things fairly efficiently.  If not, I recommend fixing g_string.

"while(i--)" is not inherintly faster than a for() loop unless you have a broken compiler.  As the standard iterator in C is for(), we use that.  Seeing "while()" a simple iterator in code typically makes me wonder why it's a) done differently than common usage, and b) why the special usage is not documented.

Adding 'fixed' as a condition to the for loop in gstvalue.c is useful and should be applied.  My personal preference would be to use 'if (!fixed) break;' instead, but that's me.
Comment 5 Laurent Glayal 2007-12-10 12:15:37 UTC
Keeping for() is ok, trusting compiler, but g_string is costly as strings for capabilities are often reallocated due to concatenation. 

g_string_new() does not (can't ?) anticipate any future use of the new string, so having glib 'fixed' seems to me quite uncertain.
Comment 6 Tim-Philipp Müller 2008-01-09 16:40:56 UTC
 2008-01-09  Tim-Philipp Müller  <tim at centricular dot net>

	* gst/gst_private.h: (STRUCTURE_ESTIMATED_STRING_LEN):
	* gst/gstcaps.c: (gst_caps_to_string):
	* gst/gststructure.c: (GST_ASCII_IS_STRING),
	  (priv_gst_structure_append_to_gstring), (gst_structure_to_string):
	  Yet another gratuitous GString micro-optimisation: add a (private)
	  function that serialises a structure appending to an existing
	  GString, so that when we serialise caps we don't need to alloc+free
	  a throwaway GString for each structure (each of which also entailing
	  multiple reallocs on the way); also use g_string_sized_new() in
	  various places with an approximate string length to avoid reallocs
	  within GString. See #500143.

 2007-12-28  Tim-Philipp Müller  <tim at centricular dot net>

	Based on patch by: Laurent Glayal  <spglegle yahoo fr>

	* gst/gstvalue.c: (gst_value_is_fixed):
	  Optimisation: bail out of the loop as early as possible (#500143).