On 9/24/12 11:06 PM, Stefan Teleman wrote:
On Mon, Sep 24, 2012 at 7:48 PM, Liviu Nicoara <nikko...@hates.ms> wrote:

Stefan, was it your intention to completely eliminate all the race
conditions with this last patch? Is this what the tools showed in your


Yes, all the race conditions in std::numpunct<> and std::moneypunct<>.
Not all the race conditions in stdcxx.

FWIW, I have analyzed the performance of the latest patch from Stefan. The test program is the same test program used in the timings attachments to the incident. The version of the library was 4.2.1, compiler gcc 4.7.1, with:

1. The simple, perfect forwarding patch, with no caching, which does eliminate race conditions.
2. Stefan's latest patch
3. A patch I created which truly, if crudely, eliminates all facet race conditions by using a top-level lock for all facet operations(*)

The numbers from the first test run are already available in the incident.

The numbers for the second test run were a bit bigger, which is expected because of the additional synchronization. E.g., I have found that the test, run against the STDCXX locale database, gives:

$ time ./t en_US.UTF-8 4 5000000
4, 5000000

real    0m19.210s
user    0m32.659s
sys     0m36.507s

where the first patch gives:

$ time ./t en_US.UTF-8 4 50000000; done
4, 50000000

real    0m7.961s
user    0m31.211s
sys     0m0.003s

Notice the number of loops, 10x larger in the second set of number.

Now, for the third patch, the numbers are embarrassingly large. It practically slows the program to a crawl because it does not do any reads of facet data without holding the lock. This, I wager, eliminates all races in the numpunct/moneypunct facet. See:

$ time ./t en_US.UTF-8 4 100000
4, 100000

real    0m24.316s
user    0m28.213s
sys     0m6.633s

Now that is a whooping performance hit.

I went back to see an explanation between the numbers I see with Stefan's more performing patch, and my lock-all patch. I believe that there are still places in Stefan's patch where the facet makes unguarded reads of facet data, e.g.:

    190     // returns a pointer to the facet's implementation data
    191     // if it exists, 0 otherwise
    192     const void* _C_data () const {
    193         return _C_impsize ? _C_impdata
    194             : _RWSTD_CONST_CAST (__rw_facet*, this)->_C_get_data ();
    195     }

where _C_impsize/_C_impdata are read outside of a lock scope, etc. I.e., a thread may read the _C_impsize while, say, another is still writing the _C_impdata member variable.

I conclude, even with such simple testing, that any solution that attempts to completely eliminate the race conditions in these facets will incur a penalty, with the safest (top-level locking) the worst by orders of magnitude and any other solution somewhere in between.

Please let me know if you have any questions.


(*) Will attach it as soon as we figure out how to undo the closing of the 

Reply via email to