Experimental patches for LMDB header-caching in Mutt

00README.txt

$ hg clone http://dev.mutt.org/hg/mutt
$ ./prepare --enable-hcache
$ vi Makefile
MUTTLIBS =  -lncurses -llmdb
$ vi config.h
/* #define HAVE_TC 1 */
... and all other DB stores
#define HAVE_LMDB 1
$ patch -p < hcache.patch
$ make
$ ./mutt -v | grep lmdb
hcache backend: lmdb LMDB 0.9.14: (September 20, 2014)


Loading a 25213 message Maildir/ :
- without hcache: 25s
- with Tokyo:      6s
- with LMDB:      <2s

However, unless I've done something completely wrongly (which is quite likely)
disk space used for all header caches of all my folders is:

 47M	using tokyo
210M	using lmdb

hcache.patch

--- hcache.c.original	2015-10-13 09:10:53.000000000 +0200
+++ hcache.c	2015-10-13 15:29:16.000000000 +0200
@@ -32,6 +32,9 @@
 #include <gdbm.h>
 #elif HAVE_DB4
 #include <db.h>
+#elif HAVE_LMDB
+#define LMDB_DB_SIZE    (1024 * 1024 * 1024)
+#include <lmdb.h>
 #endif
 
 #include <errno.h>
@@ -83,6 +86,14 @@
 
 static void mutt_hcache_dbt_init(DBT * dbt, void *data, size_t len);
 static void mutt_hcache_dbt_empty_init(DBT * dbt);
+#elif HAVE_LMDB
+struct header_cache
+{
+  MDB_env *env;
+  MDB_dbi db;
+  char *folder;
+  unsigned int crc;
+};
 #endif
 
 typedef union
@@ -732,6 +743,11 @@
 #elif HAVE_DB4
   DBT key;
   DBT data;
+#elif HAVE_LMDB
+  int rc;
+  MDB_val key;
+  MDB_val data;
+  MDB_txn *txn;
 #endif
   
   if (!h)
@@ -748,6 +764,38 @@
   h->db->get(h->db, NULL, &key, &data, 0);
   
   return data.data;
+#elif HAVE_LMDB
+  strncpy(path, h->folder, sizeof (path));
+  safe_strcat(path, sizeof (path), filename);
+
+  ksize = strlen (h->folder) + keylen (path + strlen (h->folder));  
+  key.mv_data = (char *)path;
+  key.mv_size = ksize;
+  data.mv_data = NULL;
+  data.mv_size = 0;
+  if ((rc = mdb_txn_begin(h->env, NULL, 0, &txn)) != 0)
+  	fprintf(stderr, "txn_begin: %s\n", mdb_strerror(rc));
+  rc = mdb_get(txn, h->db, &key, &data);
+  if (rc == MDB_NOTFOUND) {
+    mdb_txn_commit(txn);
+    return (NULL);
+  }
+  if (rc != 0) {
+    fprintf(stderr, "mdb_get: %s\n", mdb_strerror(rc));
+    mdb_txn_commit(txn);
+    return (NULL);
+  }
+  /* Caller frees the data we return, so I MUST make a copy of it */
+
+  char *d = malloc(data.mv_size);
+  if (d) {
+    memcpy(d, data.mv_data, data.mv_size);
+  }
+  mdb_txn_commit(txn);
+
+  return d;
+  	
+
 #else
   strncpy(path, h->folder, sizeof (path));
   safe_strcat(path, sizeof (path), filename);
@@ -813,6 +861,11 @@
 #elif HAVE_DB4
   DBT key;
   DBT databuf;
+#elif HAVE_LMDB
+  int rc;
+  MDB_val key;
+  MDB_val databuf;
+  MDB_txn *txn;
 #endif
   
   if (!h)
@@ -831,6 +884,23 @@
   databuf.ulen = dlen;
   
   return h->db->put(h->db, NULL, &key, &databuf, 0);
+#elif HAVE_LMDB
+  strncpy(path, h->folder, sizeof (path));
+  safe_strcat(path, sizeof (path), filename);
+  ksize = strlen(h->folder) + keylen(path + strlen(h->folder));
+
+  key.mv_data = (char *)path;
+  key.mv_size = ksize;
+  databuf.mv_data = data;
+  databuf.mv_size = dlen;
+  if ((rc = mdb_txn_begin(h->env, NULL, 0, &txn)) != 0)
+  	fprintf(stderr, "txn_begin: %s\n", mdb_strerror(rc));
+  rc = mdb_put(txn, h->db, &key, &databuf, 0);
+  if (rc != 0) {
+  	fprintf(stderr, "mdb_get: %s\n", mdb_strerror(rc));
+  }
+  mdb_txn_commit(txn);
+  return rc == 0;
 #else
   strncpy(path, h->folder, sizeof (path));
   safe_strcat(path, sizeof (path), filename);
