Subject: Re: General performance [was: drawing performance]
From: Dom Lachowicz (cinamod@hotmail.com)
Date: Sun Dec 17 2000 - 22:50:14 CST
I sent this to Mike but it should go to the list
>It would take some radical work to beat O(1). :-) But then you would have
>to
>create a "perfect hash" which is impossible if we don't know in advance
>what
>strings we are to use why O(1) would also be impossible.
>This was however one of the two approaches I had in mind but it won't be
>enough (IMHO) since *every call* to lookupProperties needs to tolower() the
>string argument. That's bad. I was thinking that we should create an enum
>for all (currently known - hard-coded) property names and overload
>lookupProperties on the enum type.
This is a good idea that I hadn't thought of. Read on...
>Now, why do I come to think that we really, really, need a string class?
>Yes, I'm prepared to have a look at it should enough interest be displayed.
Please do. I'm interested and that should count doubly or something
;)
>Profiled loading of docs/AbiWord_DocumentFormat.abw. This wasn't even fun.
>:-(
For a 5 page doc, this took a *while* (3-4 secs on a P3 933) to
load. Another example is any large word doc that I have. wvWare can
output it to HTML in about .0000000001sec. Abw freezes for a few
minutes while loading. Is an incremental layout engine needed? How
about a better property mechanism? I think so...
>Our timer uses almost 20% CPU in ten (10) calls to
>FL_DocLayout::_redrawUpdate. Almost all of this is to the current handling
>of properties.
Ick
>For the loading, fileOpen() uses 71% CPU, where *over 50% CPU* is used for
>*one* call to pt_PieceTable::addListener. Holy shit! And now we once again
>come back to one of the things I find as a *fundamental* design-flaw. To
>"add" a listener *is not* to tell everyone and his mother that this has
>been
>done.
I totally agree. Like in Java, a listener should just be something
else added to a queue/vector/stack/whatever that implements an
interface. It should get called for certain events and that's all.
This is a big problem, however, this is a separate issue than the
one at hand. Let's take things one at a time.
>properties. During loading of this document there was over two million
>calls
>to tolower() claiming over 43% CPU (over three second in this case). all
>but
>1156 were done from UT_stricmp, and that one in turn was called about
>375000
>times from PP_lookupProperty (94.91% of its callers). Note that I haven't
>even bothered to analyze the remaining 5.09% (=19247) of the calls to
>UT_stricmp since they didn't contribute to the total CPU load to be safely
>measured.
Ignore the lower fringe for now. If ~95% of the affending calls
(thus eating up 43% of your CPU) come from one function, we have
either bad code or a design flaw. In this case, we have both. For
now, I don't care where the other 5% come from.
>But I can conclude that UT_stricmp for this document chewed over 50% CPU on
>behalf of the property system and this tells me we need to redesign
>something.
Ok. First, although UT_stricmp is slowing us down, it is not the
primary culprit. A fast platform-specific string library would make
things a little better, but it would only hide the larger problem
which is a combination of bad design and bad code. IMHO, we should
implement platform-specific string libraries/classes *after* we find
a solution to our property design flaws. It's like taking a
decongestant for the flu - it might cure your symptom, but it
doesn't get rid of the virus.
Well, if you grep for UT_stricmp() (or search on LXR), you'll find a
lot of uses in the importers/exporters (98 files total). Sometimes,
its usage might be valid. But for things like ABW (and other XML
docs), a strcmp() like function should be used instead. We *should*
not allow things like <p PROPS="">. We have to change all of these
and be careful about it. Also, all of the XML-based importers have a
function called s_mapNameToToken() or something like that. Right now
they do a linear search. We should change this to a binary search or
something better. Combined with removing UT_stricmp(), this should
speed up XML imports hugely.
A *lot* of other places where UT_stricmp() is getting called are in
the Graphics classes (esp. the findFont method) and in the ap_App
classes. FindFont *really* slows us down noticeably. It isn't too
bad for the app classes.
In a lot of other places, usage of UT_stricmp is harmless really
(e.g. ap_Menu_LabelSet.cpp). By "harmless" I mean "used only once or
twice and could add to robustness."
Unfortunately, the rest of the places UT_stricmp is being called at
are FMT and PTBL classes like FV_View. This is *really* bad and
needs to stop. Combine this with the nonsense in the Graphics
classes, and you're in for one major performance hit.
Stop by on irc.gnome.org #abiword sometime tomorrow (Monday PM, US
EST) and we'll discuss this more. We need to get this taken care of
and I think that we're the right guys to do it. I promise to tackle
the Unix & Gnome FE if you handle Win32. We'll let Hub, Thomas, and
Chris handle their own FEs. We can work in XP-land together.
Dom
_________________________________________________________________
Get your FREE download of MSN Explorer at http://explorer.msn.com
This archive was generated by hypermail 2b25 : Sun Dec 17 2000 - 22:50:16 CST