Hi,
On Fri, Aug 20, 2004 at 04:50:36PM +0100, Robert Shearman wrote:
Index: wine/dlls/ntdll/critsection.c
RCS file: /home/wine/wine/dlls/ntdll/critsection.c,v retrieving revision 1.26 diff -u -p -r1.26 critsection.c --- wine/dlls/ntdll/critsection.c 17 Jun 2004 23:11:08 -0000 1.26 +++ wine/dlls/ntdll/critsection.c 20 Aug 2004 15:46:18 -0000 @@ -36,6 +36,14 @@ WINE_DEFAULT_DEBUG_CHANNEL(ntdll); WINE_DECLARE_DEBUG_CHANNEL(relay);
+#ifdef __GNUC__ +# define likely(x) __builtin_expect(!!(x), 1) +# define unlikely(x) __builtin_expect(!!(x), 0) +#else +# define likely(x) !!(x) +# define unlikely(x) !!(x) +#endif
Umm, this should go into a global header file, right? The theory being that it would be useful at many more performance critical Wine places...
But I have to admit that I don't know which header file this should be...
Andreas Mohr
Andreas Mohr wrote:
Hi,
On Fri, Aug 20, 2004 at 04:50:36PM +0100, Robert Shearman wrote:
Index: wine/dlls/ntdll/critsection.c
RCS file: /home/wine/wine/dlls/ntdll/critsection.c,v retrieving revision 1.26 diff -u -p -r1.26 critsection.c --- wine/dlls/ntdll/critsection.c 17 Jun 2004 23:11:08 -0000 1.26 +++ wine/dlls/ntdll/critsection.c 20 Aug 2004 15:46:18 -0000 @@ -36,6 +36,14 @@ WINE_DEFAULT_DEBUG_CHANNEL(ntdll); WINE_DECLARE_DEBUG_CHANNEL(relay);
+#ifdef __GNUC__ +# define likely(x) __builtin_expect(!!(x), 1) +# define unlikely(x) __builtin_expect(!!(x), 0) +#else +# define likely(x) !!(x) +# define unlikely(x) !!(x) +#endif
Umm, this should go into a global header file, right? The theory being that it would be useful at many more performance critical Wine places...
But I have to admit that I don't know which header file this should be...
Like in the Linux kernel (where this seems to come from) in compiler.h or something like that.
bye michael
Michael Stefaniuc mstefani@redhat.com writes:
Andreas Mohr wrote:
Umm, this should go into a global header file, right? The theory being that it would be useful at many more performance critical Wine places... But I have to admit that I don't know which header file this should be...
Like in the Linux kernel (where this seems to come from) in compiler.h or something like that.
No, I don't think we want to do that, it would only encourage people to sprinkle this stuff all over the place believing they are "optimizing" the code. This should only be added in places where it makes a significant difference, demonstrated with benchmark results.
Alexandre Julliard wrote:
Michael Stefaniuc mstefani@redhat.com writes:
Andreas Mohr wrote:
Umm, this should go into a global header file, right? The theory being that it would be useful at many more performance critical Wine places... But I have to admit that I don't know which header file this should be...
Like in the Linux kernel (where this seems to come from) in compiler.h or something like that.
No, I don't think we want to do that, it would only encourage people to sprinkle this stuff all over the place believing they are "optimizing" the code. This should only be added in places where it makes a significant difference, demonstrated with benchmark results.
Yes, I agree. In fact the patch actually decreases performance. Test: 10,000,000 calls to EnterCriticalSection & LeaveCriticalSection, simulating fast path on critical sections. Results: Win2k - 1000ms Wine w/ patch - 1346ms Wine w/o patch - 1294ms
I think the best way to improve performance of the critical section functions is to rewrite them in assembly and I will submit a patch doing so when I have a spare moment.
Rob
Robert Shearman rob@codeweavers.com writes:
Yes, I agree. In fact the patch actually decreases performance. Test: 10,000,000 calls to EnterCriticalSection & LeaveCriticalSection, simulating fast path on critical sections. Results: Win2k - 1000ms Wine w/ patch - 1346ms Wine w/o patch - 1294ms
I think the best way to improve performance of the critical section functions is to rewrite them in assembly and I will submit a patch doing so when I have a spare moment.
I don't think that's necessary either, unless you have good evidence that the critical section non-contention path represents a significant amount of the total time spent by an application. IMO we have much bigger performance issues to solve first, before we make the critical section code unreadable just to save a couple of cycles.
Win2k - 1000ms Wine w/ patch - 1346ms Wine w/o patch - 1294ms
That's a fascinatingly round number for Win2K, isn't it?
I suspect the slowness is caused by the involution: any cycles saved from the branch hinting is lost in the double-not.
thanks -mike