[haiku-commits] Re: haiku: hrev47984 - src/kits/locale src/servers/mail src/kits/tracker headers/os/locale src/add-ons/mail_daemon/inbound_filters/notifier
- From: Adrien Destugues <pulkomandy@xxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Thu, 9 Oct 2014 12:40:19 +0200
On Thu, Oct 09, 2014 at 11:35:35AM +0200, Axel Dörfler wrote:
> Am 09/10/2014 11:17, schrieb Adrien Destugues:
> >It still needs locking, no matter how rare.
>
> Why would it need any locking?
Well, either locking or "only use in a single thread".
>
> >>If Tracker formats its date column a thousand times, it should not have to
> >>lock for that at all. That's the use case you have to optimize for.
> >It currently creates one BDateTimeFormat instance and locks it, for each
> >date it formats. So we are already at the worst case here.
> >
> >Moreover, BDateTimeFormat currently doesn't cache the ICU object used to
> >format dates, instead it is created on the fly each time the Format
> >method is called. So even if the BDateTimeFormat object was reused,
> >there wouldn't be much of a performance win.
>
> Well, that's exactly what I'm saying: this should be corrected.
Indeed, but that's a problem with BDateTimeFormat, not BMessageFormat.
>
> >In all the places where I started using it in Haiku, the format is used
> >to format a single string at a time. I made the BMessageFormat instances
> >static so they are shared between windows using the same format. This
> >makes it possible to translate and parse the format string only once in
> >application lifetime (or only at locale changes).
>
> Getting a translated string isn't exactly expensive. It's just a hash
> lookup.
Since BCatalog::GetString also has a lock, that's not true. This one is
definitely needed for B_LOCALE_CHANGED to work, as the catalog is
application-global and we can't assume a BLooper lock is enough to
safely access it (the catalog is immutable, and on locale changes it is
deleted and a new one loaded from disk).
The translation can still be done once, and BMessageFormat created
on-demand anyway. However I'm worried about the initialisation of the
BMessageFormat object not being a cheap operation, as it needs to parse
the format string, and create instances of PluralFormat, ChoiceFormat,
or anything else required by it. This involves several memory
allocations which I think are better to not repeat.
I can make the B*Format immutable, and remove the locking. But these
situations will still have to be handled on the application side anyway.
It only adds the "lazy" option of using the object only in a single
thread, which has some use cases (when formatting thousands of items).
--
Adrien.
Other related posts:
