Bug 80798 - cleanup debug methods
Summary: cleanup debug methods
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.2.4.1 rc
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: difficultyMedium, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: DOCX
  Show dependency tree / graph
 
Reported: 2014-07-02 10:06 UTC by Michael Meeks
Modified: 2021-01-04 17:20 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2014-07-02 10:06:37 UTC 
Loading the bug document in fdo#76260 - we do 8.7 million calls to create OOXMLPropertySetImpl - the vast majority of the cost of that structure is the heap allocation and copying of the debugging member:

OOXMLPropertySetImpl::OOXMLPropertySetImpl()
: msType("OOXMLPropertySetImpl")

We burn 6bn cycles doing just this (per element I guess).

That type is only useful for debugging.

I'll fix the performance issue in this local case - but we shouldn't be exposing this virtual method when it is not used widely I guess.

It'd be nice to have this compiled out of the relevant writerfilter::Reference etc. vtables and/or removed completely.

Its unclear to me what good it provides that some RTTI typeid(*ptr).name() can give us...

Thoughts miklos ? =) 

Hopefully we can leave this as an easy hack though ...
Comment 1 Michael Meeks 2014-07-02 11:40:41 UTC 
I've pushed the speedup part; so dropping that from the title for now - even though not doing this OUString thrash would save quite some time really.
Comment 2 Björn Michaelsen 2014-12-02 10:53:09 UTC 
adding LibreOffice developer list as CC to unresolved Writer EasyHacks for better visibility.

see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Comment 3 Robinson Tryon (qubit) 2015-12-10 11:40:54 UTC Comment hidden (obsolete)
Comment 4 Robinson Tryon (qubit) 2016-02-18 14:51:34 UTC Comment hidden (obsolete)
Comment 5 Ekansh Jha 2018-01-27 06:25:08 UTC 
Hi! I would like to work on this issue. I have found some "writerfilter::Reference" but couldn't find anything sort of "OOXMLPropertySetImpl::OOXMLPropertySetImpl()
". 
>It'd be nice to have this compiled out of the relevant writerfilter::Reference
>etc. vtables and/or removed completely.

Could you please brief about it.
Comment 6 Michael Meeks 2018-01-29 09:03:36 UTC 
Interesting; this task got simpler, and the class got renamed:

class OOXMLPropertySet : public writerfilter::Reference<Properties>
...

Can we remove the maType member ? which is now an OUString (for no apparent reason) and gets intialized and deleted along with that class for no benefit at all (that I can see ;-). But needs some code-reading.

Thanks !
Comment 7 Miklos Vajna 2018-01-29 10:56:39 UTC 
I think the only user of that name is the SW_DEBUG_WRITERFILTER=1 ./soffice.bin foo.docx code which creates a /tmp/foo.docx.DOMAINMAPPER.xml file; and sure, having the type info there manually is probably pointless. Please just test this writerfilter XML token dump when you do changes in the tokenizer (see if you break it), as that debug code has no testcase coverage.
Comment 8 Deb 2020-12-19 06:24:29 UTC 
There's no longer a maType member in  OOXMLPropertySet. I think this is fixed. Can we close it?
Comment 9 Michael Meeks 2021-01-04 17:20:53 UTC 
Thanks Deb - good point; closing ... =)