Bug 81038 - performance: 10% of load: un-necessary pImpl
Summary: performance: 10% of load: un-necessary pImpl
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
4.4.0.0.alpha0+ Master
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:4.4.0
Keywords: difficultyInteresting, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2014-07-08 09:44 UTC by Michael Meeks
Modified: 2015-12-16 00:37 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2014-07-08 09:44:25 UTC
bug#66507 has a nice profile for a particularly slow to calculate document which shows that the FormulaTokenIterator::Jump and Push / Pop methods are particularly expensive source of the slowness there - primarily because of all the allocation and freeing they do of ImpTokenIterator structures.

include/formula/tokenarray.hxx has:

class FORMULA_DLLPUBLIC FormulaTokenIterator
{
    ImpTokenIterator* pCur;

We should make the ImpTokenIterator struct a private (but present in the header) class of FormulaTokenIterator.

We should also undo this very strange manual memory management / fragile linked list nonsense, and instead use a simple in-place vector [ie. not doing all this allocation ]. We should just push_back and pop stuff onto the vector and avoid the linked list sillies.

Of the 390bn cycles of the complete load/edit/save trace here, ~35bn are spent allocating and freeing this structure :-)
Comment 1 Michael Meeks 2014-07-08 09:46:52 UTC
Hmm, it's worse than this there is another 26bn cycles in 'PushWithoutError' here.
Comment 2 Jeffrey Stedfast 2014-07-27 01:32:48 UTC
This is now implemented & patch submitted
Comment 3 Commit Notification 2014-07-27 04:03:53 UTC
Jeffrey Stedfast committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ce4e7a830d5350848d3c83b872f916a7b1691266

fdo#81038 Fixed FormulaTokenIterator to use std::vector instead of linked list



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 4 Robinson Tryon (qubit) 2015-12-16 00:37:23 UTC
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyInteresting SkillCpp TopicCleanup)
[NinjaEdit]