Switched sync.{cpp,h} to std threading primitives. #11722

1. 
   tjps commented at 7:38 pm on November 18, 2017: contributor

   Replaced boost threading primitives with the std equivalents.
2. Switched sync.{cpp,h} to std threading primitives. bba9bd0d9d
3. 
   sipa commented at 8:06 pm on November 18, 2017: member

   This uses thread_local, a feature that we chose to not use before, though I don’t remember why. Perhaps because we needed compatibility with GCC 4.7? Is that still the case, @laanwj @theuni
4. fanquake added the label Refactoring on Nov 18, 2017
5. fanquake added this to the "In progress" column in a project

   ---

6. 
   laanwj commented at 10:17 am on November 19, 2017: member

   This uses thread_local, a feature that we chose to not use before, though I don’t remember why.

   Given that it’s only used when lock debugging is used I’d say it’s acceptable.

   Also FWIW I tried to build bitcoin on g++ 4.7 a while ago and it was a pain; for example the AX_CXX_COMPILE_STDCXX fails because the __cplusplus macro is not set correctly, and that’s only the beginning. As it’s already effectively broken and no one else seems to even notice it, I wouldn’t mind bumping the requirement to g++ 4.8 wholesale. But I’m not sure that discussion belongs here.

7. laanwj requested review from theuni on Nov 19, 2017
8. in src/sync.cpp:7 in bba9bd0d9d outdated

   3@@ -4,13 +4,12 @@
   4 
   5 #include <sync.h>
   6 
   7+#include <set>
   

   ---

   ---

   

   promag commented at 12:42 pm on November 19, 2017:

   Why?

   ---

   

   tjps commented at 5:28 pm on November 19, 2017:

   Amazingly enough the #include tree in sync.cpp did not pull in std::set already.
9. 
   promag commented at 12:43 pm on November 19, 2017: member

   Concept ACK.
10. 
    practicalswift commented at 1:02 pm on November 20, 2017: contributor

    Concept ACK
11. 
    tjps commented at 10:26 am on November 27, 2017: contributor

    Since #11732 was resolved in favor of bumping GCC requirement to 4.8, looks like this PR is good to go?
12. 
    sipa commented at 10:35 am on November 27, 2017: member

    utACK bba9bd0d9dd06f13a6b0c89181864453cab5c858
13. 
    theuni commented at 9:43 pm on November 27, 2017: member

    thread_local was also off-limits due to missing support in osx’s clang. It’s been added as of XCode 8 on September 13, 2016. I’m uneasy with that as a minimum requirement.

    Additionally, last time I checked this, the use of thread_local causes new symbols to be pulled in from glibc on Linux, meaning that we would lose back-compat with older versions. I’d have to re-test to see where the break lies.

    That said, I think it’s ok to lessen the requirement for the DEBUG_LOCKCONTENTION case, as long as there’s a comment there mentioning that it’s problematic for other code.

    As osx is probably not the only platform where this will cause headaches, I’d also prefer to add an autoconf link test for thread_local.

14. 
    TheBlueMatt commented at 9:54 pm on November 27, 2017: member

    utACK bba9bd0d9dd06f13a6b0c89181864453cab5c858 modulo thread_local concerns. I also think its probably OK in DEBUG_LOCKCONTENTION only even if its not on all supported platforms.
15. 
    theuni commented at 10:14 pm on November 27, 2017: member

    Pushed up a commit on top of this PR that adds the configure check for thread_local: https://github.com/theuni/bitcoin/commit/d6df1162d3cba1aabae4f5ae61da6a81881ea118 . I’ve verified that it works as intended for me in Linux, and breaks as intended on osx. @tjps assuming that works for you, mind cherry-picking it here?
16. 
    tjps commented at 10:30 pm on November 27, 2017: contributor

    That looks great to me, will cherry-pick it onto this PR.
17. threads: add a thread_local autoconf check f7f7e2cd34
18. theuni approved
19. 
    theuni commented at 0:22 am on November 28, 2017: member

    Thanks! utACK f7f7e2cd340c088e82d09090eb275b98b34a9812.
20. laanwj merged this on Nov 28, 2017
21. laanwj closed this on Nov 28, 2017

    ---

22. laanwj referenced this in commit 26efc220a1 on Nov 28, 2017
23. fanquake moved this from the "In progress" to the "Done" column in a project

    ---

24. tjps deleted the branch on Dec 18, 2017
25. jamesob referenced this in commit 2ed61a42f2 on May 21, 2018
26. jamesob referenced this in commit 3e1de02c25 on Jun 4, 2018
27. jamesob referenced this in commit 789758c500 on Jun 13, 2018
28. jamesob referenced this in commit b4c026b739 on Jun 13, 2018
29. jamesob referenced this in commit cdada4f684 on Jun 14, 2018
30. jamesob referenced this in commit bf2cb44e5b on Jun 14, 2018
31. jamesob referenced this in commit 4c21846ec3 on Jun 14, 2018
32. jamesob referenced this in commit 696bdb7510 on Jun 15, 2018
33. jamesob referenced this in commit 6f854f68bc on Jun 20, 2018
34. jamesob referenced this in commit 99d6a1e865 on Jul 11, 2018
35. jamesob referenced this in commit 558f3235ee on Aug 20, 2018
36. jamesob referenced this in commit 315d84314a on Jan 17, 2019
37. jamesob referenced this in commit 7550fc1bcc on Apr 18, 2019
38. jamesob referenced this in commit c1d159999c on Apr 18, 2019
39. jamesob referenced this in commit 188ca75e5f on Apr 26, 2019
40. sidhujag referenced this in commit 500e70c86d on May 1, 2019
41. HashUnlimited referenced this in commit ddcb2ca45d on Aug 30, 2019
42. barrystyle referenced this in commit e98602b60d on Nov 11, 2019
43. PastaPastaPasta referenced this in commit 605c9c02eb on Feb 27, 2020
44. PastaPastaPasta referenced this in commit 3d67b17dd2 on Mar 14, 2020
45. PastaPastaPasta referenced this in commit e6a7b6a93d on Mar 14, 2020
46. PastaPastaPasta referenced this in commit 6c7d1e4dd0 on Mar 19, 2020
47. PastaPastaPasta referenced this in commit 1e41cf9f3d on Mar 24, 2020
48. ckti referenced this in commit 0c3807fba4 on Mar 28, 2021
49. gades referenced this in commit 5f6adc1fb6 on Jun 28, 2021
50. DrahtBot locked this on Sep 8, 2021

Contributors
tjps sipa laanwj promag practicalswift theuni TheBlueMatt

Labels
Refactoring