Re: [Mutt] #3691: Add LMDB support

[Mutt]

#3691: Add LMDB support
---+--
  Reporter:  rsc   |  Owner:  mutt-dev
  Type:  enhancement   | Status:  new
  Priority:  minor |  Milestone:
 Component:  header cache  |Version:
Resolution:|   Keywords:
---+--

Comment (by Kevin McCarthy ):

 In [changeset:"42aa8b19da95b947492ef6106b62a1fb9ba2b56f"
 6923:42aa8b19da95]:
 {{{
 #!CommitTicketReference repository=""
 revision="42aa8b19da95b947492ef6106b62a1fb9ba2b56f"
 Add LMDB backend support for header cache. (see #3691)

 Based on the original from JP Mens:
 https://gist.github.com/jpmens/15969d9d678a3d450e4e

 The following performance patch was manually applied on top of the
 original patch:
 
https://github.com/neomutt/neomutt/commit/7e5380cd4c40d119ff83b2cf5f51f2cdb8a95ab3

 A variant of this patch was added to handle larger mailboxes:
 
https://github.com/neomutt/neomutt/commit/6d337642e701b1dde4b5d0812e01c85f41ba65ca

 Thanks to all the developers and contributors to this patch, and to
 Fabian Groffen for bundling and posting this to mutt-dev.
 }}}

--
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [Mutt] #3691: Add LMDB support

[Mutt]

#3691: Add LMDB support
---+--
  Reporter:  rsc   |  Owner:  mutt-dev
  Type:  enhancement   | Status:  closed
  Priority:  minor |  Milestone:
 Component:  header cache  |Version:
Resolution:  fixed |   Keywords:
---+--
Changes (by Kevin McCarthy ):

 * status:  new => closed
 * resolution:   => fixed


Comment:

 In [changeset:"fca7e504ab6a0de506f33ddde90267d07dc74049"
 6924:fca7e504ab6a]:
 {{{
 #!CommitTicketReference repository=""
 revision="fca7e504ab6a0de506f33ddde90267d07dc74049"
 Fixes to the LMDB header cache. (closes #3691)

 Use mdb_txn_abort() to free up readonly/reset transactions, and avoid
 leaking memory.

 Fix hcache_delete() key generation - looks like this was an incorrect
 copy/paste from the bdb code.

 Use dprint, not fprintf, for debugging messages.

 Remove strange blending of enum and bitfield logic for the txn_mode
 state.

 Remove some duplicate code from store/fetch raw.
 }}}

--
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [Mutt] #3691: Add LMDB support

[Mutt]

#3691: Add LMDB support
---+--
  Reporter:  rsc   |  Owner:  mutt-dev
  Type:  enhancement   | Status:  new
  Priority:  minor |  Milestone:
 Component:  header cache  |Version:
Resolution:|   Keywords:
---+--

Comment (by gahr2):

 Weird, I don't experience the same usage pattern. Actually my version and
 your modified version of my version (...) behave quite similarly on my
 environment. Anyway, yours is preferable, thanks for putting some effort
 into it!

--
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [Mutt] #3691: Add LMDB support

[Mutt]

#3691: Add LMDB support
---+--
  Reporter:  rsc   |  Owner:  mutt-dev
  Type:  enhancement   | Status:  new
  Priority:  minor |  Milestone:
 Component:  header cache  |Version:
Resolution:|   Keywords:
---+--
Changes (by neverpanic):

 * Attachment "2016-09-19-diff-re-use-transactions.patch" added.

 Diff to the version in comment 8 that re-uses transactions

--
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [Mutt] #3691: Add LMDB support

[Mutt]

#3691: Add LMDB support
---+--
  Reporter:  rsc   |  Owner:  mutt-dev
  Type:  enhancement   | Status:  new
  Priority:  minor |  Milestone:
 Component:  header cache  |Version:
Resolution:|   Keywords:
---+--

Comment (by gahr2):

 I have an alternative approach, which is, let's keep the last (read /
 write) transaction open and reuse it until a transaction for a different
 mode (write / read) is required. At that point, reset it if it was a read
 transaction and commit it if it was a write transaction. I noticed a
 relevant speed up with this patch.


 {{{#!diff
 diff -r f1f1af650910 configure.ac
 --- a/configure.ac  Tue May 24 12:08:46 2016 -0700
 +++ b/configure.ac  Mon Sep 19 14:18:01 2016 +
 @@ -855,6 +855,7 @@
  AC_ARG_WITH(qdbm, AS_HELP_STRING([--without-qdbm],[Don't use qdbm even if
 it is available]))
  AC_ARG_WITH(gdbm, AS_HELP_STRING([--without-gdbm],[Don't use gdbm even if
 it is available]))
  AC_ARG_WITH(bdb, AS_HELP_STRING([--with-bdb@<:@=DIR@:>@],[Use BerkeleyDB4
 if gdbm is not available]))
 +AC_ARG_WITH(lmdb, AS_HELP_STRING([--with-lmdb@<:@=DIR@:>@],[Use LMDB if
 gdbm is not available]))

  db_found=no
  if test x$enable_hcache = xyes
 @@ -899,6 +900,15 @@
  db_requested=bdb
fi
  fi
 +if test -n "$with_lmdb" && test "$with_lmdb" != "no"
 +then
 +  if test "$db_requested" != "auto"
 +  then
 +AC_MSG_ERROR([more than one header cache engine requested.])
 +  else
 +db_requested=lmdb
 +  fi
 +fi

  dnl -- Tokyo Cabinet --
  if test "$with_tokyocabinet" != "no" \
 @@ -1042,6 +1052,34 @@
  fi
  fi

 +dnl -- LMDB --
 +if test x$with_lmdb != xno && test $db_found = no \
 +   && test "$db_requested" = auto -o "$db_requested" = lmdb
 +then
 +if test "$with_lmdb" != "yes"
 +then
 +  CPPFLAGS="$CPPFLAGS -I$with_lmdb/include"
 +  LDFLAGS="$LDFLAGS -L$with_lmdb/lib"
 +fi
 +saved_LIBS="$LIBS"
 +LIBS="$LIBS -llmdb"
 +AC_CACHE_CHECK(for mdb_env_create, ac_cv_mdbenvcreate,[
 +ac_cv_mdbenvcreate=no
 +AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include ]],
 [[mdb_env_create(0);]])],[ac_cv_mdbenvcreate=yes],[])
 +])
 +LIBS="$saved_LIBS"
 +if test "$ac_cv_mdbenvcreate" = yes
 +then
 +  AC_DEFINE(HAVE_LMDB, 1, [LMDB Support])
 +  MUTTLIBS="$MUTTLIBS -llmdb"
 +  db_found=lmdb
 +fi
 +if test "$db_requested" != auto && test "$db_found" !=
 "$db_requested"
 +then
 +  AC_MSG_ERROR([LMDB could not be used. Check config.log for
 details.])
 +fi
 +fi
 +
  if test $db_found = no
  then
  AC_MSG_ERROR([You need Tokyo Cabinet, QDBM, GDBM or Berkeley DB4
 for hcache])
 diff -r f1f1af650910 hcache.c
 --- a/hcache.c  Tue May 24 12:08:46 2016 -0700
 +++ b/hcache.c  Mon Sep 19 14:18:01 2016 +
 @@ -32,6 +32,9 @@
  #include 
  #elif HAVE_DB4
  #include 
 +#elif HAVE_LMDB
 +#define LMDB_DB_SIZE(1024 * 1024 * 1024)
 +#include 
  #endif

  #include 
 @@ -83,6 +86,52 @@

  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_txn *txn;
 +  MDB_dbi db;
 +  char *folder;
 +  unsigned int crc;
 +  int last_mode; // 0: read, 1: write
 +};
 +
 +static int mdb_get_r_txn(header_cache_t *h)
 +{
 +  if (h->txn && h->last_mode == 1)
 +  {
 +mdb_txn_commit(h->txn);
 +h->txn = NULL;
 +  }
 +  h->last_mode = 0;
 +  if (!h->txn)
 +  {
 +return mdb_txn_begin(h->env, NULL, MDB_RDONLY, >txn);
 +  }
 +  else
 +  {
 +return mdb_txn_renew(h->txn);
 +  }
 +}
 +
 +static int mdb_get_w_txn(header_cache_t *h)
 +{
 +  if (h->txn && h->last_mode == 0)
 +  {
 +mdb_txn_reset(h->txn);
 +h->txn = NULL;
 +  }
 +  h->last_mode = 1;
 +  if (h->txn)
 +  {
 +return MDB_SUCCESS;
 +  }
 +  else
 +  {
 +return mdb_txn_begin(h->env, NULL, 0, >txn);
 +  }
 +}
  #endif

  typedef union
 @@ -732,6 +793,11 @@
  #elif HAVE_DB4
