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.