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 151957 - inlined g_str_has optimisations
inlined g_str_has optimisations
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2004-09-06 07:22 UTC by Benoît Dejean
Modified: 2018-05-24 10:33 UTC
See Also:
GNOME target: ---
GNOME version: 2.7/2.8


Attachments
inline (2.78 KB, patch)
2004-09-06 07:22 UTC, Benoît Dejean
needs-work Details | Review
Iterate g_str_has_prefix/suffix a 100000000 times (518 bytes, text/plain)
2010-03-12 11:21 UTC, Christian Dywan
  Details
Optimize g_str_has_prefix/suffix updated (2.74 KB, patch)
2010-03-12 11:22 UTC, Christian Dywan
none Details | Review
Intorduce macros g_str_starts/ends_with (1.53 KB, patch)
2010-03-12 14:42 UTC, Christian Dywan
needs-work Details | Review

Description Benoît Dejean 2004-09-06 07:22:24 UTC
gcc is able to emit optimized code for (str|strn|mem)cmp( str, "litteral" ). The
current g_str_has_prefix / g_str_has_suffix does not allow this. I think using
g_str_has_*( str, "litteral" ) is a pretty common usage and worths this
optimisation.

the following patch renames g_str_has_* to _g_str_has_* and defines 4 macros ( 2
to prevent multiple argument evaluation )

some benchmark results (debian gcc-3.4 -O2)

  for(i = 0; i < 100000000; ++i)
    {
      res += g_str_has_prefix(s, "Stargate");
      res += g_str_has_suffix(s, "Stargate");
      res += g_str_has_prefix(s, "Super");
      res += g_str_has_suffix(s, "docious");
    }

PPC G4 1GHZ orig 65s -> patched 22s
XP 1,666GHZ orig 37s -> patched 16s

please, consider this patch.
Comment 1 Benoît Dejean 2004-09-06 07:22:58 UTC
Created attachment 31319 [details] [review]
inline
Comment 2 Matthias Clasen 2004-11-23 06:05:36 UTC
Comment on attachment 31319 [details] [review]
inline

The patch won't work as is, since _-prefixed functions are not exported, thus
can't be used in public macros. Also __extension__ should not be directly used,
we have a macro for that.
Comment 3 Benoît Dejean 2004-11-23 06:17:33 UTC
what about introducing 2 new macros : g_str_starts_with and g_str_ends_width ?
Comment 4 Christian Dywan 2010-03-12 11:17:22 UTC
I like the idea, but renaming the functions as the patch does is an ABI break (existing binaries would fail to run with the new glib). The way to go would be to introduce new macros g_str_starts/ends_with including the gcc optimization. Also note that you didn't modify glib.symbols so your patch can't compile as-is.
Comment 5 Christian Dywan 2010-03-12 11:21:13 UTC
Created attachment 155954 [details]
Iterate g_str_has_prefix/suffix a 100000000 times

A complete test case for convenience
Comment 6 Christian Dywan 2010-03-12 11:22:40 UTC
Created attachment 155956 [details] [review]
Optimize g_str_has_prefix/suffix updated

This is an updated patch, which actually compiles, but it still breaks ABI.
Comment 7 Christian Dywan 2010-03-12 14:42:53 UTC
Created attachment 155969 [details] [review]
Intorduce macros g_str_starts/ends_with
Comment 8 Philip Withnall 2018-05-16 09:27:36 UTC
I think it should be possible to keep the existing functions (g_str_has_[prefix|suffix]()) and their exported symbols, and just define two macros with the same names (g_str_has_[prefix|suffix]()) in the case that GCC is being used and extensions are available.

As long as the names of the functions are protected in brackets in their declarations and calls, everything should work OK. This is what we do with (for example) g_object_ref() and g_object_ref_sink() to add type propagation for them. (See commit 3fae39a5d742afe73741f5fd7aa24e3ae8182f06, for example.)
Comment 9 GNOME Infrastructure Team 2018-05-24 10:33:06 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/24.