Liviu Nicoara commented on STDCXX-1056:

I have attached stdcxx-1056-timings.tgz -- it contains an implementation of 
numpunct::grouping with a non caching alternative to the current 
implementation, t.cpp, a multi-threaded test program, s.cpp, a single-threaded 
test program, and a text file with timing results of test runs.

This goes towards the performance of the existing code. It neglects the 
correctness of the caching algorithm which, we now know, it's broken.

The results indicate that, in multi-threaded builds, the caching implementation 
is less performing than a non-caching implementation. IMO, this occurs because 
of the atomic ops synchronization that occurs upon the copying of the cached 
std::string member in the facet. The operation that synchronizes the 
std::string reference counting triggers a memory update, just like a cache 
miss, along with the invalidation of the relevant cache lines for all 
processors, etc. 

In the single-threaded builds, the caching version fares better, consistently. 
There is no locking in std::string, no contention, and grouping makes not trips 
in __rw_get_numpunct.

> std::moneypunct and std::numpunct implementations are not thread-safe
> ---------------------------------------------------------------------
>                 Key: STDCXX-1056
>                 URL: https://issues.apache.org/jira/browse/STDCXX-1056
>             Project: C++ Standard Library
>          Issue Type: Bug
>          Components: 22. Localization
>    Affects Versions: 4.2.1, 4.2.x, 4.3.x, 5.0.0
>         Environment: Solaris 10 and 11, RedHat and OpenSuSE Linux, Sun C++ 
> Compilers 12.1, 12.2, 12.3
> Issue is independent of platform and/or compiler.
>            Reporter: Stefan Teleman
>              Labels: thread-safety
>             Fix For: 4.2.x, 4.3.x, 5.0.0
>         Attachments: 22.locale.numpunct.mt.out, runtests-linux32-all.out, 
> runtests-linux64-all.out, runtests.out, stdcxx-1056.patch, 
> stdcxx-1056-timings.tgz
> several member functions in std::moneypunct<> and std::numpunct<> return
> a std::string by value (as required by the Standard). The implication of 
> return-by-value
> being that the caller "owns" the returned object.
> In the stdcxx implementation, the std::basic_string copy constructor uses a 
> shared
> underlying buffer implementation. This shared buffer creates the first 
> problem for
> these classes: although the std::string object returned by value *appears* to 
> be owned
> by the caller, it is, in fact, not.
> In a mult-threaded environment, this underlying shared buffer can be 
> subsequently modified by a different thread than the one who made the initial 
> call. Furthermore, two or more different threads can access the same shared 
> buffer at the same time, and modify it, resulting in undefined run-time 
> behavior.
> The cure for this defect has two parts:
> 1. the member functions in question must truly return a copy by avoiding a 
> call to the copy constructor, and using a constructor which creates a deep 
> copy of the std::string.
> 2. access to these member functions must be serialized, in order to guarantee 
> atomicity
> of the creation of the std::string being returned by value.
> Patch for 4.2.1 to follow.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to