Fw: General performance [was: drawing performance]


Subject: Fw: General performance [was: drawing performance]
From: Mike Nordell (tamlin@algonet.se)
Date: Sun Dec 17 2000 - 22:37:52 CST


And once again I forgot to change the recipient adress while replying. :-(
Oh well, here's the reply, this time where it should be.

> Dom Lachowicz wrote:
> > Mike,
> >
> > Do you think that it would be prudent to follow this comment's
suggestion
> in
> > pp_Property.cpp:
> >
> > /*
> > TODO we can make this faster later by storing all the property names
> > in alphabetical order and doing a binary search.
> > */
>
> No. Please read my profiling figures; over *one million calls* to
"tolower"
> just by typing about one hundred chars in AW since the property stuff is
> _case insensitive_. Should we store the properties in lower-case and
"only"
> lowercase the given string argument, we would reduce the number of calls
to
> tolower() to one tenth, but it would IMO still be unacceptable (over 40
> million CISC CPU cycles just to insert 100 chars).
> No, we need something more efficient.
>
> > Alternatively, we might be able to store these strings in a hashtable
for
> an
> > O(1) lookup penalty, which is usually pretty good ;)
>
> 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.
>
> Do we even have any non-hard-coded property names?
>
> > Of course, we should
> > probably make platform-specific string libraries too, since stricmp
seems
> to
> > be slowing us down significantly. Neither would be too hard to do. We
can
> > always "fall back" on our existing string libs if a replacement isn't
> > currently defined.
>
> 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.
>
> > Could you profile loading a document and see if there are any similar
> > hold-ups?
>
> Sure. Any suggestions/request for a specific document (or document type
> ;-) )?
> I'll start with one of the ABW docs.
>
> All figures given are including calls to descendants unless explicitly
> stated otherwise.
>
> Profiled loading of docs/AbiWord_DocumentFormat.abw. This wasn't even fun.
> :-(
>
> 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.
>
> 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. The offending function here is
pt_PieceTable::_tellAndMaybeAddListener
> (what's that? "maybe add listener"?! It was just told to *do* add the
> listener). Well, once again it turns out that the problem is handling of
> 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.
>
> 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.
>
> /Mike - please don't cc



This archive was generated by hypermail 2b25 : Sun Dec 17 2000 - 22:36:21 CST