Archive of RubyForge sup-talk mailing list
 help / color / mirror / Atom feed
* [sup-talk] [PATCH] maildir speedups
@ 2008-04-30 18:50 Ben Walton
  2008-05-20  3:29 ` William Morgan
  0 siblings, 1 reply; 11+ messages in thread
From: Ben Walton @ 2008-04-30 18:50 UTC (permalink / raw)


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>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [sup-talk] [PATCH] maildir speedups
  2008-04-30 18:50 [sup-talk] [PATCH] maildir speedups Ben Walton
@ 2008-05-20  3:29 ` William Morgan
       [not found]   ` <f96e0240805200924u1c991392oe36de853e208bab8@mail.gmail.com>
  0 siblings, 1 reply; 11+ messages in thread
From: William Morgan @ 2008-05-20  3:29 UTC (permalink / raw)


Reformatted excerpts from Ben Walton's message of 2008-04-30:
> 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 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>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [sup-talk] [PATCH] maildir speedups
       [not found]   ` <f96e0240805200924u1c991392oe36de853e208bab8@mail.gmail.com>
@ 2008-05-21  1:08     ` Ben Walton
  2008-05-25  2:34       ` William Morgan
  0 siblings, 1 reply; 11+ messages in thread
From: Ben Walton @ 2008-05-21  1:08 UTC (permalink / raw)


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
-- 
---------------------------------------------------------------------------------------------------------------------------
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: 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>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [sup-talk] [PATCH] maildir speedups
  2008-05-21  1:08     ` Ben Walton
@ 2008-05-25  2:34       ` William Morgan
  2008-05-29  2:49         ` Grant Hollingworth
  0 siblings, 1 reply; 11+ messages in thread
From: William Morgan @ 2008-05-25  2:34 UTC (permalink / raw)


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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [sup-talk] [PATCH] maildir speedups
  2008-05-25  2:34       ` William Morgan
@ 2008-05-29  2:49         ` Grant Hollingworth
  2008-05-29  3:05           ` Grant Hollingworth
  2008-05-29 13:34           ` Ben Walton
  0 siblings, 2 replies; 11+ messages in thread
From: Grant Hollingworth @ 2008-05-29  2:49 UTC (permalink / raw)


* William Morgan [2008-05-24 22:34 -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| !@ids.include?(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?


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [sup-talk] [PATCH] maildir speedups
  2008-05-29  2:49         ` Grant Hollingworth
@ 2008-05-29  3:05           ` Grant Hollingworth
  2008-05-29  3:06             ` Grant Hollingworth
  2008-05-29 13:34           ` Ben Walton
  1 sibling, 1 reply; 11+ messages in thread
From: Grant Hollingworth @ 2008-05-29  3:05 UTC (permalink / raw)


* Grant Hollingworth [2008-05-28 22:49 -0400]:
> Or (less pretty but faster and probably clearer)
> [...]

Best:

    @ids_to_fns = @ids_to_fns.values_at(@ids)

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [sup-talk] [PATCH] maildir speedups
  2008-05-29  3:05           ` Grant Hollingworth
@ 2008-05-29  3:06             ` Grant Hollingworth
  0 siblings, 0 replies; 11+ messages in thread
From: Grant Hollingworth @ 2008-05-29  3:06 UTC (permalink / raw)


* Grant Hollingworth [2008-05-28 23:05 -0400]:
> Best:
> 
>     @ids_to_fns = @ids_to_fns.values_at(@ids)
> 
> Next time I should check the docs first....

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [sup-talk] [PATCH] maildir speedups
  2008-05-29  2:49         ` Grant Hollingworth
  2008-05-29  3:05           ` Grant Hollingworth
@ 2008-05-29 13:34           ` Ben Walton
  2008-05-30 16:26             ` William Morgan
  1 sibling, 1 reply; 11+ messages in thread
From: Ben Walton @ 2008-05-29 13:34 UTC (permalink / raw)


On Wed, May 28, 2008 at 10:49 PM, Grant Hollingworth <grant at antiflux.org> wrote:
> * William Morgan [2008-05-24 22:34 -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| !@ids.include?(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?

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
-- 
---------------------------------------------------------------------------------------------------------------------------
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

---------------------------------------------------------------------------------------------------------------------------


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [sup-talk] [PATCH] maildir speedups
  2008-05-29 13:34           ` Ben Walton
@ 2008-05-30 16:26             ` William Morgan
  2008-06-04  0:39               ` Ben Walton
  0 siblings, 1 reply; 11+ messages in thread
From: William Morgan @ 2008-05-30 16:26 UTC (permalink / raw)


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>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [sup-talk] [PATCH] maildir speedups
  2008-05-30 16:26             ` William Morgan
@ 2008-06-04  0:39               ` Ben Walton
  2008-06-04  2:49                 ` William Morgan
  0 siblings, 1 reply; 11+ messages in thread
From: Ben Walton @ 2008-06-04  0:39 UTC (permalink / raw)


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

-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: 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>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [sup-talk] [PATCH] maildir speedups
  2008-06-04  0:39               ` Ben Walton
@ 2008-06-04  2:49                 ` William Morgan
  0 siblings, 0 replies; 11+ messages in thread
From: William Morgan @ 2008-06-04  2:49 UTC (permalink / raw)


Reformatted excerpts from Ben Walton's message of 2008-06-03:
> Ok, I took Grant's suggestion and just dropped the cleanup of the id
> -> filename mapping hash.

Merged right in. Thanks!
-- 
William <wmorgan-sup at masanjin.net>


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-06-04  2:49 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-04-30 18:50 [sup-talk] [PATCH] maildir speedups Ben Walton
2008-05-20  3:29 ` William Morgan
     [not found]   ` <f96e0240805200924u1c991392oe36de853e208bab8@mail.gmail.com>
2008-05-21  1:08     ` Ben Walton
2008-05-25  2:34       ` William Morgan
2008-05-29  2:49         ` Grant Hollingworth
2008-05-29  3:05           ` Grant Hollingworth
2008-05-29  3:06             ` Grant Hollingworth
2008-05-29 13:34           ` Ben Walton
2008-05-30 16:26             ` William Morgan
2008-06-04  0:39               ` Ben Walton
2008-06-04  2:49                 ` William Morgan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox