Skip to content

Commit eea8d27

Browse files
trueqbittrueqbit
committed
Updated retaining connections after testing contention
1 parent 289abe2 commit eea8d27

File tree

2 files changed

+164
-152
lines changed

2 files changed

+164
-152
lines changed

‎dev/connection_holder.h‎

Lines changed: 82 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -50,37 +50,6 @@ namespace sqlite_orm {
5050
explicit connection_holder(const connection_holder& other, std::true_type /*openedForeverHint*/) :
5151
_control{true}, dbArgs{other.dbArgs}, _didOpenDb{other._didOpenDb} {}
5252

53-
/*
54-
Open from a single-threaded context.
55-
*/
56-
void _do_open() {
57-
int openFlags = db_open_mode_to_int_flags(this->dbArgs.open_mode);
58-
#if SQLITE_VERSION_NUMBER >= 3037002
59-
openFlags |= SQLITE_OPEN_EXRESCODE;
60-
#endif
61-
62-
const int rc = sqlite3_open_v2(this->dbArgs.filename.c_str(),
63-
&_control.db,
64-
openFlags,
65-
this->dbArgs.vfs_name.c_str());
66-
67-
if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ {
68-
throw_translated_sqlite_error(rc);
69-
}
70-
}
71-
72-
/*
73-
Close from a single-threaded context.
74-
*/
75-
void _do_close() {
76-
const int rc = sqlite3_close_v2(_control.db);
77-
if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY {
78-
throw_translated_sqlite_error(_control.db);
79-
} else {
80-
_control.db = nullptr;
81-
}
82-
}
83-
8453
/*
8554
Open the database once and for all from a single-threaded context when it should be opened permanently.
8655
*/
@@ -109,47 +78,19 @@ namespace sqlite_orm {
10978
_do_close();
11079
}
11180

81+
/*
82+
Retain the database handle if the database is already open, `nullptr` otherwise.
83+
*/
11284
sqlite3* retain_if_open() {
113-
// optional marginal optimization for permanently opened connections;
114-
if (_control.openedForeverHint) {
115-
#ifdef SQLITE_ORM_CONTRACTS_SUPPORTED
116-
contract_assert(_control.db);
117-
#endif
118-
return _control.db;
119-
}
120-
121-
// required fast path: if connection is already open, just increment counter;
122-
// it is required otherwise 'retain if open' would be useless;
123-
// with respect to performance,
124-
// this can make a difference while a transaction is active where all things happen in memory only;
125-
// it makes a difference if the `_didOpenDb` callback has a lot of work to do.
126-
if (int currentCount = _control.retainCount.load(std::memory_order_acquire)) {
127-
do {
128-
if (_control.retainCount.compare_exchange_weak(currentCount,
129-
currentCount + 1,
130-
std::memory_order_release,
131-
std::memory_order_acquire)) {
132-
// successfully incremented, connection is guaranteed to be open
133-
return _control.db;
134-
}
135-
// CAS failed - retry
136-
} while (currentCount > 0);
137-
}
138-
// test for recursion from the same thread
139-
else /*currentCount==0*/ {
140-
const std::thread::id threadId = _control.initializingThreadId.load(std::memory_order_acquire);
141-
if (threadId != std::thread::id{} && std::this_thread::get_id() == threadId)
142-
SQLITE_ORM_CPP_UNLIKELY {
143-
return _control.db;
144-
}
145-
}
146-
147-
return nullptr;
85+
return _try_retain_if_open(false);
14886
}
14987

88+
/*
89+
Retain the database handle if the database is already open, otherwise open the database.
90+
*/
15091
sqlite3* retain() {
151-
// optional fast path: if connection is already open, just increment counter;
152-
if (sqlite3* db = retain_if_open()) {
92+
// optional fast path: if connection is already open, just try incrementing the counter;
93+
if (sqlite3* db = _try_retain_if_open(true)) {
15394
return db;
15495
}
15596

@@ -177,7 +118,7 @@ namespace sqlite_orm {
177118
}
178119

179120
void release() {
180-
// optional marginal optimization for permanently opened connections;
121+
// optional optimization for permanently opened connections;
181122
if (_control.openedForeverHint) {
182123
#ifdef SQLITE_ORM_CONTRACTS_SUPPORTED
183124
contract_assert(_control.db);
@@ -186,7 +127,7 @@ namespace sqlite_orm {
186127
}
187128

188129
// test for recursion from the same thread;
189-
// testing against an empty thread id is sufficient because recursion is only possible while calling the `_didOpenDb` callback in `retain()`
130+
// testing against an empty thread id is sufficient because recursion is only possible while the `_didOpenDb` callback is executing in `retain()`
190131
if (_control.initializingThreadId.load(std::memory_order_acquire) != std::thread::id{})
191132
SQLITE_ORM_CPP_UNLIKELY {
192133
return;
@@ -205,18 +146,83 @@ namespace sqlite_orm {
205146
}
206147
}
207148

149+
/*
150+
Open from a single-threaded context.
151+
*/
152+
void _do_open() {
153+
int openFlags = db_open_mode_to_int_flags(this->dbArgs.open_mode);
154+
#if SQLITE_VERSION_NUMBER >= 3037002
155+
openFlags |= SQLITE_OPEN_EXRESCODE;
156+
#endif
157+
158+
const int rc = sqlite3_open_v2(this->dbArgs.filename.c_str(),
159+
&_control.db,
160+
openFlags,
161+
this->dbArgs.vfs_name.c_str());
162+
163+
if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ {
164+
throw_translated_sqlite_error(rc);
165+
}
166+
}
167+
168+
/*
169+
Close from a single-threaded context.
170+
*/
171+
void _do_close() {
172+
const int rc = sqlite3_close_v2(_control.db);
173+
if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY {
174+
throw_translated_sqlite_error(_control.db);
175+
} else {
176+
_control.db = nullptr;
177+
}
178+
}
179+
180+
sqlite3* _try_retain_if_open(const bool yieldIfContended) {
181+
// optional optimization for permanently opened connections;
182+
if (_control.openedForeverHint) {
183+
#ifdef SQLITE_ORM_CONTRACTS_SUPPORTED
184+
contract_assert(_control.db);
185+
#endif
186+
return _control.db;
187+
}
188+
189+
if (int currentCount = _control.retainCount.load(std::memory_order_relaxed)) {
190+
do {
191+
if (_control.retainCount.compare_exchange_weak(currentCount,
192+
currentCount + 1,
193+
std::memory_order_release,
194+
std::memory_order_acquire)) {
195+
// successfully incremented, connection is guaranteed to be open
196+
return _control.db;
197+
}
198+
// CAS failed due to contention;
199+
// 1. !yieldIfContended (compete) : retry until successful or count reaches zero because another thread closed the database;
200+
// 2. yieldIfContended: do not try again; it does not have to be lock-free at all costs, which avoids CPUs competing for incrementation.
201+
} while (!yieldIfContended && currentCount > 0);
202+
}
203+
// test for recursion from the same thread
204+
else /*currentCount == 0*/ {
205+
const std::thread::id threadId = _control.initializingThreadId.load(std::memory_order_acquire);
206+
if (threadId != std::thread::id{} && std::this_thread::get_id() == threadId)
207+
SQLITE_ORM_CPP_UNLIKELY {
208+
return _control.db;
209+
}
210+
}
211+
212+
return nullptr;
213+
}
214+
208215
// note: members of the `control_block` are deliberately put on the same cache-line
209216
SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(alignas(polyfill::hardware_destructive_interference_size))
210217
struct control_block {
211-
// the optimization gain is very small;
212-
// at some design point it served as a flag to not use a mutex at all;
213-
// now it merely saves all the atomic operations, which actually perform without noticeable difference;
214-
// however it may be kept for conveying logic or future optimizations.
215-
const bool openedForeverHint = false;
218+
// Optional optimization hint that also serves to convey logic.
219+
// in a test scenario involving a tight retain()/releae() loop from multiple threads the performance gain is outstanding;
220+
// in a real-world scenario it merely saves all the atomic operations and the CPU cache updates they entail;
221+
bool openedForeverHint = false;
216222
std::atomic_int retainCount{};
217223
// `db` synchronizes with `retainCount`
218224
orm_gsl::owner<sqlite3*> db = nullptr;
219-
// we don't know what the user-provided `on_open` callback might do, so we need to track recursion;
225+
// we don't know what the user-provided `on_open` callback might do, so we need to track recursion during the `_didOpenDb` callback;
220226
std::atomic<std::thread::id> initializingThreadId{};
221227
} _control;
222228

0 commit comments

Comments
 (0)