We have two UI modes, with or without preview pane. In preview-pane mode on each keep_alive timeout is executed 'check-recent' action which searches for recent messages and sets unread counters on folder list, quota, etc. When we're working without preview pane on messages list we've got the same behaviour, but when switching to message view there executed is only 'keep-alive' action.
Proposition: Call 'check-recent'-like action instead of 'keep-alive'. We've got folders list which should be updated like in preview-pane mode.
We have also a performance issue here, because 'getunread' action is called after each message preview which increases load and slows down moving from one message to another. When working in preview-pane mode 'getunread' is called only on folder selection.
Proposition: Don't call 'genunread' action when 'show' action was called.
Does it make sens or I miss something?
Aleksander Machniak wrote:
We have two UI modes, with or without preview pane. In preview-pane mode on each keep_alive timeout is executed 'check-recent' action which searches for recent messages and sets unread counters on folder list, quota, etc. When we're working without preview pane on messages list we've got the same behaviour, but when switching to message view there executed is only 'keep-alive' action.
Proposition: Call 'check-recent'-like action instead of 'keep-alive'. We've got folders list which should be updated like in preview-pane mode.
This could be an option. I don't see any negative consequences here, except machine load.
We have also a performance issue here, because 'getunread' action is called after each message preview which increases load and slows down moving from one message to another. When working in preview-pane mode 'getunread' is called only on folder selection.
Proposition: Don't call 'genunread' action when 'show' action was called.
The unread count is not added to the message list directly because this can slow down the page-load (getting unread count of all mailboxes can take several seconds). To increase the page-load time the unread count is fetched after the page loaded in order to update the folder list. This is called whenever there's a folder list. If you omit that, the folder list does not display ANY unread count.
Does it make sens or I miss something?
Your first proposition is to check for recent count on message view page but your second proposition requests to omit the display of the unread count while on message view. This sounds like a contradiction to me.
~Thomas _______________________________________________ List info: http://lists.roundcube.net/dev/
Thomas Bruederli wrote:
Aleksander Machniak wrote:
We have two UI modes, with or without preview pane. In preview-pane mode on each keep_alive timeout is executed 'check-recent' action which searches for recent messages and sets unread counters on folder list, quota, etc. When we're working without preview pane on messages list we've got the same behaviour, but when switching to message view there executed is only 'keep-alive' action.
Proposition: Call 'check-recent'-like action instead of 'keep-alive'. We've got folders list which should be updated like in preview-pane mode.
This could be an option. I don't see any negative consequences here, except machine load.
But current behaviour is inconsistent. When you have a message opened (show action) you will be not informed when new mail will arrive, because keep-alive is not update counters on folders list. You must go back to messages list. When working with preview pane counters are updated, so I don't see problem to do this also in non-messages-list mode.
We have also a performance issue here, because 'getunread' action is called after each message preview which increases load and slows down moving from one message to another. When working in preview-pane mode 'getunread' is called only on folder selection.
Proposition: Don't call 'genunread' action when 'show' action was called.
The unread count is not added to the message list directly because this can slow down the page-load (getting unread count of all mailboxes can take several seconds). To increase the page-load time the unread count is fetched after the page loaded in order to update the folder list. This is called whenever there's a folder list. If you omit that, the folder list does not display ANY unread count.
Thomas, the point is that getunread is called too often when working without preview pane. It's called after each show action and that's performance issue what I'm talking about. When messages list is open only check-recent is called. So I think we can remove getunread calls also from show action. It's not problem for me, I use preview mode, but something is not right here. I can't explain better. My English not allows me to be more descriptive ;)
A.L.E.C wrote:
But current behaviour is inconsistent. When you have a message opened (show action) you will be not informed when new mail will arrive, because keep-alive is not update counters on folders list. You must go back to messages list. When working with preview pane counters are updated, so I don't see problem to do this also in non-messages-list mode.
I agree. We should call check-recent whenever this.gui_objects.mailboxlist is available. Currently it's only done when this.gui_objects.messagelist is there.
Thomas, the point is that getunread is called too often when working without preview pane. It's called after each show action and that's performance issue what I'm talking about. When messages list is open only check-recent is called. So I think we can remove getunread calls also from show action. It's not problem for me, I use preview mode, but something is not right here.
OK, then I suggest to use the cached messagecount value when loading the page and only call getunread if in list mode:
Index: program/include/main.inc
--- program/include/main.inc (revision 1296) +++ program/include/main.inc (working copy) @@ -1811,6 +1811,10 @@ } }
- // add unread message count display
- if ($unread = $IMAP->messagecount($folder['id'], 'UNSEEN', false))
$foldername .= sprintf(' (%d)', $unread);// make folder name safe for ids and class names $folder_id = preg_replace('/[^A-Za-z0-9\-_]/', '', $folder['id']); $class_name = preg_replace('/[^a-z0-9\-_]/', '', $folder_class ?
$folder_class : strtolower($folder['id'])); Index: program/js/app.js =================================================================== --- program/js/app.js (revision 1296) +++ program/js/app.js (working copy) @@ -226,7 +226,8 @@ if (this.gui_objects.mailboxlist) { this.gui_objects.folderlist = this.gui_objects.mailboxlist;
this.http_request('getunread', '');
if (!this.env.action)this.http_request('getunread', ''); } // ask user to send MDN
@@ -355,7 +356,7 @@ // start interval for keep-alive/recent_check signal this.start_keepalive = function() {
- if (this.env.keep_alive && !this.env.framed && this.task=='mail' &&
this.gui_objects.messagelist)
- if (this.env.keep_alive && !this.env.framed && this.task=='mail' &&
this.gui_objects.mailboxlist) this._int = setInterval(function(){ ref.check_for_recent(); }, this.env.keep_alive * 1000); else if (this.env.keep_alive && !this.env.framed && this.task!='login') this._int = setInterval(function(){ ref.send_keep_alive(); }, this.env.keep_alive * 1000);
This can make the page-load slow if one has disabled caching. Then building the folder list will request the unread count from IMAP for each folder. Maybe a if ($IMAP->caching_enabled) could help here.
What do you think? Could this change solve your problems? If yes, I'll commit the above changes to trunk.
~Thomas
List info: http://lists.roundcube.net/dev/
On Sun, Apr 13, 2008 at 8:02 PM, Thomas Bruederli roundcube@gmail.com wrote:
A.L.E.C wrote:
But current behaviour is inconsistent. When you have a message opened (show action) you will be not informed when new mail will arrive, because keep-alive is not update counters on folders list. You must go back to messages list. When working with preview pane counters are updated, so I don't see problem to do this also in non-messages-list mode.
I agree. We should call check-recent whenever this.gui_objects.mailboxlist is available. Currently it's only done when this.gui_objects.messagelist is there.
Thomas, the point is that getunread is called too often when working without preview pane. It's called after each show action and that's performance issue what I'm talking about. When messages list is open only check-recent is called. So I think we can remove getunread calls also from show action. It's not problem for me, I use preview mode, but something is not right here.
OK, then I suggest to use the cached messagecount value when loading the page and only call getunread if in list mode:
Index: program/include/main.inc
--- program/include/main.inc (revision 1296) +++ program/include/main.inc (working copy) @@ -1811,6 +1811,10 @@ } }
- // add unread message count display
- if ($unread = $IMAP->messagecount($folder['id'], 'UNSEEN', false))
$foldername .= sprintf(' (%d)', $unread);- // make folder name safe for ids and class names $folder_id = preg_replace('/[^A-Za-z0-9-_]/', '', $folder['id']); $class_name = preg_replace('/[^a-z0-9-_]/', '', $folder_class ?
$folder_class : strtolower($folder['id'])); Index: program/js/app.js =================================================================== --- program/js/app.js (revision 1296) +++ program/js/app.js (working copy) @@ -226,7 +226,8 @@ if (this.gui_objects.mailboxlist) { this.gui_objects.folderlist = this.gui_objects.mailboxlist;
this.http_request('getunread', '');
if (!this.env.action)this.http_request('getunread', ''); } // ask user to send MDN@@ -355,7 +356,7 @@ // start interval for keep-alive/recent_check signal this.start_keepalive = function() {
- if (this.env.keep_alive && !this.env.framed && this.task=='mail' &&
this.gui_objects.messagelist)
- if (this.env.keep_alive && !this.env.framed && this.task=='mail' &&
this.gui_objects.mailboxlist) this._int = setInterval(function(){ ref.check_for_recent(); }, this.env.keep_alive * 1000); else if (this.env.keep_alive && !this.env.framed && this.task!='login') this._int = setInterval(function(){ ref.send_keep_alive(); }, this.env.keep_alive * 1000);
This can make the page-load slow if one has disabled caching. Then building the folder list will request the unread count from IMAP for each folder. Maybe a if ($IMAP->caching_enabled) could help here.
What do you think? Could this change solve your problems? If yes, I'll commit the above changes to trunk.
~Thomas
What about only periodically checking for the "unread count" from the server - for example, only during "check new mail" and besides we just decrement the number "on read" (for example with the previewpane). I think Google Mail does it like that.
Till _______________________________________________ List info: http://lists.roundcube.net/dev/
Thomas Bruederli wrote:
OK, then I suggest to use the cached messagecount value when loading the page and only call getunread if in list mode:
Index: program/include/main.inc
--- program/include/main.inc (revision 1296) +++ program/include/main.inc (working copy) @@ -1811,6 +1811,10 @@ } }
- // add unread message count display
- if ($unread = $IMAP->messagecount($folder['id'], 'UNSEEN', false))
$foldername .= sprintf(' (%d)', $unread);- // make folder name safe for ids and class names $folder_id = preg_replace('/[^A-Za-z0-9-_]/', '', $folder['id']); $class_name = preg_replace('/[^a-z0-9-_]/', '', $folder_class ?
$folder_class : strtolower($folder['id'])); Index: program/js/app.js =================================================================== --- program/js/app.js (revision 1296) +++ program/js/app.js (working copy) @@ -226,7 +226,8 @@ if (this.gui_objects.mailboxlist) { this.gui_objects.folderlist = this.gui_objects.mailboxlist;
this.http_request('getunread', '');
if (!this.env.action)this.http_request('getunread', ''); } // ask user to send MDN@@ -355,7 +356,7 @@ // start interval for keep-alive/recent_check signal this.start_keepalive = function() {
- if (this.env.keep_alive && !this.env.framed && this.task=='mail'
&& this.gui_objects.messagelist)
- if (this.env.keep_alive && !this.env.framed && this.task=='mail'
&& this.gui_objects.mailboxlist) this._int = setInterval(function(){ ref.check_for_recent(); }, this.env.keep_alive * 1000); else if (this.env.keep_alive && !this.env.framed && this.task!='login') this._int = setInterval(function(){ ref.send_keep_alive(); }, this.env.keep_alive * 1000);
This can make the page-load slow if one has disabled caching. Then building the folder list will request the unread count from IMAP for each folder. Maybe a if ($IMAP->caching_enabled) could help here.
What do you think? Could this change solve your problems? If yes, I'll commit the above changes to trunk.
That solves keep_alive problem requested by me, but getunread issue is still not solved. Unseen counters are still readed on each message display and that's the performance issue what I'm talking about. But now I see the solution. Messageframe div in message.html should be an iframe. Then we can call getunread on first message display, but when moving to next/prev message (with firstmessage/previousmessage/nextmessage/lastmessage buttons) will be loaded only a message to the iframe. Folders list will be not touched and will waiting for check-recent event changes.
till wrote:
What about only periodically checking for the "unread count" from the server - for example, only during "check new mail" and besides we just decrement the number "on read" (for example with the previewpane). I think Google Mail does it like that.
This is exactly what RoundCube does. Only the unread count of the current folder is updated directly from the IMAP server. All other counts are read from cache. Also when marking a message as read, the new count is retrieved from IMAP and stored in cache again.
The 'getunread' command also reads the counts from cache. I don't understand why this should cause so much traffic. OK, it's an extra request causing an extra IMAP-Login.
~Thomas _______________________________________________ List info: http://lists.roundcube.net/dev/
A.L.E.C wrote:
That solves keep_alive problem requested by me, but getunread issue is still not solved. Unseen counters are still readed on each message display and that's the performance issue what I'm talking about. But now I see the solution. Messageframe div in message.html should be an iframe. Then we can call getunread on first message display, but when moving to next/prev message (with firstmessage/previousmessage/nextmessage/lastmessage buttons) will be loaded only a message to the iframe. Folders list will be not touched and will waiting for check-recent event changes.
Now you understand why all other webmail suites use frames...
However it looks to me like you disabled caching because otherwise the message counts are read from the local database (all in one record) and this should definitely not be a performance issue.
~Thomas _______________________________________________ List info: http://lists.roundcube.net/dev/
Thomas Bruederli wrote:
The 'getunread' command also reads the counts from cache. I don't understand why this should cause so much traffic. OK, it's an extra request causing an extra IMAP-Login.
With disabled cache or not it causes extra imap login, database connection and http request. If you agree with my opinion about iframe in message.html I'll try to make a patch.
A.L.E.C wrote:
With disabled cache or not it causes extra imap login, database connection and http request. If you agree with my opinion about iframe in message.html I'll try to make a patch.
Of course your patch fixes extra connection problem, but I think an iframe would be better solution.
-- Aleksander 'A.L.E.C' Machniak http://alec.pl gg:2275252 LAN Management System Developer http://lms.org.pl Roundcube Webmail Project Developer http://roundcube.net _______________________________________________ List info: http://lists.roundcube.net/dev/
A.L.E.C wrote:
Thomas Bruederli wrote:
The 'getunread' command also reads the counts from cache. I don't understand why this should cause so much traffic. OK, it's an extra request causing an extra IMAP-Login.
With disabled cache or not it causes extra imap login, database connection and http request. If you agree with my opinion about iframe in message.html I'll try to make a patch.
Sorry but I don't agree with iframes. We already use iframes in preview pane and address book and they only cause problems because they are very hard to synchronize with the main page.
When loading the message with an iframe we again have an extra http-request which opens an IMAP connection and reads from the database. So finally we won nothing with this change.
With my proposed solution we could at least get rid of that extra http-request when taking the message counts from cache directly when the page is built.
~Thomas _______________________________________________ List info: http://lists.roundcube.net/dev/
Thomas Bruederli wrote:
Sorry but I don't agree with iframes. We already use iframes in preview pane and address book and they only cause problems because they are very hard to synchronize with the main page.
When loading the message with an iframe we again have an extra http-request which opens an IMAP connection and reads from the database. So finally we won nothing with this change.
With my proposed solution we could at least get rid of that extra http-request when taking the message counts from cache directly when the page is built.
Yes, but it delays message preview. So, better to call getunread once after entering to message preview than call it on each preview. That would be the same as in three boxes mode. There getunread is not called on each message.
A.L.E.C wrote:
Thomas Bruederli wrote:
Sorry but I don't agree with iframes. We already use iframes in preview pane and address book and they only cause problems because they are very hard to synchronize with the main page.
When loading the message with an iframe we again have an extra http-request which opens an IMAP connection and reads from the database. So finally we won nothing with this change.
With my proposed solution we could at least get rid of that extra http-request when taking the message counts from cache directly when the page is built.
Yes, but it delays message preview. So, better to call getunread once after entering to message preview than call it on each preview. That would be the same as in three boxes mode. There getunread is not called on each message.
And what if people open a message to read, click back to list mode and select the next message? Then we doubled the requests to the server again.
We should rather investigate what really causes your performance problems. I cannot believe that reading those message counts does really use so much performance. At least it should not. Probably there's a major bug in RoundCube and we should try to find and fix it before we build some work around which will cause 10 new problems.
~Thomas _______________________________________________ List info: http://lists.roundcube.net/dev/
Thomas Bruederli wrote:
And what if people open a message to read, click back to list mode and select the next message? Then we doubled the requests to the server again.
Then will be as is now. We can't eliminate every issue, but we could improve performance of moving between messages (with arrow buttons). When you double click message on the list you switch to (let's say) preview mode and now you can move between messages with arrows. Those moves are causing excessive (in my opinion) getunread calls.
We should rather investigate what really causes your performance problems. I cannot believe that reading those message counts does really use so much performance. At least it should not. Probably there's a major bug in RoundCube and we should try to find and fix it before we build some work around which will cause 10 new problems.
I've got disabled caching because of "enable caching of messages and mailbox data in the local database. This is recommended if the IMAP server does not run on the same machine" (from config file). I have a big mailbox with several folders and getunread goes 2-3 secs. It not causes big load, but my goal is to eliminate excessive requests. What if you'll have a few hundreds of simultanous roundcube sessions?
In your proposal request is eliminated but message preview is delayed. The second reason of that discussion is inconsequent roundcube's behaviour (on the other hand it's consequent - getunread on each page load). Finally, I don't think it's a major issue, it's just one block of a whole structure.
A.L.E.C wrote:
Thomas Bruederli wrote:
And what if people open a message to read, click back to list mode and select the next message? Then we doubled the requests to the server again.
Then will be as is now. We can't eliminate every issue, but we could improve performance of moving between messages (with arrow buttons). When you double click message on the list you switch to (let's say) preview mode and now you can move between messages with arrows. Those moves are causing excessive (in my opinion) getunread calls.
I understand but I suggest to postpone this issue. One goal of the further development of RoundCube is to use more ajax-stuff to dynamically load components that are currently needed. Initially meant to define the plugin-API, we have a draft written by Brennan Stehling for the client-server communication and how the UI is divided into modules that can individually be filled with content. Making use of this, we could get rid of the full-page-loads as we have them right now and maybe even introduce a tabbed UI and this would certainly solve your issue.
But for now, we have to concentrate on cleaning up the code and then implementing the plugin-API.
For further reading, see http://trac.roundcube.net/wiki/RoundCube_vNext
List info: http://lists.roundcube.net/dev/
Thomas Bruederli wrote:
But for now, we have to concentrate on cleaning up the code and then implementing the plugin-API.
For further reading, see http://trac.roundcube.net/wiki/RoundCube_vNext
I only have a hope that this will not produce tons of requests from UI to backend.
OK, to close that thread I propose to right now apply only following fragment (as it was a bug):
@@ -355,7 +356,7 @@ // start interval for keep-alive/recent_check signal this.start_keepalive = function() {
- if (this.env.keep_alive && !this.env.framed && this.task=='mail' &&
this.gui_objects.messagelist)
- if (this.env.keep_alive && !this.env.framed && this.task=='mail' &&
this.gui_objects.mailboxlist) this._int = setInterval(function(){ ref.check_for_recent(); }, this.env.keep_alive * 1000); else if (this.env.keep_alive && !this.env.framed && this.task!='login') this._int = setInterval(function(){ ref.send_keep_alive(); }, this.env.keep_alive * 1000);
On Mon, Apr 14, 2008 at 9:15 AM, A.L.E.C alec@alec.pl wrote:
A.L.E.C wrote:
With disabled cache or not it causes extra imap login, database connection and http request. If you agree with my opinion about iframe in message.html I'll try to make a patch.
Of course your patch fixes extra connection problem, but I think an iframe would be better solution.
With Jquery it's easy to manipulate <div> containers as they were iframes. I am strongly against iframes. ;-) _______________________________________________ List info: http://lists.roundcube.net/dev/
-
A.L.E.C -
Aleksander Machniak
-
Thomas Bruederli -
till