Hi Dominik,
> Fvwm's speed depends to 90% or more on how the communication with
> the X server is done. All other effects of calling functions and
> such only add very little to the CPU load. Once I profiled fvwm
> for ten minutes using gprof, and as far as I remember it did not
> even consume one second of CPU time. The things that slow it down
> are file I/O and X server communication.
I agree there are bigger fish to fry - this was my first look at fvwm source.
I thought it wiser to start with small steps.... I was thinking I would take a
look at XFree86 too sometime. I need to learn much about X primitives.
In terms of the performance - I felt like the system was more responsive in
cygwin with the modifications. Perhaps it was just my hopeful imagination!
What OS and CPU were you running when you profiled?
> Talking about the code execution time, there is only one big
> factor, the PositiveWrite() function that sends information to
> the modules. It can be called hundreds of thousands of times
> during a busy work day. Much effort has been made to optimize
> that function.
>
AddToMessageQueue has 2 safemalloc calls - That equates to "hundreds of
thousands"*2 unnecessary function calls.
If you want to really optimise this, the first call could use malloc directly
dispensing with an unnecessary if (length == 0) as well as the function call.
Shouldn't do this on the second one because size could be any value.
> > 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).
>
> I feel the added code size is not worth the effort most of the
> time. If individual functions are called ofthen from a certain
> place it may be better to only optimize that place instead of all
> the others where it isn't a problem. For example, many of the
> safemalloc() calls could be replaced with alloca(), even saving
> the free() call.
Hmm that sounds like a good change to do. I will look at that.
> > 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.
> >
> > [ code snipped ]
> > This would completely remove the malloc for 90% of users.
>
> Yes, but what is the cost of the malloc?
What does the cost matter when it can be reduced to 10% of the cost?
> RelieveRectangle is
> perhaps called 20 times when a window changes focus. I'd guess on
> average you do not do this more than 5 times per minute, giving a
> total of 6000 calls per hour. I'd estimate that can easily be
> done by any but the slowest machines in less than the tenth of a
> second.
My only contention here is that if you don't need to do it, why do it. Even
tenths of seconds here and there add up.
> It is likely that more time could be saved by preventing
> unnecessary calls to RelieveRectangle() in a single place in the
> code since it requires communication over the network.
Sounds like another promising avenue - I will take a look.
> > 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.
>
> Do you have a list of these? Some of the memset() calls are
> redundant. Not all of these are bad though. Some are merely
> intended to prevent uninitialised elements of structures if new
> members are added to the structures in the future.
The places where I'm thinking of use sizeof in both calls so if one misses a
new element, so would the other.
Here they are:
menus.c NewMenuStyle()
tmpms = (MenuStyle *)safemalloc(sizeof(MenuStyle));
memset(tmpms, 0, sizeof(MenuStyle));
if (ms)
{
/* copy the structure over our temporary menu face. */
memcpy(tmpms, ms, sizeof(MenuStyle));
The memset can be safely moved to the else leg of the if.
scanForColor()
/* save instring in case can't find pixmap */
save_instring = (char *)safemalloc(strlen(instring)+1);
name = (char *)safemalloc(strlen(instring)+1);
strcpy(save_instring,instring);
/* Scan whole string */
for (txt = instring; *txt != '\0'; txt++)
{
...
}
free(name);
free(save_instring);
can become:
int l = strlen(instring) + 1;
/* save instring in case can't find pixmap */
save_instring = (char *)safemalloc(l*2);
name = save_instring + l;
strcpy(save_instring,instring);
/* Scan whole string */
for (txt = instring; *txt != '\0'; txt++)
{
...
}
free(save_instring);
add_window.c setup_window_structure()
This memset isn't needed its only used in the else leg, and then its copied
over.:
memset(&save_state, '\0', sizeof(FvwmWindow));
I thought I found more, but these were the ones I had written down.
> > 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.
>
> I suggest this:
>
> - Find places where alloca() can be used instead of safemalloc.
> alloca is either a system function or a replacement from the
> fvwm library. This eliminates the additional function call and
> even cuts down the chances of running out of memory. A program
> that can't get more space on the stack is doomed to crash
> anyway. It also reduces the chance of memory leaks.
> - Leave safemalloc as it is. If there are specific functions
> that are called frequently, optimizing them is a good thing.
> - In many places there is code like
> ptr = safemalloc(size);
> memset(ptr, 0, size);
> This could be done faster and smaller with safecalloc().
> - For the big gain: optimize the X calls. Any call that
> contacts the X server needlessly causes delays.
>
I can easily attack the first three. My current inexperience with X limits the
last one, but I am interested in learning X development so I will look around
(I will look at the ReleiveRectangle calls first).
> And a comment on coding style:
>
> - Macros with code are evil. Never use them. Instead, you can
> inline the code (although it doesn't work with any compiler).
Here we will have to agree to disagree. I believe macros with and without code
have their place. There are certain things that shouldn't be done in macros,
but I wouldn't call them evil!
Cheers
Dave
>
> Bye
>
> Dominik ^_^ ^_^
>
> --
> Dominik Vogt, email: d.vogt_at_lifebits.de
> LifeBits Aktiengesellschaft, Albrechtstr. 9, D-72072 Tuebingen
> fon: ++49 (0) 7071/7965-0, fax: ++49 (0) 7071/7965-20
--
Visit the official FVWM web page at <URL: http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm" in the body of a
message to majordomo_at_fvwm.org.
To report problems, send mail to fvwm-owner_at_fvwm.org.
Received on Fri Mar 22 2002 - 21:49:45 GMT