Re: [Evolution-hackers] libdb performance issue? (was: Re: libebook: errors when using asynchronous contact addition/removal functions)

[Patrick Ohly <patrick ohly gmx de>]

[Milan, can you please keep other people on CC?

I know that you personally prefer to not be CCed on mailing list emails
and I try to remember that in my own replies, but others are not on the
list and may depend on being CCed. For example, I don't know whether
Chris is subscribed.]

On Mo, 2011-05-16 at 11:12 +0200, Patrick Ohly wrote:
> It might be easier to set DB_INIT_CDB, which enforces multiple
> reads/single writer access without deadlocks. I'll give that a try.

And it works beautifully. One word added to the source code and the
stress test passes reliably and quickly :-)

Patch attached. Okay to submit into master (not tested there, though)
and gnome-2-32 branches (which is where I have tested it)?

-- 
Bye, Patrick Ohly
--  
Patrick Ohly gmx de
http://www.estamos.de/

>From 2b4cff9230a049d5ad7fe86873aba9a7bba35af1 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick ohly intel com>
Date: Mon, 16 May 2011 11:21:04 +0200
Subject: [PATCH] addressbook file backend: libdb must be initialized for concurrent read/write

Very bad performance (100% CPU load, several minutes run time) were
seen for multiple concurrent writes. gdb shows that libdb is
apparently busy polling while writing.

The libdb API docs for DB_ENV->open() imply that either DB_INIT_CDB or
DB_INIT_LOCK must be used in apps which are not read-only, like
EDS. This patch adds DB_INIT_CDB because it is simple and fixes the
performance problem.

In some rare cases, DB_INIT_LOCK might provide better performance by
allowing concurrent writes of independent data, but that seems too
complicated for not enough gain right now (must check for deadlocks).
---
 addressbook/backends/file/e-book-backend-file.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/addressbook/backends/file/e-book-backend-file.c b/addressbook/backends/file/e-book-backend-file.c
index d21c527..372ade5 100644
--- a/addressbook/backends/file/e-book-backend-file.c
+++ b/addressbook/backends/file/e-book-backend-file.c
@@ -1146,7 +1146,18 @@ e_book_backend_file_load_source (EBookBackend           *backend,
 				(gpointer (*)(gpointer , gsize))g_try_realloc,
 				g_free);
 
-		db_error = (*env->open) (env, NULL, DB_CREATE | DB_INIT_MPOOL | DB_PRIVATE | DB_THREAD, 0);
+		/*
+		 * We need either DB_INIT_CDB or DB_INIT_LOCK, because we will have
+		 * multiple threads reading and writing concurrently without
+		 * any locking above libdb.
+		 *
+		 * DB_INIT_CDB enforces multiple reader/single writer by locking inside
+		 * the database. It is used instead of DB_INIT_LOCK because DB_INIT_LOCK
+		 * may deadlock, which would have to be called in a separate thread.
+		 * Considered too complicated for not enough gain (= concurrent writes)
+		 * at this point.
+		 */
+		db_error = (*env->open) (env, NULL, DB_INIT_CDB | DB_CREATE | DB_INIT_MPOOL | DB_PRIVATE | DB_THREAD, 0);
 		if (db_error != 0) {
 			env->close(env, 0);
 			g_warning ("db_env_open failed with %s", db_strerror (db_error));
-- 
1.7.2.5