Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 12, 2013 at 10:29 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:

> Jeff King <peff@xxxxxxxx> writes:
>
>> I think there are basically three classes of solution:
>>
>>   1. Declare __NO_INLINE__ everywhere. I'd worry this might affect other
>>      environments, who would then not inline and lose performance (but
>>      since it's a non-standard macro, we don't really know what it will
>>      do in other places; possibly nothing).
>>
>>   2. Declare __NO_INLINE__ on mingw. Similar to above, but we know it
>>      only affects mingw, and we know the meaning of NO_INLINE there.
>>
>>   3. Try to impact only the uses as a function pointer (e.g., by using
>>      a wrapper function as suggested in the thread).
>>
>> Your patch does (1), I believe. Junio's patch does (3), but is a
>> maintenance burden in that any new callsites will need to remember to do
>> the same trick.

Well, if by "everywhere" in (1) you mean "on all platforms" then
you're right. But my patch does not define __NO_INLINE__ globally, but
only at the time string.h / strings.h is included. Afterwards
__NO_INLINE__ is undefined. In that sense, __NO_INLINE__ is not
defined "everywhere".

> Agreed.  If that #define __NO_INLINE__ does not appear in the common
> part of our header files like git-compat-util.h but is limited to
> somewhere in compat/, that would be the perfect outcome.

It's not that easy to move the definition of __NO_INLINE__ into
compat/ because git-compat-util.h includes string.h / strings.h before
anything of compat/. More over, defining __NO_INLINE__ in somewhere in
compat/ would not limit its definition to the string.h / strings.h
headers only. So how about something like this on top of my original
patch:

--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,12 +85,16 @@
 #define _NETBSD_SOURCE 1
 #define _SGI_SOURCE 1

+#ifdef __MINGW32__
 #define __NO_INLINE__ /* do not inline strcasecmp() */
+#endif
 #include <string.h>
+#ifdef __MINGW32__
+#undef __NO_INLINE__
+#endif
 #ifdef HAVE_STRINGS_H
 #include <strings.h> /* for strcasecmp() */
 #endif
-#undef __NO_INLINE__

 #ifdef WIN32 /* Both MinGW and MSVC */
 #ifndef _WIN32_WINNT

-- 
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]