Mikhail Ramendik wrote:
Paul J Stevens wrote:
Please do share your ideas. Anything that will boost perfomance will
be seriously considered.
I've already implemented one performance booster. Well, at least for
MySQL. Want a patch against 2.0? Or should I pull CVS first? This is the
sole question that prevents me from posting a patch.
for (i = 0; i < db_num_rows(); i++) {
if (db_get_result(i, 1)[0] == '0')
mb->unseen++;
if (db_get_result(i, 2)[0] == '1')
mb->recent++;
mb->seq_list[i] = db_get_result_u64(i, 0);
}
db_free_result();
Well, with db_num_rows() in the tens of thousands one would expect a CPU
hog here! Three calls to db_get_result for every message - and
db_get_result performs seeking every single time, too.
And only to count the number of recent_flags, and seen_flags for a certain subset of message_idnrs all those
message rows are actually selected !! Clearly usage of COUNT() comes to mind as an optimization.
Actually one must also fill in mb-> seq_list, so one could count as well
- if db_get_result would not perform seeking and retrieving when it does
not have to!
I have this implemented already, and the loop now is through in no time.
Moreover, my version is probably faster than yours - because no extra
COUNT is needed, and because my db_get_result won't perform a single
seek in all this loop! It checks if the row we want is the next row from
the one already fetched, and if so, just fetches it!
SHow me the code. against 2.0 or cvs I don't really care. This change is so small and localized it doesn't
much matter. I too have fixed this, but I'm curious how you did it. I've attached my own patch.
I think that any writing of mime-parsers today is a waste of developer
time. There are many lying ready.
Which is why I started using gmime.
But, what has a mime-parser got to do with the number of queries here?
Joining all the queries, except the one for messageblks, into one or two
big queries for ALL messages can be implemented with no changes in the
mime parser, right? And even that will give us a 3 or 4 times better
performance.
When messages are retrieved by _ic_fetch, they are parsed by dbmail's mime-parser in many cases, like for
retrieving a list of the message-headers.
Unfortunately, this time (unlike db_get_result) I can't jump in and
implement it myself, because I don't understand the C code in _ic_fetch.
Ah. Well, I have a cleanup ready for merging. Nothing dramatic, just some extract to function stuff, and
dropping of some unnecessary variables and loops, etc. It does make it all more readable. More on that later.
--
________________________________________________________________
Paul Stevens mailto:[EMAIL PROTECTED]
NET FACILITIES GROUP PGP: finger [EMAIL PROTECTED]
The Netherlands________________________________http://www.nfg.nl
#! /bin/sh -e
## 03_db_get_mailbox.dpatch by <[EMAIL PROTECTED]>
##
## All lines beginning with `## DP:' are a description of the patch.
## DP: No description.
if [ $# -lt 1 ]; then
echo "`basename $0`: script expects -patch|-unpatch as argument" >&2
exit 1
fi
[ -f debian/patches/00patch-opts ] && . debian/patches/00patch-opts
patch_opts="${patch_opts:--f --no-backup-if-mismatch} ${2:+-d $2}"
case "$1" in
-patch) patch -p1 ${patch_opts} < $0;;
-unpatch) patch -R -p1 ${patch_opts} < $0;;
*)
echo "`basename $0`: script expects -patch|-unpatch as argument" >&2
exit 1;;
esac
exit 0
@DPATCH@
diff -urNad /usr/src/dbmail2/dbmail-2.1/db.c dbmail-2.1/db.c
--- /usr/src/dbmail2/dbmail-2.1/db.c 2004-10-23 22:59:50.000000000 +0200
+++ dbmail-2.1/db.c 2004-10-23 23:15:59.000000000 +0200
@@ -2323,37 +2323,41 @@
db_free_result();
- /* select messages */
+ /* count messages */
snprintf(query, DEF_QUERYSIZE,
- "SELECT message_idnr, seen_flag, recent_flag "
+ "SELECT COUNT(message_idnr), COUNT(message_idnr) -
SUM(seen_flag), SUM(recent_flag) "
"FROM %smessages WHERE mailbox_idnr = '%llu' "
- "AND status < '%d' "
- "ORDER BY message_idnr ASC",DBPFX, mb->uid,
MESSAGE_STATUS_DELETE);
+ "AND status < '%d' ",
+ DBPFX, mb->uid, MESSAGE_STATUS_DELETE);
if (db_query(query) == -1) {
- trace(TRACE_ERROR, "%s,%s: could not retrieve messages",
- __FILE__, __func__);
+ trace(TRACE_ERROR, "%s,%s: query error", __FILE__, __func__);
return -1;
}
- mb->exists = db_num_rows();
-
- /* alloc mem */
- mb->seq_list = (u64_t *) my_malloc(sizeof(u64_t) * mb->exists);
- if (!mb->seq_list) {
- /* out of mem */
+ mb->exists = db_get_result(0,0);
+ mb->unseen = db_get_result(0,1);
+ mb->recent = db_get_result(0,2);
+ db_free_result();
+
+ /* get messages */
+ snprintf(query, DEF_QUERYSIZE,
+ "SELECT message_idnr FROM %smessages WHERE mailbox_idnr =
'%llu' "
+ "AND status < '%d' ORDER BY message_idnr ASC",
+ DBPFX, mb->uid, MESSAGE_STATUS_DELETE);
+
+ if (db_query(query) == -1) {
+ trace(TRACE_ERROR, "%s,%s: query error [%s]", __FILE__,
__func__, query);
+ return -1;
+ }
+
+ if (! (mb->seq_list = (u64_t *) my_malloc(sizeof(u64_t) * mb->exists)))
{
db_free_result();
return -1;
}
-
- for (i = 0; i < db_num_rows(); i++) {
- if (db_get_result(i, 1)[0] == '0')
- mb->unseen++;
- if (db_get_result(i, 2)[0] == '1')
- mb->recent++;
-
+
+ for (i = 0; i < db_num_rows(); i++)
mb->seq_list[i] = db_get_result_u64(i, 0);
- }
db_free_result();
@@ -2361,20 +2365,15 @@
* NOTE expunged messages are selected as well in order to be
* able to restore them
*/
- snprintf(query, DEF_QUERYSIZE,
- "SELECT MAX(message_idnr) FROM %smessages",DBPFX);
+ snprintf(query, DEF_QUERYSIZE, "SELECT MAX(message_idnr)+1 FROM
%smessages",DBPFX);
if (db_query(query) == -1) {
- trace(TRACE_ERROR,
- "%s,%s: could not determine highest message ID",
- __FILE__, __func__);
+ trace(TRACE_ERROR, "%s,%s: query error [%s]", __FILE__,
__func__, query);
my_free(mb->seq_list);
mb->seq_list = NULL;
return -1;
}
-
- highest_id = db_get_result_u64(0, 0);
- mb->msguidnext = highest_id + 1;
+ mb->msguidnext = db_get_result_u64(0, 0);
db_free_result();
return 0;
