I recently downloaded and compiled fvwm2 for cygwin (having been using it on solaris for years) and was pleased to have it compile and run first time.
However, cygwin is a little slow and I was curious about the fvwm2 source, so I downloaded it and decided to see if there was anything (even in the smallest way) to improve performance.
I think I found something that could improve general performance which I describe below, and would welcome feedback on whether to implement this.
1) I came across libs/safemalloc.c.
I was surprised that malloc was wrapped in a function like this since this is called whenever something is put on the (I think) event queue. Everytime I move the pointer from one window to another, launch tools etc, safemalloc is called. Wrapping malloc in a function is an overhead that will cause branches from the CPU cache probably when only one is necessary (the actual call to malloc itself).
So #if 0'd out the safemalloc, safecalloc, saferealloc functions and recoded them as macros. E.G.
#define safemalloc(length) \
(malloc((length) <= 0 ? 1 : (length)) ? : (char *)alloc_failed("malloc",(length)))
The main advantage of this is that only one function call for each malloc is required (malloc itself) instead of two (safemalloc+malloc). ?: is supposed to be more efficient than if () but I'm not sure thats really true. If I could only disassemble the code.... Suggestions? One function call should reduce the number of branches (and therefore cache usage is improved).
The biggest disadvantage of this however is that the binary size increases because these decisions (length <= 0) are duplicated everywhere. Somewhat prone to casting issues of length.
2) The other related issue I found was that in some cases, dynamic memory is used for simple temporary use. Local stack arrays are much more efficient for this, however you don't want to restrict the size of the data being processed. I suggest that the majority of users will have little data and most optimisation can come from putting those into a local array, and only calling on safemalloc when handling larger data quantities.
libs/Graphics.c - do_relieve_rectangle
XSegment* seg,segarr[5];
..
if(line_width > 5)seg = (XSegment*)safemalloc(sizeof(XSegment) * line_width);elseseg = &segarr[0];...if(line_width > 5)free(seg)
This would completely remove the malloc for 90% of users.
I have half a dozen of these kind of things and a place or two where memset is called prior to a memcpy that overwrites the same block memset cleared.
What do you think? While these are trivial changes, for a function that is called hundreds of times, even a small gain adds up. Shall I go ahead and make these changes? Perhaps as I grow more experienced with the fvwm2 source I can find some bigger issues.
Cheers
Dave
