Patch to use imap_root before imap connect (significant speedup)

[ziba]

By communicating the roundcube imap_root preference to iloha imap before connecting to imap, we can take advantage of iloha's ability to skip the namespace discovery phase.

In an environment like ours (80K users, 900K mailboxes), this cuts the time it takes the backend to load a single email message from ~1.7 seconds to ~0.2 seconds (on an unloaded test system).

Please let me know if I can change anything to make this patch more acceptable. Thanks.

[maharaja]

two things

# if rcmail::get_instance()->config->get('imap_root') gives an empty string, the patch renders roundcube unusable. # normally i am against the usage of globals in such a way. but this is better than nothing :)

[maharaja]

updated patch

[ziba]

v2 is missing a semicolon

[alec]

Two cons: this will not work with empty (NIL) imap_root, also setting this will cause that delimiter will be not read

[ziba]

Thanks for looking into this. I've restated the problems to make sure that I understand them and offered some fixes. Let me know what you think and I can remake the patch.

problem:

Setting myprefs["rootdir"] to save the time spent determining rootdir prevents conn->delimiter from being populated. However, determining the delimiter is done with the exact same potentially time consuming process.

possible fix:

Add myprefs["delimiter"] preference to iloha mail which is set from a new roundcube preference: imap_delimiter. Require imap_root and imap_delimiter to be set together if set at all

problem:

patch does not work with empty string as rootdir

possible fix:

Explicitly check for and allow empty string

in rcube_imap.php: if ($imap_root || $imap_root === "") {

in imap.php: if ($my_prefs["rootdir"] || $my_prefs["rootdir"] === "") {

[alec]

Sounds OK, with one note. Will be better to check imap_root with is_string(), then in config we can just set those options to FALSE or NULL. Also I've found that setting imap_delimiter will give as another performance upswing (one unnecessary LIST command will be ommited). Make a patch. I'm also curious how imap_delimiter option will improve performance on heavy load. Give some numbers here, please.

[ziba]

now with delimiter option and is_string checking

[ziba]

I've made the discussed patch changes and added config/main.inc.php.dist values (you might want different defaults). Let me know if I can make any more changes. We're already using the earlier version of this patch in production, so I can't turn it off and give you numbers for the impact on a loaded system. On an unloaded machine, the benefit for us (a bit of a weird case) is ~1.5 seconds every imap connect.

Thanks.

[alec]

With your previous patch delimiter was set using LIST command, when imap_delimiter was added in your patch last version, LIST command is not executed. I was talking about that effect. Is that change noticeable?

[alec]

Applied with some changes in r2143.

[maharaja]

minor correction to UPGRADING, line 21:

WARNING: If you don't know what an "IMAP root directory" is, set the imap_root option to NULL.

[ziba]

Sorry. I misunderstood your question about imap delimiter fetch time. It's not in production yet, but on an unloaded system it takes less than 0.001 seconds to fetch the delimiter from the imapproxy that runs on the webserver. I would expect similar results in production.

[alec]

... and r2149 (lost commit in rcube_imap.inc).