Cor Bosman wrote:

...

So, here's the patch which joins 1. and 2. into one command call and joins all fetches from 3. into one command call. Speed up is significant for all messages, specially for those having many attachments. Imagine a message with ten attachments. Previously there was ca.10 + 3 fetches * 0.2 sec. (in my mailbox with ca.800 messages) = 2.6 sec. Now, only 3 fetches * 0.2 = 0.6 sec.

This is only for attachments that are shown immediately like images right?

No, not only.

Cor Bosman wrote:

Would it be an idea to only immediately fetch the attachments that

can > be shown, and only fetch others if the reader clicks on them?

Attachments are only a (second) part of my patch. There are fetches for headers of attachment parts not the body of attachments. I'm not sure if we can skip those fetches (it's one FETCH command after patching) when creating the message structure.

If we are talking about displaying message parts (attachments), yes, there's a place for more improvements.

Thomas Bruederli wrote:

It looks good so far. With this changes we'll always fetch the BODYSTRUCTURE when reading the message headers. Well this is actually some overhead, especially when just listing a mailbox. The message structure is only needed when a message is displayed and the new mechanism will probably fetch too much data that is never used. It reduces the number of fetches but one FETCH command will probably take longer because the bodystructure has do be created by the IMAP server.

Don't get me wrong, these are just some worst-case thoughts and I didn't do any speed-tests on my environment so far. I just question your calculations about the performance improvements. The 10+3 fetches on a 800 message mailbox only occur if you open all of those mails but not when they're just listed.

You're right, but my speed-tests don't confirm your concerns. There's no difference on my mailbox (pagesize=100, 7000 messages in folder) using dovecot-1.0. Fetch for headers of a hundred messages tooks 0.2 to 0.3 sec. (with or without bodystructure). For pagesize=1000 it's ca.0.5 sec. and then the difference is more noticeable (up to 20%).

We can skip bodystructure in fetch for messages listing, but it will require additional fetch when displaying message and cache is enabled. So, in my opinion, it will be simpler to do more performance tests and use the patch as is.

Next version with small fix. Please, someone check the patch with enabled caching. In my opinion, it can be commited to trunk now.

A.L.E.C wrote:

Next version with small fix. Please, someone check the patch with enabled caching. In my opinion, it can be commited to trunk now.

Well, I'm still not convinced about the bodystructure stuff in headers. Especially if you have caching disabled, all headers need to be fetched when listing a folder. This IMO is too much overhead just to safe one FETCH command in the case one opens a message.

You made the bodystructure fetch optional in iil_C_FetchHeader() so why not add a thirg argument to rcube_imap::get_headers() which defaults to false (for listing) but will be used in rcube_message class.

Let's check this for caching enabled and disabled:

Caching disabled:

• headers are fetched for listing in one FETCH command
• headers + bodystructure are fetched together when viewing the message
• part headers are fetched in an additional FETCH command

-> list + view totals in 3 FETCH commands

(even if we always fetch the bodystructure there won't be less fetch commands because caching is disabled)

Caching enabled:

• headers are fetched for listing in one FETCH command
• headers are cached, bodystructure needs to be fetched when viewing msg.
• part headers are fetched in an additional FETCH command

-> list + view totals in 3 FETCH commands (of not cached yet)

(once the bodystructure is fetched it is also saved in cache)

My conclusion: making the bodystructure optional in rcube_imap::get_headers() and use it in rcube_message::__construct() would be a reasonable improvement.

~Thomas _______________________________________________ List info: http://lists.roundcube.net/dev/

A.L.E.C wrote:

You mean 4th argument? From conclusions below you'd see that get_headers() isn't used for messages listing (list_headers() -> _fetch_headers() is used)

Yes, it's the 4th argument. But I'd add it anyway since rcube_imap::get_headers() is also used in other places where it does not make sense to fetch the structure (check_recent.inc, viewsource.inc, rcube_imap::get_structure(), rcube_imap::get_body())

~Thomas _______________________________________________ List info: http://lists.roundcube.net/dev/