Change in ...osmo-hlr[master]: src/db.c: integrate SQLite3 with talloc allocator
Vadim Yanitskiy has submitted this change and it was merged. (
https://gerrit.osmocom.org/c/osmo-hlr/+/14939 )
Change subject: src/db.c: integrate SQLite3 with talloc allocator
..
src/db.c: integrate SQLite3 with talloc allocator
This change introduces an optional feature that allows to make
SQLite3 use talloc for all internal allocations. This would
facilitate finding memleaks. OsmoHLR needs to be configured
with '--enable-sqlite-talloc'.
full talloc report on 'OsmoHLR' (total 292168 bytes in 449 blocks)
struct osmo_gsup_servercontains162 bytes in 3 blocks (ref 0)
...
struct db_context contains 288407 bytes in 420 blocks (ref 0)
hlr.db contains 7 bytes in 1 blocks (ref 0)
SQLite3contains 288192 bytes in 418 blocks (ref 0)
db.c:95 contains 48 bytes in 1 blocks (ref 0)
db.c:95 contains 2 bytes in 1 blocks (ref 0)
...
Unfortunately, old SQLite3 versions (such as 3.8.2) run out
of memory when trying to initialize a new database:
DDB ERROR db.c:88 (7) statement aborts at 3: []
DDB ERROR db.c:420 Unable to set Write-Ahead Logging: out of memory
DDB ERROR db.c:88 (7) statement aborts at 3: []
DDB ERROR db.c:238 Unable to prepare SQL statement
'SELECT name FROM sqlite_master WHERE type='table' AND name=?'
...
I've noticed a huge difference in heap usage footprint compared to
generic malloc. At the same time, the recent versions (at least
3.24.0), work just fine.
Change-Id: Icfe67ed0f063b63e6794f9516da3003d01cf20a7
---
M configure.ac
M src/Makefile.am
M src/db.c
M src/db.h
A src/db_debug.c
M tests/db/Makefile.am
6 files changed, 126 insertions(+), 0 deletions(-)
Approvals:
laforge: Looks good to me, approved
neels: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/configure.ac b/configure.ac
index 6694f80..ef703f2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -59,6 +59,21 @@
CPPFLAGS="$CPPFLAGS -fsanitize=address -fsanitize=undefined"
fi
+AC_ARG_ENABLE([sqlite_talloc],
+ AC_HELP_STRING([--enable-sqlite-talloc],
+ [Configure SQLite3 to use talloc memory
allocator [default=no]]),
+ [sqlite_talloc="$enableval"],[sqlite_talloc="no"])
+if test "x$sqlite_talloc" = "xyes" ; then
+ # Older versions of SQLite3 (at least 3.8.2) become unstable with
talloc.
+ # Feel free to relax to 3.24.0 > VER > 3.8.2 if it works for you.
+ # FIXME: PKG_CHECK_MODULES() may return cached result here!
+ PKG_CHECK_MODULES(SQLITE3, sqlite3 >= 3.24.0)
+ AC_DEFINE([SQLITE_USE_TALLOC], 1, [Use talloc for SQLite3])
+fi
+AC_MSG_CHECKING([whether to use talloc for SQLite3])
+AC_MSG_RESULT([$sqlite_talloc])
+AM_CONDITIONAL([DB_SQLITE_DEBUG], [test "x$sqlite_talloc" = "xyes"])
+
AC_ARG_ENABLE(werror,
[AS_HELP_STRING(
[--enable-werror],
diff --git a/src/Makefile.am b/src/Makefile.am
index 131b44f..a042e4e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -97,6 +97,11 @@
$(LIBOSMOGSM_LIBS) \
$(NULL)
+if DB_SQLITE_DEBUG
+osmo_hlr_SOURCES += db_debug.c
+osmo_hlr_db_tool_SOURCES += db_debug.c
+endif
+
BOOTSTRAP_SQL = $(top_srcdir)/sql/hlr.sql
db_bootstrap.h: $(BOOTSTRAP_SQL) $(srcdir)/db_sql2c.sed
diff --git a/src/db.c b/src/db.c
index 7de61a2..5e6b5eb 100644
--- a/src/db.c
+++ b/src/db.c
@@ -365,6 +365,17 @@
LOGP(DDB, LOGL_INFO, "Compiled against SQLite3 lib version %s\n",
SQLITE_VERSION);
LOGP(DDB, LOGL_INFO, "Running with SQLite3 lib version %s\n",
sqlite3_libversion());
+#ifdef SQLITE_USE_TALLOC
+ /* Configure SQLite3 to use talloc memory allocator */
+ rc = db_sqlite3_use_talloc(ctx);
+ if (rc == SQLITE_OK) {
+ LOGP(DDB, LOGL_NOTICE, "SQLite3 is configured to use talloc\n");
+ } else {
+ LOGP(DDB, LOGL_ERROR, "Failed to configure SQLite3 "
+"to use talloc, using default memory allocator\n");
+ }
+#endif
+
dbc->fname = talloc_strdup(dbc, fname);
for (i = 0; i < 0xf; i++) {
diff --git a/src/db.h b/src/db.h
index 6e4bf49..15d83de 100644
--- a/src/db.h
+++ b/src/db.h
@@ -39,6 +39,11 @@
sqlite3_stmt *stmt[_NUM_DB_STMT];
};
+/* Optional feature to make SQLite3 using talloc */
+#ifdef SQLITE_USE_TALLOC
+int db_sqlite3_use_talloc(void *ctx);
+#endif
+
void db_remove_reset(sqlite3_stmt *stmt);
bool db_bind_text(sqlite3_stmt *stmt, const char *param_name, const char
*text);
bool db_bind_int(sqlite3_stmt *stmt, const char *param_name, int nr);
diff --git a/src/db_debug.c b/src/db_debug.c
new file mode 100644
index 000..13ccdd6
--- /dev/null
+++ b/src/db_debug.c
@@ -0,0 +1,86 @@
+/*
+ * libtalloc based memory allocator for SQLite3.
+ *
+ * (C) 2019 by Vadim
Change in ...osmo-hlr[master]: src/db.c: integrate SQLite3 with talloc allocator
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/14939 ) Change subject: src/db.c: integrate SQLite3 with talloc allocator .. Patch Set 3: (1 comment) https://gerrit.osmocom.org/#/c/14939/3//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/14939/3//COMMIT_MSG@34 PS3, Line 34: I've noticed a huge difference in heap usage footprint compared to > difference in what way? which one is smaller? It was in 2018 when I was trying to debug this odd bug. Basically I added a debug printf() to each (de|re|_)allocation wrapper. I don't remember which one was smaller, but with recent SQLite3 versions I did not notice any difference comparing the footprints of both malloc() and talloc(). What's even more interesting is that an old SQLite3 (e.g. 3.8.2) does pass 'make check' just fine... -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/14939 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: Icfe67ed0f063b63e6794f9516da3003d01cf20a7 Gerrit-Change-Number: 14939 Gerrit-PatchSet: 3 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Tue, 30 Jul 2019 17:14:20 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Gerrit-MessageType: comment
Change in ...osmo-hlr[master]: src/db.c: integrate SQLite3 with talloc allocator
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/14939 ) Change subject: src/db.c: integrate SQLite3 with talloc allocator .. Patch Set 3: Code-Review+1 (1 comment) https://gerrit.osmocom.org/#/c/14939/3//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/14939/3//COMMIT_MSG@34 PS3, Line 34: I've noticed a huge difference in heap usage footprint compared to difference in what way? which one is smaller? -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/14939 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: Icfe67ed0f063b63e6794f9516da3003d01cf20a7 Gerrit-Change-Number: 14939 Gerrit-PatchSet: 3 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Tue, 30 Jul 2019 17:04:13 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in ...osmo-hlr[master]: src/db.c: integrate SQLite3 with talloc allocator
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/14939 ) Change subject: src/db.c: integrate SQLite3 with talloc allocator .. Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/14939 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: Icfe67ed0f063b63e6794f9516da3003d01cf20a7 Gerrit-Change-Number: 14939 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Comment-Date: Fri, 26 Jul 2019 12:13:59 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in ...osmo-hlr[master]: src/db.c: integrate SQLite3 with talloc allocator
Hello pespin, laforge, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hlr/+/14939 to look at the new patch set (#3). Change subject: src/db.c: integrate SQLite3 with talloc allocator .. src/db.c: integrate SQLite3 with talloc allocator This change introduces an optional feature that allows to make SQLite3 use talloc for all internal allocations. This would facilitate finding memleaks. OsmoHLR needs to be configured with '--enable-sqlite-talloc'. full talloc report on 'OsmoHLR' (total 292168 bytes in 449 blocks) struct osmo_gsup_servercontains162 bytes in 3 blocks (ref 0) ... struct db_context contains 288407 bytes in 420 blocks (ref 0) hlr.db contains 7 bytes in 1 blocks (ref 0) SQLite3contains 288192 bytes in 418 blocks (ref 0) db.c:95 contains 48 bytes in 1 blocks (ref 0) db.c:95 contains 2 bytes in 1 blocks (ref 0) ... Unfortunately, old SQLite3 versions (such as 3.8.2) run out of memory when trying to initialize a new database: DDB ERROR db.c:88 (7) statement aborts at 3: [] DDB ERROR db.c:420 Unable to set Write-Ahead Logging: out of memory DDB ERROR db.c:88 (7) statement aborts at 3: [] DDB ERROR db.c:238 Unable to prepare SQL statement 'SELECT name FROM sqlite_master WHERE type='table' AND name=?' ... I've noticed a huge difference in heap usage footprint compared to generic malloc. At the same time, the recent versions (at least 3.24.0), work just fine. Change-Id: Icfe67ed0f063b63e6794f9516da3003d01cf20a7 --- M configure.ac M src/Makefile.am M src/db.c M src/db.h A src/db_debug.c M tests/db/Makefile.am 6 files changed, 126 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/39/14939/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/14939 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: Icfe67ed0f063b63e6794f9516da3003d01cf20a7 Gerrit-Change-Number: 14939 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-MessageType: newpatchset
Change in ...osmo-hlr[master]: src/db.c: integrate SQLite3 with talloc allocator
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/14939 ) Change subject: src/db.c: integrate SQLite3 with talloc allocator .. Patch Set 2: Code-Review-1 (2 comments) https://gerrit.osmocom.org/#/c/14939/2/src/db.c File src/db.c: https://gerrit.osmocom.org/#/c/14939/2/src/db.c@88 PS2, Line 88: #ifdef SQLITE_USE_TALLOC I'd rather move all this block into its own file + header, in case somebody wants to quickly reuse it in other apps using talloc + sqlite (so they don't need to go look all along this file). https://gerrit.osmocom.org/#/c/14939/2/src/db.c@426 PS2, Line 426: #ifdef SQLITE_USE_TALLOC Move this into the file and provide a public function on that file (header). -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/14939 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: Icfe67ed0f063b63e6794f9516da3003d01cf20a7 Gerrit-Change-Number: 14939 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Comment-Date: Thu, 25 Jul 2019 17:13:11 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in ...osmo-hlr[master]: src/db.c: integrate SQLite3 with talloc allocator
Hello pespin, laforge, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hlr/+/14939 to look at the new patch set (#2). Change subject: src/db.c: integrate SQLite3 with talloc allocator .. src/db.c: integrate SQLite3 with talloc allocator This change introduces an optional feature that allows to make SQLite3 use talloc for all internal allocations. This would facilitate finding memleaks. OsmoHLR needs to be configured with '--enable-sqlite-talloc'. full talloc report on 'OsmoHLR' (total 292168 bytes in 449 blocks) struct osmo_gsup_servercontains162 bytes in 3 blocks (ref 0) ... struct db_context contains 288407 bytes in 420 blocks (ref 0) hlr.db contains 7 bytes in 1 blocks (ref 0) SQLite3contains 288192 bytes in 418 blocks (ref 0) db.c:95 contains 48 bytes in 1 blocks (ref 0) db.c:95 contains 2 bytes in 1 blocks (ref 0) ... Unfortunately, old SQLite3 versions (such as 3.8.2) somehow become unstable and fail to initialize the database with talloc. There is a huge difference in heap usage footprint compared to malloc. At the same time, the recent versions (at least 3.24.0), work just fine. Change-Id: Icfe67ed0f063b63e6794f9516da3003d01cf20a7 --- M configure.ac M src/db.c 2 files changed, 85 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/39/14939/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/14939 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: Icfe67ed0f063b63e6794f9516da3003d01cf20a7 Gerrit-Change-Number: 14939 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-MessageType: newpatchset
Change in ...osmo-hlr[master]: src/db.c: integrate SQLite3 with talloc allocator
fixeria has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-hlr/+/14939
Change subject: src/db.c: integrate SQLite3 with talloc allocator
..
src/db.c: integrate SQLite3 with talloc allocator
This change introduces an optional feature that allows to make
SQLite3 use talloc for all internal allocations. This would
facilitate finding memleaks. OsmoHLR needs to be configured
with '--enable-sqlite-talloc'.
full talloc report on 'OsmoHLR' (total 292168 bytes in 449 blocks)
struct osmo_gsup_servercontains162 bytes in 3 blocks (ref 0)
...
struct db_context contains 288407 bytes in 420 blocks (ref 0)
hlr.db contains 7 bytes in 1 blocks (ref 0)
SQLite3 contains 288192 bytes in 418 blocks (ref 0)
db.c:95contains 48 bytes in 1 blocks (ref 0)
db.c:95contains 2 bytes in 1 blocks (ref 0)
...
Unfortunately, old SQLite3 versions (such as 3.8.2) somehow become
unstable and fail to initialize the database with talloc. There is
a huge difference in heap usage footprint compared to malloc. At the
same time, the recent versions (at least 3.24.0), work just fine.
Change-Id: Icfe67ed0f063b63e6794f9516da3003d01cf20a7
---
M configure.ac
M src/db.c
2 files changed, 85 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/39/14939/1
diff --git a/configure.ac b/configure.ac
index 6694f80..c59f40f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -59,6 +59,20 @@
CPPFLAGS="$CPPFLAGS -fsanitize=address -fsanitize=undefined"
fi
+AC_ARG_ENABLE([sqlite_talloc],
+ AC_HELP_STRING([--enable-sqlite-talloc],
+ [Configure SQLite3 to use talloc memory
allocator [default=no]]),
+ [sqlite_talloc="$enableval"],[sqlite_talloc="no"])
+if test "x$sqlite_talloc" = "xyes" ; then
+ # Older versions of SQLite3 (at least 3.8.2) become unstable with
talloc.
+ # Feel free to relax to 3.24.0 > VER > 3.8.2 if it works for you.
+ # FIXME: PKG_CHECK_MODULES() may return cached result here!
+ PKG_CHECK_MODULES(SQLITE3, sqlite3 >= 3.24.0)
+ AC_DEFINE([SQLITE_USE_TALLOC], 1, [Use talloc for SQLite3])
+fi
+AC_MSG_CHECKING([whether to use talloc for SQLite3])
+AC_MSG_RESULT([$sqlite_talloc])
+
AC_ARG_ENABLE(werror,
[AS_HELP_STRING(
[--enable-werror],
diff --git a/src/db.c b/src/db.c
index 7de61a2..f0bac2f 100644
--- a/src/db.c
+++ b/src/db.c
@@ -18,6 +18,7 @@
*/
#include
+#include
#include
#include
@@ -83,6 +84,63 @@
[DB_STMT_EXISTS_BY_MSISDN] = "SELECT 1 FROM subscriber WHERE msisdn =
$msisdn",
};
+/* Optional feature to make SQLite using talloc */
+#ifdef SQLITE_USE_TALLOC
+
+/* Dedicated talloc context for SQLite */
+static void *db_sqlite_ctx = NULL;
+
+static void *tall_xMalloc(int size)
+{
+ return talloc_size(db_sqlite_ctx, size);
+}
+
+static void tall_xFree(void *ptr)
+{
+ talloc_free(ptr);
+}
+
+static void *tall_xRealloc(void *ptr, int size)
+{
+ return talloc_realloc_fn(db_sqlite_ctx, ptr, size);
+}
+
+static int tall_xSize(void *ptr)
+{
+ return talloc_total_size(ptr);
+}
+
+/* DUMMY: talloc doesn't round up the allocation size */
+static int tall_xRoundup(int size) { return size; }
+
+/* DUMMY: nothing to initialize */
+static int tall_xInit(void *data) { return 0; }
+
+/* DUMMY: nothing to deinitialize */
+static void tall_xShutdown(void *data) { }
+
+/* Interface between SQLite and talloc memory allocator */
+static const struct sqlite3_mem_methods tall_sqlite_if = {
+ /* Memory allocation function */
+ .xMalloc = _xMalloc,
+ /* Free a prior allocation */
+ .xFree = _xFree,
+ /* Resize an allocation */
+ .xRealloc = _xRealloc,
+ /* Return the size of an allocation */
+ .xSize = _xSize,
+ /* Round up request size to allocation size */
+ .xRoundup = _xRoundup,
+ /* Initialize the memory allocator */
+ .xInit = _xInit,
+ /* Deinitialize the memory allocator */
+ .xShutdown = _xShutdown,
+ /* Argument to xInit() and xShutdown() */
+ .pAppData = NULL,
+};
+
+#endif /* SQLITE_USE_TALLOC */
+
static void sql3_error_log_cb(void *arg, int err_code, const char *msg)
{
LOGP(DDB, LOGL_ERROR, "(%d) %s\n", err_code, msg);
@@ -365,6 +423,19 @@
LOGP(DDB, LOGL_INFO, "Compiled against SQLite3 lib version %s\n",
SQLITE_VERSION);
LOGP(DDB, LOGL_INFO, "Running with SQLite3 lib version %s\n",
sqlite3_libversion());
+#ifdef SQLITE_USE_TALLOC
+ db_sqlite_ctx = talloc_named_const(dbc, 0, "SQLite3");
+
+ /* Configure SQLite3 to use talloc memory allocator */
+ rc = sqlite3_config(SQLITE_CONFIG_MALLOC, _sqlite_if);
+ if (rc == SQLITE_OK) {
+
