From ae2c47f99c1d25e31a90dc91a0785865115feec0 Mon Sep 17 00:00:00 2001 From: Ryan Haining Date: Thu, 29 Aug 2013 01:22:58 -0700 Subject: [PATCH 1/6] Removes the need for the _Table_Locker class Replaces it with an anonymous struct type in the regular synchronized blocks. This seems desirable to me since it prevents creating an extra class that is exposed but really shouldn't be. --- synctable/synctable.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/synctable/synctable.h b/synctable/synctable.h index f97165a..bb7865b 100644 --- a/synctable/synctable.h +++ b/synctable/synctable.h @@ -6,11 +6,9 @@ namespace synclock{ class SyncTable{ - friend class _Table_Locker; private: std::unordered_map locks_table; std::mutex table_lock; - std::mutex * get_lock_address(void *addr); public: SyncTable(){} @@ -21,6 +19,7 @@ namespace synclock{ SyncTable & operator=(const SyncTable &) = delete; ~SyncTable(); + std::mutex * get_lock_address(void *addr); }; // This class is only for use by the synchronized/tablesynchronized blocks @@ -68,8 +67,12 @@ for(synclock::_Table_Locker _table_locker_obj_ABCDEFAOEUI(TABLE, (void*)(ADDR)); // It is also exception safe since destructon occurs when an exception // causes the block to exit #define synchronized(ADDR) \ -for(synclock::_Table_Locker _table_locker_obj_ABCDEFAOEUI(synclock::globalsynctable, static_cast(ADDR)); \ - !_table_locker_obj_ABCDEFAOEUI.finished; \ - _table_locker_obj_ABCDEFAOEUI.finished = true) +for(struct {const std::lock_guard & lg; bool finished;} pair = \ + {std::lock_guard( \ + *synclock::globalsynctable.get_lock_address( \ + static_cast(ADDR))), \ + false}; \ + !pair.finished; \ + pair.finished = true) #endif // __SYNCHRONIZER__H__ From f2400e1d3837f43181ee438a3c27f24c893c142d Mon Sep 17 00:00:00 2001 From: Ryan Haining Date: Thu, 29 Aug 2013 14:45:18 -0700 Subject: [PATCH 2/6] Removes _Table_Locker entirely The _Table_Locker object is replaced with an anonymous struct type created in the foor loop. It's definitely uglier but it removes the need to expose an additional class, something I never wanted to do in the first place. --- synctable/synctable.cpp | 15 ------------- synctable/synctable.h | 47 ++++++++++++++++++----------------------- 2 files changed, 21 insertions(+), 41 deletions(-) diff --git a/synctable/synctable.cpp b/synctable/synctable.cpp index dfcbec1..d270c41 100644 --- a/synctable/synctable.cpp +++ b/synctable/synctable.cpp @@ -1,9 +1,6 @@ #include "synctable.h" namespace synclock { - - // SyncTable - SyncTable::~SyncTable() { for (auto iter = this->locks_table.begin(), @@ -31,18 +28,6 @@ namespace synclock { return var_lock; } - - // _Table_Locker - - _Table_Locker::_Table_Locker(SyncTable & sync_table, void * addr) - : var_lock_holder(*sync_table.get_lock_address(addr)), - finished(false) - { } - - - _Table_Locker::~_Table_Locker() { } - - // The Global SyncTable for synchronized() blocks SyncTable globalsynctable; } diff --git a/synctable/synctable.h b/synctable/synctable.h index bb7865b..981551a 100644 --- a/synctable/synctable.h +++ b/synctable/synctable.h @@ -22,27 +22,10 @@ namespace synclock{ std::mutex * get_lock_address(void *addr); }; - // This class is only for use by the synchronized/tablesynchronized blocks - // and should not be used directly. The name of the class is - // intentionlly poorly formed. - class _Table_Locker{ - private: - std::lock_guard var_lock_holder; - - public: - bool finished; - _Table_Locker(SyncTable &sync_table, void * addr); - _Table_Locker(const _Table_Locker &) = delete; - _Table_Locker & operator=(const _Table_Locker &) = delete; - ~_Table_Locker(); - }; - // global table for use in synchronized blocks extern SyncTable globalsynctable; } -// the _Table_Lockers have a bunch of capital letters on the end of them -// to (try to) ensure there are no collisions // tablesynchronized(synctable, &var) { critical section } // @@ -51,27 +34,39 @@ namespace synclock{ // this is provided so that groups of unrelated threads do not result in a // large, slow, globalsynctable. // using a value in a local SyncTable will NOT add it to tho global synctable -// this is exception safe since the _Table_Locker releases the lock on +// this is exception safe since the lock_guard releases the lock on // destruction #define tablesynchronized(TABLE, ADDR) \ -for(synclock::_Table_Locker _table_locker_obj_ABCDEFAOEUI(TABLE, (void*)(ADDR)); \ - !_table_locker_obj_ABCDEFAOEUI.finished; \ - _table_locker_obj_ABCDEFAOEUI.finished = true) +for(struct { \ + const std::lock_guard & lg; \ + bool finished; \ + } pair = \ + { \ + std::lock_guard( \ + *TABLE.get_lock_address( \ + static_cast(ADDR))), \ + false }; \ + !pair.finished; \ + pair.finished = true) // synchronized(&var) { critical section } // -// synchronized blocks construct a _Table_Locker on entry and destroy it +// synchronized blocks construct a lock_guard on entry and destroy it // on exit. This results in a locking of var for the body of the block. -// It is also exception safe since destructon occurs when an exception +// It is also exception safe since destruction occurs when an exception // causes the block to exit #define synchronized(ADDR) \ -for(struct {const std::lock_guard & lg; bool finished;} pair = \ - {std::lock_guard( \ +for(struct { \ + const std::lock_guard & lg; \ + bool finished; \ + } pair = \ + { \ + std::lock_guard( \ *synclock::globalsynctable.get_lock_address( \ static_cast(ADDR))), \ - false}; \ + false }; \ !pair.finished; \ pair.finished = true) From 8218b8665772740cf11844b83a8182d98f11edfa Mon Sep 17 00:00:00 2001 From: Ryan Haining Date: Wed, 11 Sep 2013 13:36:33 -0700 Subject: [PATCH 3/6] Removes anonymous struct, replaces with std::pair The previous anonymous struct has a const reference to a temporary. I realized this was unsafe (though it did, unfortunately work). Using std::piecewise_construct and a pair, the _Table_Locker class can be eliminated safely. It's not very pretty but I can finally eliminate that class entirely. --- synctable/synctable.h | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/synctable/synctable.h b/synctable/synctable.h index 981551a..f745092 100644 --- a/synctable/synctable.h +++ b/synctable/synctable.h @@ -38,17 +38,14 @@ namespace synclock{ // destruction #define tablesynchronized(TABLE, ADDR) \ -for(struct { \ - const std::lock_guard & lg; \ - bool finished; \ - } pair = \ - { \ - std::lock_guard( \ - *TABLE.get_lock_address( \ - static_cast(ADDR))), \ - false }; \ - !pair.finished; \ - pair.finished = true) +for(std::pair, bool> pairwithREALLYlongname( \ + std::piecewise_construct, \ + std::make_tuple(std::ref( \ + *TABLE.get_lock_address( \ + static_cast(ADDR)))), \ + std::make_tuple(false)); \ + !pairwithREALLYlongname.second; \ + pairwithREALLYlongname.second = true) // synchronized(&var) { critical section } @@ -58,16 +55,13 @@ for(struct { \ // It is also exception safe since destruction occurs when an exception // causes the block to exit #define synchronized(ADDR) \ -for(struct { \ - const std::lock_guard & lg; \ - bool finished; \ - } pair = \ - { \ - std::lock_guard( \ - *synclock::globalsynctable.get_lock_address( \ - static_cast(ADDR))), \ - false }; \ - !pair.finished; \ - pair.finished = true) + for(std::pair, bool> pairwithREALLYlongname( \ + std::piecewise_construct, \ + std::make_tuple(std::ref( \ + *synclock::globalsynctable.get_lock_address( \ + static_cast(ADDR)))), \ + std::make_tuple(false)); \ + !pairwithREALLYlongname.second; \ + pairwithREALLYlongname.second = true) #endif // __SYNCHRONIZER__H__ From 0f19e7fd4b48c21ee769d7bca8a33c998d47a635 Mon Sep 17 00:00:00 2001 From: Ryan Haining Date: Sat, 14 Sep 2013 12:29:18 -0700 Subject: [PATCH 4/6] Adds #include Coincidentally std::ref worked because something else was including . --- synctable/synctable.h | 1 + 1 file changed, 1 insertion(+) diff --git a/synctable/synctable.h b/synctable/synctable.h index f745092..622679f 100644 --- a/synctable/synctable.h +++ b/synctable/synctable.h @@ -3,6 +3,7 @@ #include #include +#include namespace synclock{ class SyncTable{ From 6365cecfe0baae09d4690f41124eaf57986033ad Mon Sep 17 00:00:00 2001 From: Ryan Haining Date: Thu, 26 Sep 2013 16:28:57 -0700 Subject: [PATCH 5/6] Replaces pair/piecewise_construct with anon class Using c++11 inheriting constructors the pair can be eliminated and the TableLocker alternative is defined inline. Inherits the lock_guard constructor and has a bool finished with a default of false. afaik this behavior is all defined, there is no pair, just an anonymous class (finally). --- synctable/synctable.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/synctable/synctable.h b/synctable/synctable.h index 622679f..7e9587b 100644 --- a/synctable/synctable.h +++ b/synctable/synctable.h @@ -56,13 +56,13 @@ for(std::pair, bool> pairwithREALLYlongname( \ // It is also exception safe since destruction occurs when an exception // causes the block to exit #define synchronized(ADDR) \ - for(std::pair, bool> pairwithREALLYlongname( \ - std::piecewise_construct, \ - std::make_tuple(std::ref( \ - *synclock::globalsynctable.get_lock_address( \ - static_cast(ADDR)))), \ - std::make_tuple(false)); \ - !pairwithREALLYlongname.second; \ - pairwithREALLYlongname.second = true) + for (class : public std::lock_guard { \ + public: \ + bool finished = false; \ + using std::lock_guard::lock_guard; \ + } lg(*synclock::globalsynctable.get_lock_address( \ + static_cast(ADDR))); \ + !lg.finished; \ + lg.finished = true) #endif // __SYNCHRONIZER__H__ From 5545fbb0fccde34d46bcbebb3ed1ba55862693e9 Mon Sep 17 00:00:00 2001 From: Ryan Haining Date: Thu, 26 Sep 2013 16:32:02 -0700 Subject: [PATCH 6/6] Manges the name of the locker Instead of lg, it's now lgwithREALLYlongname to avoid collisions. If I was writing for the standard I'd just prefix with an underscore, but I'm not implementing stdlib facilities so this seems for the best. --- synctable/synctable.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synctable/synctable.h b/synctable/synctable.h index 7e9587b..c412103 100644 --- a/synctable/synctable.h +++ b/synctable/synctable.h @@ -60,9 +60,9 @@ for(std::pair, bool> pairwithREALLYlongname( \ public: \ bool finished = false; \ using std::lock_guard::lock_guard; \ - } lg(*synclock::globalsynctable.get_lock_address( \ + } lgwithREALLYlongname(*synclock::globalsynctable.get_lock_address( \ static_cast(ADDR))); \ - !lg.finished; \ - lg.finished = true) + !lgwithREALLYlongname.finished; \ + lgwithREALLYlongname.finished = true) #endif // __SYNCHRONIZER__H__