Create a gist now

Instantly share code, notes, and snippets.

@jpmens /00README.txt
Last active May 24, 2016

What would you like to do?
Experimental patches for LMDB header-caching in Mutt
$ 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.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)
{
hyc commented Oct 14, 2015

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?

gahr commented May 24, 2016

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment