Bug 635708
Summary: | Huge performance problem with libstdc++ and multithread applications | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | Red Hat Enterprise Linux 5 | Reporter: | Alan Matsuoka <alanm> | ||||||||||||
Component: | gcc | Assignee: | Jakub Jelinek <jakub> | ||||||||||||
Status: | CLOSED ERRATA | QA Contact: | qe-baseos-tools-bugs | ||||||||||||
Severity: | medium | Docs Contact: | |||||||||||||
Priority: | medium | ||||||||||||||
Version: | 5.5 | CC: | bkoz, mnowak, pmuller, tao | ||||||||||||
Target Milestone: | rc | ||||||||||||||
Target Release: | --- | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | Linux | ||||||||||||||
Whiteboard: | |||||||||||||||
Fixed In Version: | gcc-4.1.2-49.el5 | Doc Type: | Bug Fix | ||||||||||||
Doc Text: | Story Points: | --- | |||||||||||||
Clone Of: | Environment: | ||||||||||||||
Last Closed: | 2011-01-13 23:58:33 UTC | Type: | --- | ||||||||||||
Regression: | --- | Mount Type: | --- | ||||||||||||
Documentation: | --- | CRM: | |||||||||||||
Verified Versions: | Category: | --- | |||||||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||||||
Embargoed: | |||||||||||||||
Attachments: |
|
Description
Alan Matsuoka
2010-09-20 14:54:55 UTC
Created attachment 448478 [details]
r155342.patch
Created attachment 448479 [details]
r155342-core-backport.patch
Created attachment 448480 [details]
reproducer.cc
Created attachment 448481 [details]
vtune1.png
Created attachment 448482 [details]
vtune2.png
Adjusted pri to high to reflect Issue tracker priorities. Double-checking with Benjamin, but this looks ok to me for backport. Seems the condition_variable and testsuite hunks in the commit are completely unrelated though and shouldn't be put in. yes, this should be done for the gcc-4.4 branch. gcc-4.5 and above already have the patch (as expected). -benjamin The bugreport is about gcc-4.1-RH actually, of course if it is applied there it will have to be applied to gcc-4.4-RH too. Actually, looking at the patch, it doesn't seem to be 100% safe, the compiler could (not very likely, but possible) decide to read _S_global outside of the critical section more than once if the register pressure was too high. _M_impl = _S_global; if (_M_impl == _S_classic) _M_impl->_M_add_reference(); else { __gnu_cxx::__scoped_lock sentry(get_locale_mutex()); _S_global->_M_add_reference(); _M_impl = _S_global; } I think there is a (minor) risk that this will do: _M_impl = _S_global; if (_M_impl == _S_classic) { _M_impl = _S_global; _M_impl->_M_add_reference(); } else { __gnu_cxx::__scoped_lock sentry(get_locale_mutex()); _S_global->_M_add_reference(); _M_impl = _S_global; } and that would be thread unsafe. Of course this is in libstdc++.so, so it can be analyzed for all targets. The other option is to use atomic read, in glibc we have a macro for such thing. I guess something like __asm ("" : "=r" (_M_impl) : "0" (_S_global)); instead of _M_impl = _S_global; (the first occurrence, the second one is of course fine, as it is guarded by lock). This ensures reload uses the same value read from _S_global for the comparison, for _M_add_reference () as well as _M_impl member of the class. In gcc-4.1.2-49.el5 (In reply to comment #10) > Actually, looking at the patch, it doesn't seem to be 100% safe, the compiler > could (not very likely, but possible) decide to read _S_global outside of the > critical section more than once if the register pressure was too high. Actually it seems the patch we have in CVS do not differ to what is in in this bug at all. Is that expected? Try raising the number of threads to 32 or 64 and see if that works. An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on therefore solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHSA-2011-0025.html |