DBT key;
DBT data;
 +#elif HAVE_LMDB
 +  MDB_val key;
 +  MDB_val data;
 +  size_t folderlen;
 +  int rc;
  #endif

if (!h)
 @@ -748,6 +814,43 @@
h->db->get(h->db, NULL, , , 0);

return data.data;
 +#elif HAVE_LMDB
 +  strncpy(path, h->folder, sizeof (path));
 +  safe_strcat(path, sizeof (path), filename);
 +
 +  folderlen = strlen(h->folder);
 +  ksize = folderlen + keylen(path + folderlen);
 +  key.mv_data = (char *)path;
 +  key.mv_size = ksize;
 +  data.mv_data = NULL;
 +  data.mv_size = 0;
 +  rc = mdb_get_r_txn(h);
 +  if (rc != MDB_SUCCESS)
 +  {
 +h->txn = NULL;
 +fprintf(stderr, "txn_renew: %s\n", mdb_strerror(rc));
 +return NULL;
 +  }
 +  rc = mdb_get(h->txn, h->db, , );
 +  if (rc == 

Re: [Mutt] #3691: Add LMDB support

[Mutt]

#3691: Add LMDB support
---+--
  Reporter:  rsc   |  Owner:  mutt-dev
  Type:  enhancement   | Status:  new
  Priority:  minor |  Milestone:
 Component:  header cache  |Version:
Resolution:|   Keywords:
---+--

Comment (by neverpanic):

 LMDB performs much better than Tokyo Cabinet while reading, so this is
 definitely nice to have. However, I've noticed that while writing the
 cache, your code starts a transaction for every single write operation,
 which causes this to be much slower than I think it needs to be.

 Here's a quick comparison when opening a 130k mail mailbox with empty
 cache. In all cases I asked mutt to exit by pressing 'q' while it was
 generating the cache.
 {{{
 rm ./headers.lmdb*; time ./mutt -e 'set header_cache = "./headers.lmdb"'
 -f ~/Mail/MacPorts/tickets

 real0m29.271s
 user0m8.320s
 sys 0m11.525s
 }}}

 The same command with changes that explicitly start a single transaction
 before updating the cache:
 {{{
 real0m10.819s
 user0m6.886s
 sys 0m2.762s
 }}}

 I'm including the changes I used to generate these numbers below, but note
 that this is nowhere near ready for a merge as-is. I think `hcache.c`
 shouldn't abstract the need for a transaction away from the users of
 header cache, because those users alone know best whether they are trying
 to run multiple updates in sequence.
 {{{
 #!patch
 diff --git a/hcache.c b/hcache.c
 index f3ca50f..8f2d009 100644
 --- a/hcache.c
 +++ b/hcache.c
 @@ -100,6 +100,7 @@ struct header_cache
  {
MDB_env *env;
MDB_txn *txn;
 +  MDB_txn *wtxn;
MDB_dbi db;
char *folder;
unsigned int crc;
 @@ -898,6 +899,7 @@ mutt_hcache_store_raw (header_cache_t* h, const char*
 filename, void* data,
  #elif HAVE_LMDB
MDB_val key;
MDB_val databuf;
 +  MDB_txn *local_txn;
MDB_txn *txn;
size_t folderlen;
int rc;
 @@ -929,20 +931,32 @@ mutt_hcache_store_raw (header_cache_t* h, const
 char* filename, void* data,
key.mv_size = ksize;
databuf.mv_data = data;
databuf.mv_size = dlen;
 -  rc = mdb_txn_begin(h->env, NULL, 0, );
 -  if (rc != MDB_SUCCESS)
 -  {
 -fprintf(stderr, "txn_begin: %s\n", mdb_strerror(rc));
 -return rc;
 +
 +  txn = NULL;
 +  local_txn = NULL;
 +  if (h->wtxn) {
 +txn = h->wtxn;
 +  } else {
 +rc = mdb_txn_begin(h->env, NULL, 0, _txn);
 +if (rc != MDB_SUCCESS)
 +{
 +  fprintf(stderr, "txn_begin: %s\n", mdb_strerror(rc));
 +  return rc;
 +}
 +txn = local_txn;
}
rc = mdb_put(txn, h->db, , , 0);
if (rc != MDB_SUCCESS)
{
  fprintf(stderr, "mdb_put: %s\n", mdb_strerror(rc));
 -mdb_txn_abort(txn);
 +if (local_txn) {
 +  mdb_txn_abort(txn);
 +}
  return rc;
}
 -  rc = mdb_txn_commit(txn);
 +  if (local_txn) {
 +rc = mdb_txn_commit(txn);
 +  }
return rc;
  #else
strncpy(path, h->folder, sizeof (path));
 @@ -1324,6 +1338,7 @@ hcache_open_lmdb (struct header_cache* h, const
 char* path)
int rc;

h->txn = NULL;
 +  h->wtxn = NULL;

rc = mdb_env_create(>env);
if (rc != MDB_SUCCESS)
 @@ -1414,6 +1429,38 @@ mutt_hcache_delete(header_cache_t *h, const char
 *filename,
mdb_txn_commit(txn);
return rc;
  }
 +
 +int mutt_hcache_txn_begin(header_cache_t *h) {
 +  if (!h) {
 +return -1;
 +  }
 +
 +  return mdb_txn_begin(h->env, NULL, 0, >wtxn);
 +}
 +int mutt_hcache_txn_commit(header_cache_t *h) {
 +  int rc;
 +
 +  if (!h || !h->wtxn) {
 +return -1;
 +  }
 +
 +  rc = mdb_txn_commit(h->wtxn);
 +  if (rc != MDB_SUCCESS) {
 +h->wtxn = NULL;
 +  }
 +
 +  return rc;
 +}
 +int mutt_hcache_txn_abort(header_cache_t *h) {
 +  if (!h || !h->wtxn) {
 +return -1;
 +  }
 +
 +  mdb_txn_abort(h->wtxn);
 +  h->wtxn = NULL;
 +
 +  return 0;
 +}
  #endif

  header_cache_t *
 diff --git a/hcache.h b/hcache.h
 index ca0b425..10eead1 100644
 --- a/hcache.h
 +++ b/hcache.h
 @@ -45,6 +45,10 @@ int mutt_hcache_store_raw (header_cache_t *h, const
 char* filename, void* data,
 size_t dlen, size_t(*keylen) (const char*
 fn));
  int mutt_hcache_delete(header_cache_t *h, const char *filename, size_t
 (*keylen)(const char *fn));

 +int mutt_hcache_txn_begin(header_cache_t *h);
 +int mutt_hcache_txn_commit(header_cache_t *h);
 +int mutt_hcache_txn_abort(header_cache_t *h);
 +
  const char *mutt_hcache_backend (void);

  #endif /* _HCACHE_H_ */
 diff --git a/mh.c b/mh.c
 index c90679e..62325be 100644
 --- a/mh.c
 +++ b/mh.c
 @@ -1154,6 +1154,7 @@ static void maildir_delayed_parsing (CONTEXT * ctx,
 struct maildir **md,

  #if USE_HCACHE
hc = mutt_hcache_open (HeaderCache, ctx->path, NULL);
 +  mutt_hcache_txn_begin (hc);
  #endif

for (p = *md, count = 0; p; p = p->next, count++)
 @@ -1216,6 +1217,7 @@ static void maildir_delayed_parsing (CONTEXT * ctx,
 struct maildir **md,
  last = p;
 }
  #if USE_HCACHE
 +  

Re: [Mutt] #3691: Add LMDB support

[Mutt]

#3691: Add LMDB support
---+--
  Reporter:  rsc   |  Owner:  mutt-dev
  Type:  enhancement   | Status:  new
  Priority:  minor |  Milestone:
 Component:  header cache  |Version:
Resolution:|   Keywords:
---+--

Comment (by gahr2):

 For completeness, I should say that this patch derived from JP Mens'
 original gist available at
 https://gist.github.com/jpmens/15969d9d678a3d450e4e

--
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [Mutt] #3691: Add LMDB support

[Mutt]

#3691: Add LMDB support
---+--
  Reporter:  rsc   |  Owner:  mutt-dev
  Type:  enhancement   | Status:  new
  Priority:  minor |  Milestone:
 Component:  header cache  |Version:
Resolution:|   Keywords:
---+--
Changes (by gahr2):

 * cc: gahr@… (added)


--
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [Mutt] #3691: Add LMDB support

[Mutt]

#3691: Add LMDB support
---+--
  Reporter:  rsc   |  Owner:  mutt-dev
  Type:  enhancement   | Status:  new
  Priority:  minor |  Milestone:
 Component:  header cache  |Version:
Resolution:|   Keywords:
---+--

Comment (by gahr2):

 Please consider the attached patch. Thank you.

--
Ticket URL: 
Mutt 
The Mutt mail user agent

[Mutt] #3691: Add LMDB support

[Mutt]

#3691: Add LMDB support
--+--
 Reporter:  rsc   |  Owner:  mutt-dev
 Type:  enhancement   | Status:  new
 Priority:  minor |  Milestone:
Component:  header cache  |Version:
 Keywords:|
--+--
 I would like to see LMDB support (http://symas.com/mdb/) in mutt in the
 future - as an additional alternative to BDB.

-- 
Ticket URL: http://dev.mutt.org/trac/ticket/3691
Mutt http://www.mutt.org/
The Mutt mail user agent