Speedup for deleting files

Speedup for deleting files

[David Bremner]

Thanks to discussion with Olly Betts and some perf runs, I realized
the current clean up of deleted files from the database is somewhat
wasteful since it modifies the message documents (by deleting the
filename) before in most cases deleting the whole record. While trying
this out, I triggered what seems to be a bug in existing notmuch; the
code checks for the existence of a certain Xapian term after deleting
the document.

Compared to git master [1], cleaning up 50k of 200k messages now takes
about 44s versus 80. So not quite a 50% improvement, but not bad. I
would expect a larger relative improvement on larger databases.

[1]: https://notmuchmail.org/perf-test-results/2023-07-18-minkowski/

[PATCH 1/2] lib/message: check message type before deleting document

[David Bremner]

It isn't really clear how this worked before. Traversing the terms of
a document after deleting it from the database seems likely to be
undefined behaviour at best
---
 lib/message.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 53f35dd1..46638f80 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1397,14 +1397,15 @@ _notmuch_message_delete (notmuch_message_t *message)
 	Xapian::PostingIterator thread_doc, thread_doc_end;
 	Xapian::PostingIterator mail_doc, mail_doc_end;
 
-	message->notmuch->writable_xapian_db->delete_document (message->doc_id);
-
 	/* look for a non-ghost message in the same thread */
 	/* if this was a ghost to begin with, we are done */
 	private_status = _notmuch_message_has_term (message, "type", "ghost", &is_ghost);
 	if (private_status)
 	    return COERCE_STATUS (private_status,
 				  "Error trying to determine whether message was a ghost");
+
+	message->notmuch->writable_xapian_db->delete_document (message->doc_id);
+
 	if (is_ghost)
 	    return NOTMUCH_STATUS_SUCCESS;
 
-- 
2.40.1

[PATCH 2/2] lib/n_d_remove_message: do not remove unique filename

[David Bremner]

It is wasteful to remove a filename term when the whole message
document is about to be removed from the database. Profiling with perf
shows this takes a significant portion of the time when cleaning up
removed files in the database.

The logic of n_d_remove_message becomes a bit more convoluted here in
order to make the change minimal.

It is possible that this function can be further optimized, since the
expansion of filename terms into filenames is probably not needed
here.
---
 lib/database.cc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/database.cc b/lib/database.cc
index 6987e2f4..737a3f30 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1454,7 +1454,9 @@ notmuch_database_remove_message (notmuch_database_t *notmuch,
 							&message);
 
     if (status == NOTMUCH_STATUS_SUCCESS && message) {
-	status = _notmuch_message_remove_filename (message, filename);
+	if (notmuch_message_count_files (message) > 1) {
+	    status = _notmuch_message_remove_filename (message, filename);
+	}
 	if (status == NOTMUCH_STATUS_SUCCESS)
 	    status = _notmuch_message_delete (message);
 	else if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID)
-- 
2.40.1

Re: Speedup for deleting files

[Tomi Ollila]

On Thu, Jul 20 2023, David Bremner wrote:

> Thanks to discussion with Olly Betts and some perf runs, I realized
> the current clean up of deleted files from the database is somewhat
> wasteful since it modifies the message documents (by deleting the
> filename) before in most cases deleting the whole record. While trying
> this out, I triggered what seems to be a bug in existing notmuch; the
> code checks for the existence of a certain Xapian term after deleting
> the document.
>
> Compared to git master [1], cleaning up 50k of 200k messages now takes
> about 44s versus 80. So not quite a 50% improvement, but not bad. I
> would expect a larger relative improvement on larger databases.
>
> [1]: https://notmuchmail.org/perf-test-results/2023-07-18-minkowski/

Series LGTM.

Tomi

Re: Speedup for deleting files

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

>
> Series LGTM.
>
> Tomi

Series applied to master.