[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdelibs/kdecore
From:       John Layt <john () layt ! net>
Date:       2010-12-15 11:57:59
Message-ID: 201012151157.59847.john () layt ! net
[Download RAW message or body]

On Tuesday 14 December 2010 21:26:50 Frank Osterfeld wrote:
> 
> The setLanguage() you added in KDateTimeFormatter::formatDateTimePosix():
> 
> kdatetimeformatter.cpp:133:
>     KLocale *englishLocale = new KLocale( *locale );
>     englishLocale->setLanguage( QStringList() <<
> QString::fromLatin1("en_US") ); KCalendarSystem *englishCalendar =
> KCalendarSystem::create( calendar->calendarType(), englishLocale );
> 
> is insanely expensive. 539 million instructions are spent formatting 297
> dates. That's because setLanguage() calls updateCatalogs(), which does all
> kinds of stuff with the filesystem, (KStandardDirs::locate being the main
> culprit). Makes my Kontact freeze on startup when processing mails. See my
> attached callgrind dump (I accidentally had a kdelibs-trunk named as
> kdelibs-4.5, don't get confused by the paths there; my actual setup (by
> accident) is kdelibs trunk +  kdepim 4.4).
> 
> Isn't there a way to avoid all any per-date-to-format-filesystem access?
> Making that KLocale a member of KDateTimeFormatter doesn't help
> unfortunately, as KDateTimeFormatter is also created for each formatting
> operation. Maybe the formatter can be made a member of the calendarsystem
> or something?

Ouch :-(  Thanks for finding that Frank.

The reason for KDateTimeFormatter is to have a single implementation of the 
date/time formatting code shared between KCalendarSystem, KLocale and 
KDateTime rather than the 3 separate and inconsistent implementations we 
currently have, but it's only used in KCalendarSystem for dates so far as the 
time code hasn't been tested enough.  

The English names feature comes from KDateTime, but there David uses static 
variables for the names instead of a locale and I'm now guessing efficiency 
issues like this is why.  That's OK for KDateTime as it only supports 
Gregorian calendar, but for KCalendarSystem use I'd have to copy every 
calendar system's names which would be a lot of duplication, but I may have no 
other choice.  

For now I've hopefully fixed it in r1206679 by making the locale a member and 
delaying the creation until it is actually used (i.e. never in the normal use 
case), but I don't think that will be efficient enough for KDateTime's 
purposes, and I'm also thinking of making the function calls static, so I'll 
have a think about the alternatives.

Thanks,

John.
[prev in list] [next in list] [prev in thread] [next in thread]