GNOME Bugzilla – Bug 101997
g_string_printf and g_string_append_printf
Last modified: 2011-02-18 16:07:18 UTC
The implementation of g_string_printf and g_string_append_printf is very wasteful. If my count is right, then the resulting string is copied twice in the normal case. It is also iterated over more times than I care to count. It seems to me that if a working C99 vsnprintf is around, one could do something like this: 1. If <100 characters available, make room for, say, 200 characters. 2. Do vsnprintf. 3. If buffer too small, resize based on return value and redo the vsnprintf. If vsnprintf is not around, a call to g_printf_string_upper_bound would get the size needed.
In the case where we have vasprintf, then (normally) g_strdup_printf() is just as efficient as the system vasprintf. If we don't have vasprintf, then we iterate over the string once to find the length, allocate a buffer, printf into it and return the buffer. We could kill one of the iterations in the second case in exchange for an extra copy, by guessing that the string will fit into a fixed size buffer, but I'm not sure that's worth it. (A benchmark might provide evidence one way or the other, though it would be very system dependent) In any case, we never copy the string (except for the unusual case where someone has changed the malloc vtable) and we walk over the arguments at most twice, so I don't see the huge amounts of inefficiency that you are talking about.
Ok, let me see... 1. Let's be nice and say that vasprintf is only one pass. 2. Then the string is copied from a malloc string to a gmalloc string. That's one copy plus one extra strlen pass. 3. Back over in g_string_append_printf_internal we call g_string_append. That's one extra copy and one extra strlen. (And the length presumably didn't change since step 2.) A total of five passes over that result and two copies of it. (Or three passes and one copy if step 2 isn't done.)
OK, I was thinking about g_strdup_printf() not g_string_..* You probably could optimize the g_string_* a bit.
I have removed the extra strlen in step 2 by using strndup. Removing the extra strlen inside g_string_append in step 3 would require a variant of g_strdup_vprintf which returns not only the allocated buffer, but also its length. Might be handy in other places as well in order to kill unnecessary strlens. How about gchar *g_strdup_vprintf_length (gint *length, const gchar *format, va_list args)
Note that g_strdup_vprintf_len is also requested for bug 92492
The patch attached to bug 92492 fixes step 3.