[poppler] Small perf improvement to bug 11849

[Albert Astals Cid]

A Dimecres 26 Setembre 2007, Krzysztof Kowalczyk va escriure:
> The code is really trivial: a 4 element stack, in delete objects are
> pushed on stack (if there's space), in new objects are popped from
> stack.
>
> What's brainded about overloading new/delete?

In that i've never ever seen new/delete overloaded in any code, while i've 
seen almost every other operator overloaded. 

So i as a pretty decent C++ developer expect new to behave as new and not give 
me something reused.

> After some thinking I think overloading is better because it has one
> big advantage: it requires no changes to poppler code (other than
> adding some more code).

That's true.

As said, i'll have a better opinion once i have a look at the code :-)

Albert

>
> In that it makes merges with xpdf easier.
>
> Without overloading you need to replace every new/delete SplashPath
> with SplashPath::create/SplashPath::destroy, which results in more
> changes.
>
> -- kjk
>
> On 9/26/07, Albert Astals Cid <aacid at kde.org> wrote:
> > A Dimecres 26 Setembre 2007, Krzysztof Kowalczyk va escriure:
> > > So I started looking at perf again.
> > >
> > > The first improvement is ~4% rendering speedup for PDF in
> > > https://bugs.freedesktop.org/show_bug.cgi?id=11849.
> > >
> > > From the profile it looked like in this case a great deal (129899) of
> > > SplashPath objects was created/destroyed, yet at any given time only a
> > > few were allocated at the same time, so a small cache to recycle
> > > objects improves that. With this PDF I get the following changes for
> > > alloc functions counts:
> > >
> > > before:
> > > gfree  0.31   0.31    100.00  1092627
> > > gmalloc        0.18   0.18    100.00  548663
> > > grealloc 0.13 0.13    100.00  389808
> > >
> > > after:
> > > gfree 0.16    0.16    100.00  702930
> > > gmalloc       0.14    0.14    100.00  548663
> > > grealloc0.03  0.03    100.00  98382
> > >
> > > I measure speed up with the newly added test/perf-test, using release
> > > build on windows, like this:
> > > perftest.exe -timings c:\kjk\downloads\slow00.pdf
> > >
> > > For comparison I take the smallest number out of few runs.
> > >
> > > You'll see the code when I solve my branch pushing troubles.
> > >
> > > There are some issues:
> > > * this applies to all caches: cache must be emptied at some point,
> > > especially at the end, in order to not be seen as a leak. But poppler
> > > doesn't have mandatory init/free function to stuff it. So for now the
> > > user of the library needs to know. I was thinking it could be added
> > > either in PDFDoc destructor (based on assumption that we only want the
> > > cache to live as long as the document, because it really only improves
> > > a small class of PDFs) or in GlobalParams, since it must be
> > > constructed/destructed
> > > * right now a static function SplashFont::create()/destroy() must be
> > > called instead of new/delete SplashFont but I think it could be done
> > > transparently by substituting new/delete for this class. Need to dust
> > > up those C++ books.
> >
> > I'm very hesitant on the amount of complex code that can introduce just
> > to win a 4% on some specific pdf, but we'll see once your branch is
> > online for testing.
> >
> > And also please, don't overload the new operator, imho that's braindead
> > and using SplashFont::create()/destroy() is just OK. new gives you the
> > "normal" behvaiour and create()/destroy() the cached one, that makes
> > sense.
> >
> > Albert
> >
> > > Also this approach could be used for a few other classes:
> > > SplashXPath                 47889    4        48      0       0      
> > > 574668 GfxPath                              42970    2       80      0 
> > >      0       1718800 SplashXPathScanner     42074    4     208     0   
> > >    0       2187848 SplashSolidColor           38410   20 160     0     
> > >  0       307280
> > >
> > > First number is total number of created objects during rendering this
> > > PDF, second is max number of objects existing at the same time. Second
> > > number is important for deciding cache size. If it's 4, cache of size
> > > 4 should provide almost 100% hit ratio (i.e. almost all allocations
> > > should be satisfied from the cache).
> > >
> > > -- kjk
> > > _______________________________________________
> > > poppler mailing list
> > > poppler at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/poppler
> >
> > _______________________________________________
> > poppler mailing list
> > poppler at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/poppler
>
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler