messages<charT>::do_get() inefficiency requires unnecessary locking

• Key: STDCXX-907
• Type: Bug
• Status: Open
• Priority: Minor
• Assignee: Unassigned
• Reporter: Travis Vitek
• Component/s: 22. Localization
• Affects Version/s: 4.2.1
• Fix Version/s: 4.3.0

Description

The _STD::messages<T>::do_get() method might actually call __rw_manage_cat_data() up to three times to access the cache. Since each access involves a mutex lock, there are going to be some wasted cycles. It would be nice to reduce this to one access. Perhaps the _RW::__rw_get_message() function could be changed to fill in a pointer to the _STD::locale that is kept in the cache and the _RW::__rw_get_locale() function could be removed.

For backward binary compatibility we would need to keep the existing functions around, but we could add an overload and then deprecate the old ones using the _RWSTD_VER macro.

Comments

[Travis Vitek]

The proposed patch assumes that the patch for STDCXX-845 was applied. It should still work without that patch, but I haven't tested it.

2008-05-02  Travis Vitek  <[e-mail]>

	STDCXX-845
	* src/messages.cpp (__rw_get_message): Add overload which takes a
	pointer reference, allowing us to get the message and the locale
	in a single function call to avoid some unnecessary locking.
	(__rw_get_message, __rw_get_locale) [_RWSTD_VER]: Conditionally
	deprecate old functions to be sure they are removed in 5.0

[Martin Sebor]

I'm wondering if __rw_get_message() is actually exposed in the library's ABI given that the facet template has only two possible specializations and they are both instantiated in the library. If not, we could make the change without keeping the old signature around.

Btw., I'm not sure using the version macro this way is the best approach for the library (then again, I'm not sure it isn't, either). Would adding an #else block along these lines be a better way of guaranteeing that the code does in fact get removed?

#else
#  error "remove me!"
#endif

As for the signature of the new overload, FWIW, my personal preference is for the convention where objects are passed by reference as in parameters and by their address as out parameters. That way it's usually more clear at the call site that the object will be modified. I.e.,

@@ -61,16 +61,20 @@
                           int                    __msgid,
                           const string_type     &__dfault) const
 {
-    const char* const __text = _RW::__rw_get_message (__cat, __set, __msgid);
- 
+    const _STD::locale* __ploc;   // will be set in __rw_get_message()
+
+    const char* const __text = _RW::__rw_get_message (__cat, __set, __msgid, &__ploc);
     if (__text) {