Bug 78479 - very slow Asian text entry in writer ...
Summary: very slow Asian text entry in writer ...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: sdk (show other bugs)
Version:
(earliest affected)
4.2.4.1 rc
Hardware: Other All
: medium major
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-09 09:30 UTC by Michael Meeks
Modified: 2014-05-12 09:56 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
a photo of the issue =) (380.51 KB, image/png)
2014-05-09 09:31 UTC, Michael Meeks
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2014-05-09 09:30:05 UTC
Using my new feature/debugevent work - I entered random UCS2 into writer and was amazed to discover that the cost per key-stroke was -extremely- high.

Luckily its rather easy to profile that with callgrind on that branch; I've up-loaded a profile here:

http://users.freedesktop.org/~michael/callgrind-asian-entry-at-34ae7b16d.txt.gz

And I attach a screenshot. The punch-line is that ~all the time is spent continually loading and unloading an UNO component for break iterating =)

21% of the time from ~6000 calls to osl_loadModuleRelative, 25% of the time from OFactoryComponentHelper::createInstance

and that is rather an under-estimate since the profile also includes startup etc. =) and I imagine un-loading the module also costs time.

So ...

What would be -really- nice would be a way to timeout / expire such components (if we think they are worth un-loading) - of course, I expect UNO doesn't have a thread lying around just to unref components (at a later date) ;-) and who would want one ? but it also doesn't have an event loop we can plug a timer into easily (though we could add a hook to provide one I guess (?)).

I imagine doing this a few seconds after last use might work quite nicely.

Then again - the immediate horrible hack is to pretend a systemic problem doesn't exist, and go add some cache just for this single specific case to just writer ;-> but I'd like to avoid that if possible.

Thoughts ?
Comment 1 Michael Meeks 2014-05-09 09:31:04 UTC
Created attachment 98739 [details]
a photo of the issue =)
Comment 2 Stephan Bergmann 2014-05-09 10:19:17 UTC
UNO does not unload any component libraries.  I assume the calls to osl_loadModuleRelative you are observing are coming from code in i18npool, and that some UNO services provided by i18npool are either created afresh too often by client code (instead of keeping references around) or should rather be singletons instead of services.  But would be easier to tell when seeing backtraces for those calls to osl_loadModuleRelative.
Comment 3 Stephan Bergmann 2014-05-09 10:46:52 UTC
Indeed, wading through <http://users.freedesktop.org/~michael/callgrind-asian-entry-at-34ae7b16d.txt.gz>,

SwEditShell:Insert2 ->
SwEditShell:EndAllAction ->
SwCrsrShell::EndAction ->
SwViewShell::ImplEndAction ->
SwRootFrm::Paint ->
SwLayoutFrm::Paint ->
SwTxtFrm::Paint ->
SwTxtPainter::DrawTextLine ->
SwTxtPortion::Paint ->
SwTxtPaintInfo::_DrawText ->
SwSubFont::_DrawText ->
SwFntObj::DrawText ->
OutputDevice::GetTextArray ->
OutputDevice::ImplLayout ->
OutputDevice::ImplGlyphFallbackLayout ->
OutputDevice::getFallbackFont ->
ServerFontLayout::LayoutText ->
HbLayoutEngine::layout ->
ServerFontLayout::setNeedFallback ->
css::i18n::BreakIteratorImpl::previousCharacters ->
css::i18n::BreakIteratorImpl::getLocaleSpecificBreakIterator ->
css::i18n::BreakIteratorImpl::createLocaleSpecificBreakIterator ->
cppuhelper::ServiceManager::createInstanceWithContext ->
cppuhelper::ServiceManager::Data::Implementation::createInstance ->
cppu::OFactoryComponentHelper::createInstanceWithContext ->
cppu::OSingleFactoryHelper::createInstanceWithContext ->
cppu::OSingleFactoryHelper::createInstanceEveryTime ->
BreakIterator_zh_CreateInstance ->
BreakIterator_zh::BreakIterator_zh: new xdictionary("zh") ->
xdictionary::xdictionary: loadModuleRelative

looks plausible.
Comment 4 Michael Meeks 2014-05-09 10:50:37 UTC
> UNO does not unload any component libraries.  

Ah - fair enough; glad that's so.

> I assume the calls to osl_loadModuleRelative you are observing are coming
> from code in i18npool, and that some UNO services provided by i18npool are
> either created afresh too often by client code (instead of
> keeping references around)

That is the 'caller is at fault' one off argument :-)

> or should rather be singletons instead of services. But would be easier
> to tell when seeing backtraces for those calls to osl_loadModuleRelative.

The backtraces are in the callgrind file - but not in a pretty form - run in Kcachegrind and everything you need and more is there. My strong suspicion is that the UNO core could do better here - wrt. not going to osl_loadModuleRelative when we did that load / OS / symbol lookup just recently. Surely it is always possible to blame the caller for not working around performance issues underneath and there is a limit to how fast any one thing can be but ... ;-) In this case it seems that we could do better [ this is far from the first time I've seen performance problems of this sort ].

i18npool/source/breakiterator/breakiteratorImpl.cxx:sal_Int32 SAL_CALL BreakIteratorImpl::previousCharacters( const OUString& Text, sal_Int32 nStartPos,

is near the root of the trace.

#define LBI getLocaleSpecificBreakIterator(rLocale)
...
    return LBI->nextCharacters( Text, nStartPos, rLocale, nCharacterIteratorMode, nCount, nDone);

the cost being in 'getLocaleSpecificBreakIterator':

which already has some caching left/right: perhaps that caching doesn't work that well.

The issue here is when you get a complex document with lots of different characters next to each other, or in the same paragraph / string - then performance goes to pot.

Surely this is easier to fix in the infrastructure than by everyone having to wrap DIY caches around UNO service instantiation =)
Comment 5 Stephan Bergmann 2014-05-09 11:10:28 UTC
(In reply to comment #4)
> Surely this is easier to fix in the infrastructure than by everyone having
> to wrap DIY caches around UNO service instantiation =)

No.  There are legitimate cases for multiple instances of a single service.  The UNO infrastructure cannot help with poor uses of services.

So if it turns out that com.sun.star.i18n.BreakIterator_zh must remain a UNO service and cannot be changed to a UNO singleton, options to address this specific problem at the callee rather than caller side might be to go from cppu::createSingleFactory to cppu::createOneInstanceFactory (cf. i18npool/source/registerservices/registerservices.cxx) or to share xdictionary instancs across BreakIterator_zh instances.
Comment 6 Michael Meeks 2014-05-09 14:51:23 UTC
Hah :-) it seems that in fact that at least one big chunk of the cost is not UNO loading anything but some internal dlopen in the breakiterator:

xdictionary::xdictionary(const sal_Char *lang) :
...
    OUStringBuffer aBuf( strlen(lang) + 7 + 4 );    // mostly "*.dll" (with * == dict_zh)
#endif
    aBuf.appendAscii( "dict_" ).appendAscii( lang ).appendAscii( SAL_DLLEXTENSION );
    hModule = osl_loadModuleRelative( &thisModule, aBuf.makeStringAndClear().pData, SAL_LOADMODULE ...

etc. which looks like a more fruitful place to start :-)

Thanks !
Comment 7 Michael Meeks 2014-05-12 09:56:32 UTC
breakiterator fix pushed to master, should help.