Re: g_strjoinv() is very slow
- From: David Odin <David Odin bigfoot com>
- To: Raja R Harinath <harinath cs umn edu>
- Cc: Havoc Pennington <hp redhat com>, gtk-devel-list gnome org
- Subject: Re: g_strjoinv() is very slow
- Date: Sun, 24 Sep 2000 20:10:11 +0200
On Sun, Sep 24, 2000 at 12:39:01PM -0500, Raja R Harinath wrote:
>
> The decision to use stpcpy is IMO independent of providing g_stpcpy().
> It is quite a simple function, and is usually a better alternative to
> strcat (at least Ulrich Drepper says so :-) -- so we should use it.
>
I agree with Ulrich :-)
> > diff -Nrudb glib.orig/glib.h glib/glib.h
> > --- glib.orig/glib.h Tue Sep 19 16:30:35 2000
> > +++ glib/glib.h Sun Sep 24 16:21:51 2000
> > @@ -1761,8 +1761,8 @@
> > gchar* g_strjoinv (const gchar *separator,
> > gchar **str_array);
> > void g_strfreev (gchar **str_array);
> > -
> > -
> > +gchar* stpcpy (gchar *dest,
> > + const char *src);
>
> That should be g_stpcpy.
>
Thanks. Fixed.
> > separator_len = strlen (separator);
> > len = 1 + strlen (str_array[0]);
> > - for(i = 1; str_array[i] != NULL; i++)
> > - len += separator_len + strlen(str_array[i]);
> > + for (i = 1; str_array[i] != NULL; i++)
> > + len += strlen(str_array[i]);
> > + len += separator_len*i;
>
> Should it be
>
> len += separator_len * (i - 1);
>
You're right.
> > string = g_new (gchar, len);
> > *string = 0;
> > strcat (string, *str_array);
> > + ptr = string + strlen (*str_array);
>
> Maybe just replace it with
>
> string = g_new (gchar, len);
> ptr = g_stpcpy (string, *str_array);
>
Yes.
> > for (i = 1; str_array[i] != NULL; i++)
> > {
> > - strcat (string, separator);
> > - strcat (string, str_array[i]);
> > + ptr = g_stpcpy (ptr, separator);
> > + ptr = g_stpcpy (ptr, str_array[i]);
>
> GLibc uses a more compact:
>
> ptr = g_stpcpy (g_stpcpy (ptr, separator), str_array[i]);
>
It may be more compact, but it isn't really different and it's harder to
read.
> > s = va_arg (args, gchar*);
> > strcat (string, s);
> > + ptr = string + strlen (s);
> >
> > s = va_arg (args, gchar*);
> > while (s)
> > {
> > - strcat (string, separator);
> > - strcat (string, s);
> > + ptr = g_stpcpy(ptr, separator);
> > + ptr = g_stpcpy(ptr, s);
>
> Similar comments.
>
OK. Thanks for all your comments.
I've attached a new version which takes all your remarks in account.
Best regards,
DindinX
--
David Odin bigfoot com
If Bill Gate$ had a nickel for every time window$ crashed...
... oh, wait, he does...
diff -Nrudb glib.orig/CVS/Entries.Log glib/CVS/Entries.Log
--- glib.orig/CVS/Entries.Log Sun Sep 24 17:05:42 2000
+++ glib/CVS/Entries.Log Thu Jan 1 01:00:00 1970
@@ -1,2 +0,0 @@
-A D/gmemres////
-R D/gmemres////
diff -Nrudb glib.orig/config.h.win32.in glib/config.h.win32.in
--- glib.orig/config.h.win32.in Thu Sep 21 18:17:31 2000
+++ glib/config.h.win32.in Sun Sep 24 16:19:04 2000
@@ -102,6 +102,9 @@
/* Define if you have the on_exit function. */
/* #undef HAVE_ON_EXIT */
+/* Define if you have the stpcpy function. */
+/* #undef HAVE_STPCPY */
+
/* Define if you have the strcasecmp function. */
/* #undef HAVE_STRCASECMP */
diff -Nrudb glib.orig/configure.in glib/configure.in
--- glib.orig/configure.in Thu Sep 21 14:49:10 2000
+++ glib/configure.in Sun Sep 24 16:19:29 2000
@@ -357,7 +357,7 @@
GLIB_SIZEOF([$size_includes], intmax_t, intmax_t)
# Check for some functions
-AC_CHECK_FUNCS(lstat strerror strsignal memmove vsnprintf strcasecmp strncasecmp poll getcwd)
+AC_CHECK_FUNCS(lstat strerror strsignal memmove vsnprintf stpcpy strcasecmp strncasecmp poll getcwd)
# Check if bcopy can be used for overlapping copies, if memmove isn't found.
# The check is borrowed from the PERL Configure script.
diff -Nrudb glib.orig/glib.def glib/glib.def
--- glib.orig/glib.def Thu Sep 21 18:17:31 2000
+++ glib/glib.def Sun Sep 24 16:20:07 2000
@@ -391,6 +391,7 @@
g_static_rw_lock_writer_lock
g_static_rw_lock_writer_trylock
g_static_rw_lock_writer_unlock
+ g_stpcpy
g_str_equal
g_str_hash
g_strcanon
diff -Nrudb glib.orig/glib.h glib/glib.h
--- glib.orig/glib.h Tue Sep 19 16:30:35 2000
+++ glib/glib.h Sun Sep 24 19:55:55 2000
@@ -1761,8 +1761,8 @@
gchar* g_strjoinv (const gchar *separator,
gchar **str_array);
void g_strfreev (gchar **str_array);
-
-
+gchar* g_stpcpy (gchar *dest,
+ const char *src);
/* calculate a string size, guarranteed to fit format + args.
*/
diff -Nrudb glib.orig/gstrfuncs.c glib/gstrfuncs.c
--- glib.orig/gstrfuncs.c Sun Sep 10 18:04:33 2000
+++ glib/gstrfuncs.c Sun Sep 24 20:05:07 2000
@@ -118,6 +118,21 @@
return str;
}
+gchar *
+g_stpcpy (gchar *dest,
+ const gchar *src)
+{
+ g_return_val_if_fail (dest != NULL, NULL);
+ g_return_val_if_fail (src != NULL, NULL);
+
+#ifdef HAVE_STPCPY
+ return stpcpy (dest, src);
+#else
+ strcpy(dest, src);
+ return dest+strlen(src);
+#endif
+}
+
gchar*
g_strdup_vprintf (const gchar *format,
va_list args1)
@@ -156,6 +171,7 @@
va_list args;
gchar *s;
gchar *concat;
+ gchar *ptr;
g_return_val_if_fail (string1 != NULL, NULL);
@@ -170,14 +186,14 @@
va_end (args);
concat = g_new (gchar, l);
- concat[0] = 0;
+ ptr = concat;
- strcat (concat, string1);
+ ptr = g_stpcpy (ptr, string1);
va_start (args, string1);
s = va_arg (args, gchar*);
while (s)
{
- strcat (concat, s);
+ ptr = g_stpcpy (ptr, s);
s = va_arg (args, gchar*);
}
va_end (args);
@@ -1554,6 +1570,7 @@
gchar **str_array)
{
gchar *string;
+ gchar *ptr;
g_return_val_if_fail (str_array != NULL, NULL);
@@ -1566,17 +1583,19 @@
guint separator_len;
separator_len = strlen (separator);
+ /* First part, getting length */
len = 1 + strlen (str_array[0]);
- for(i = 1; str_array[i] != NULL; i++)
- len += separator_len + strlen(str_array[i]);
+ for (i = 1; str_array[i] != NULL; i++)
+ len += strlen(str_array[i]);
+ len += separator_len*(i-1);
+ /* Second part, building string */
string = g_new (gchar, len);
- *string = 0;
- strcat (string, *str_array);
+ ptr = g_stpcpy(string, *str_array);
for (i = 1; str_array[i] != NULL; i++)
{
- strcat (string, separator);
- strcat (string, str_array[i]);
+ ptr = g_stpcpy (ptr, separator);
+ ptr = g_stpcpy (ptr, str_array[i]);
}
}
else
@@ -1593,6 +1612,7 @@
va_list args;
guint len;
guint separator_len;
+ gchar *ptr;
if (separator == NULL)
separator = "";
@@ -1605,7 +1625,8 @@
if (s)
{
- len = strlen (s);
+ /* First part, getting length */
+ len = 1 + strlen (s);
s = va_arg (args, gchar*);
while (s)
@@ -1615,19 +1636,19 @@
}
va_end (args);
- string = g_new (gchar, len + 1);
- *string = 0;
+ /* Second part, building string */
+ string = g_new (gchar, len);
va_start (args, separator);
s = va_arg (args, gchar*);
- strcat (string, s);
+ ptr = g_stpcpy(string, s);
s = va_arg (args, gchar*);
while (s)
{
- strcat (string, separator);
- strcat (string, s);
+ ptr = g_stpcpy(ptr, separator);
+ ptr = g_stpcpy(ptr, s);
s = va_arg (args, gchar*);
}
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