@@ -1126,6 +1196,84 @@
   mutt_hcache_dbt_init(&key, (void *) filename, keylen(filename));
   return h->db->del(h->db, NULL, &key, 0);
 }
+#elif HAVE_LMDB
+
+static int
+hcache_open_lmdb (struct header_cache* h, const char* path)
+{
+  int ret;
+  MDB_txn *txn;
+
+  ret = mdb_env_create(&h->env);
+  if (ret != 0) {
+	  fprintf(stderr, "hcache_open_lmdb: mdb_env_create: %s", mdb_strerror(ret));
+	  goto fail_env;
+  }
+
+  mdb_env_set_mapsize(h->env, LMDB_DB_SIZE);
+
+  ret = mdb_env_open(h->env, path, MDB_NOSUBDIR, 0644);
+  if (ret != 0) {
+	  fprintf(stderr, "hcache_open_lmdb: mdb_env_open: %s", mdb_strerror(ret));
+	  goto fail_env;
+  }
+
+  mdb_txn_begin(h->env, NULL, 0, &txn);
+  ret = mdb_dbi_open(txn, NULL, MDB_CREATE, &h->db);
+  if (ret != 0) {
+	  fprintf(stderr, "hcache_open_lmdb: mdb_dbi_open: %s", mdb_strerror(ret));
+	  goto fail_env;
+  }
+  mdb_txn_commit(txn);
+
+  return 0;
+
+  fail_env:
+  mdb_env_close(h->env);
+
+  return -1;
+}
+
+void
+mutt_hcache_close(header_cache_t *h)
+{
+  if (!h)
+    return;
+
+  mdb_env_close(h->env);
+  FREE (&h->folder);
+  FREE (&h);
+}
+
+int
+mutt_hcache_delete(header_cache_t *h, const char *filename,
+		   size_t(*keylen) (const char *fn))
+{
+  MDB_val key;
+  MDB_val databuf;
+  MDB_txn *txn;
+  int rc;
+
+  if (!h)
+    return -1;
+
+  if (filename[0] == '/')
+    filename++;
+
+  key.mv_data = (char *)filename;
+  key.mv_size = strlen(filename);
+  databuf.mv_data = NULL;
+  databuf.mv_size = 0;
+  if ((rc = mdb_txn_begin(h->env, NULL, 0, &txn)) != 0)
+  	fprintf(stderr, "txn_begin: %s\n", mdb_strerror(rc));
+  rc = mdb_put(txn, h->db, &key, &databuf, 0);
+  if (rc != 0) {
+  	fprintf(stderr, "mdb_get: %s\n", mdb_strerror(rc));
+  }
+  mdb_txn_commit(txn);
+
+  return rc;
+}
 #endif
 
 header_cache_t *
@@ -1143,6 +1291,8 @@
   hcache_open = hcache_open_gdbm;
 #elif HAVE_DB4
   hcache_open = hcache_open_db4;
+#elif HAVE_LMDB
+  hcache_open = hcache_open_lmdb;
 #endif
 
   /* Calculate the current hcache version from dynamic configuration */
@@ -1180,7 +1330,11 @@
     hcachever = digest.intval;
   }
 
+#if HAVE_LMDB
+  h->db = 0;
+#else
   h->db = NULL;
+#endif
   h->folder = get_foldername(folder);
   h->crc = hcachever;
 
@@ -1215,6 +1369,11 @@
 {
   return DB_VERSION_STRING;
 }
+#elif HAVE_LMDB
+const char *mutt_hcache_backend (void)
+{
+  return "lmdb " MDB_VERSION_STRING;
+}
 #elif HAVE_GDBM
 const char *mutt_hcache_backend (void)
 {

Line 54 - you must return failure as soon as txn_begin fails. If it fails, then your line 55 will be using a NULL txn.

Line 48 - computing strlen(x) twice? Shame on you.

Line 53 - for read-only use you should create a read-only txn.

Everywhere: When cleaning up on failure you should use txn_abort() not txn_commit().

Line 108 - your errmsg should say mdb_put, not mdb_get.

Line 189 - Shouldn't your delete function be using mdb_del? Is there a reason to keep the key in the DB while only removing its data?

Hi @hyc, in agreement with @jpmens, I have reviewed your comments and uploaded an updated gist here:

https://gist.github.com/gahr/7991dd8d9c06af75353540fdfbd7580a

Would you mind taking a look?

Thanks a lot!