Subject: [sup-talk] [PATCH] maildir speedups

From: [email-masked] (Ben Walton)
Date: Wed, 30 Apr 2008 14:50:52 -0400

Hi All,

The attached small patch improves maildir performance by making the
assumption that nothing will be modifying the underlying files
themselves.  It uses the mtimes of cur/ and new/ to mark whether or
not to poll files in the directory.  This sees the initial poll be a
somewhat slow process (nfs for me again), but subsequent polls are
much faster.  I'm caching the mtimes in sources.yaml too, but
currently this doesn't save anything across loads of sup since the
id's (and id -> fn map) are not cached across restarts.

I hope you find this useful.
-Ben
-- 
---------------------------------------------------------------------------------------------------------------------------
Ben Walton <bdwalton at gmail.com>

When one person suffers from a delusion, it is called insanity. When
many people suffer from a delusion it is called Religion.
Robert M. Pirsig, Zen and the Art of Motorcycle Maintenance

---------------------------------------------------------------------------------------------------------------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-maildir-speed-improvements.patch
Type: text/x-patch
Size: 4256 bytes
Desc: not available
URL: <http://rubyforge.org/pipermail/sup-talk/attachments/20080430/1ba57542/attachment-0001.bin>

---

From: [email-masked] (William Morgan)
Date: Mon, 19 May 2008 20:29:21 -0700

Reformatted excerpts from Ben Walton's message of 2008-04-30:
> not to poll files in the directory.

This sounds great. My only comment is that the Logger::log stuff should
probably be Redwood::log. If you make that change I'm happy to apply.
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Ben Walton)
Date: Tue, 20 May 2008 21:08:48 -0400

Ok, the attached patch should address the use of Logger::log vs
Redwood::log.  Additionally, I changed the names of the cached mtimes
and corrected a bug that saw new sources not work correctly.

I hope you find this acceptable (and useful).

-Ben

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-maildir-speedups.patch
Type: text/x-diff
Size: 4549 bytes
Desc: not available
URL: <http://rubyforge.org/pipermail/sup-talk/attachments/20080520/648ed94d/attachment.bin>

---

From: [email-masked] (William Morgan)
Date: Sat, 24 May 2008 19:34:03 -0700

Published on 'maildir-speedups' and merged into next. Thanks!
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Grant Hollingworth)
Date: Wed, 28 May 2008 22:49:32 -0400

> Published on 'maildir-speedups' and merged into next. Thanks!

Sup had my CPU working overtime after I updated. I ran ruby-prof and found
that the line

    @ids_to_fns.delete_if { |k, v| [email-masked]?(k) }

in Maildir#scan_mailbox was the culprit.

Those id lists are pretty long and include? means comparing each id (a Bignum)
in @ids_to_fns with every id in @ids.

A faster method is

    @ids_to_fns = @ids.inject({}) do |hash, i|
      hash[i] = @ids_to_fns[i]
      hash
    end

Or (less pretty but faster and probably clearer)

    new_ids_to_fns = {}
    @ids.each {|i| new_ids_to_fns[i] = @ids_to_fns[i] }
    @ids_to_fns = new_ids_to_fns

But I guess the real question is whether the line is even needed. There
probably won't be a big difference between @ids_to_fns.keys and @ids, so why
not leave some extra values in the hash?

---

From: [email-masked] (Grant Hollingworth)
Date: Wed, 28 May 2008 23:05:04 -0400

* Grant Hollingworth [2008-05-28 22:49 -0400]:
> [...]

Best:

    @ids_to_fns = @ids_to_fns.values_at(@ids)

Next time I should check the docs first....

---

From: [email-masked] (Grant Hollingworth)
Date: Wed, 28 May 2008 23:06:46 -0400

> Next time I should check the docs first....

Ugh. Never mind. Next time I should get some sleep before posting crap to the
list.

---

From: [email-masked] (Ben Walton)
Date: Thu, 29 May 2008 09:34:09 -0400

> not leave some extra values in the hash?

In hindsight, that seems rather obvious, although I've not noticed the
cpu consumption on my box...Either of the above to fixes would work.
I had a reason for explicitly purging old records from the mapping
when I created this, but it escapes me now...dropping that line may be
acceptable unless there is a reason that the keys in @ids_to_fns
should be a 1:1 mapping to values in @ids.  Really, there shouldn't be
'extra' keys in @ids_to_fns unless a message disappears from the
Maildir anyway, which should cause an OutOfSync exception, no?

Thanks
-Ben

---

From: [email-masked] (William Morgan)
Date: Fri, 30 May 2008 16:26:30 +0000

Reformatted excerpts from Ben Walton's message of 2008-05-29:
> Really, there shouldn't be 'extra' keys in @ids_to_fns unless a
> message disappears from the Maildir anyway, which should cause an
> OutOfSync exception, no?

I believe this is the case. I'm happy to take a patch when you guys (who
actually use Maildir!) are ready. :)
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Ben Walton)
Date: Tue, 3 Jun 2008 20:39:05 -0400

Ok, I took Grant's suggestion and just dropped the cleanup of the id
-> filename mapping hash.

-Ben

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-maildir-cpu-usage-regression-fix.patch
Type: text/x-diff
Size: 1048 bytes
Desc: not available
URL: <http://rubyforge.org/pipermail/sup-talk/attachments/20080603/adff285d/attachment-0001.bin>

---

From: [email-masked] (William Morgan)
Date: Tue, 03 Jun 2008 19:49:02 -0700

> -> filename mapping hash.

Merged right in. Thanks!