[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 13:17:31 +0200

On Thu, Oct 09, 2014 at 12:54:55PM +0200, Stephan Aßmus wrote:
> Hi,
> 
> Am 09.10.2014 12:40, schrieb Adrien Destugues:
> >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".
> 
> It is perfectly normal that various Be API objects are not thread-safe.
> BString, BList, etc. are all not threadsafe. I think most of them are not.
> But that doesn't mean you can't use them from multiple threads, you just
> have to take care of locking yourself - *if* that is your use-case at all.
> As Axel explained, the normal use-case would not be to use these objects
> from different threads (in which case thread-safety would be a convenience).
> And then the performance impact of the unnecessary locking in the normal
> use-case is really not what you want.

That's what I'm debatting. The current use case in our sources for
BMessageFormat is to use statically allocated instances shared between
many threads. The reason for this is the creation of the object involves
a lot of operations (parsing the string, creating the ICU formatter
instances) and thus takes some time.

If we decide that this is not the normal use case, we can use
"throw-away" objects and we won't need the locking. But I think that
will result in overall slower performance.

Right now the normal use case for BMessageFormat is to share it between
several similar windows, which each use it once or rarely.

> 
> >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).
> 
> This sounds really broken. The application should be notified about the
> locale change (as is the case), but then it needs to be up to the
> application to act on the change, including the triggering of re-loading the
> catalog. 

This is what happens. But you can't easily "freeze" the whole
application and make sure no one calls B_TRANSLATE while the catalog is
being reloaded. For this reason, you need a lock. And in order to keep
B_TRANSLATE easy to use, the lock is "hidden" in BCatalog.

> What good is it if a string changes underneath the app, but the app
> has not updated (the rest of) the UI? In the best scenario, you get a mix of
> different languages in the UI, in the worst case, you have cut-off labels.

It doesn't change underneath the app, the app triggers the reloading of
the catalog. But the lock is needed so the reloading does not crash the
app because (for example) some window decided to update a label at the
same moment.

The use of BMessageFormat as a static class may be a way to avoid this
problem. Instead of repeated calls to B_TRANSLATE every time a value
changes (typically B_TRANSLATE("%num objects")), with BMessageFormat you
only need a single B_TRANSLATE call when creating the BMessageFormat
object, then you don't need access to the BCatalog again, unless the
locale changes. (you can achieve something similar by getting all your
strings once and storing them in an array for later access).

With that scheme, calls to B_TRANSLATE would happen in less occasions
(when creating an application or window, and when changing the locale
after receiving a B_LOCALE_CHANGED). This may make it possible, with
some care, to have a more efficient locking scheme handled on the
application side.

-- 
Adrien.

Other related posts: