User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.15) Gecko/2009101601 Firefox/2.0.0.18 X-ORACLE-DEBUG=STATS (.NET CLR 3.5.30729) Build Identifier: Using the cache for many calendars can make Thunderbird startup time really slow and cpu intensive, I've investigated this with a 0.9 build and found that simply adding indexes would help a lot, one user went from a 45 seconds startup time to a 7 seconds startup time. I added the indexes in initDB of calStorageCalendar.js , they were done based on the statements i saw in that file so they might now all yield the same speed improvement, creating events has shown no noticeable slowdowns. I currently do not have time to provide a proper patch for 1.0 so I'm dumping the indexes i added in 0.9 here: this.mDB.executeSimpleSQL( "CREATE INDEX IF NOT EXISTS " + "idx_cal_properties_cal_id ON cal_properties(cal_id);" ); this.mDB.executeSimpleSQL( "CREATE INDEX IF NOT EXISTS " + "idx_cal_event_cal_id ON cal_events(cal_id);" ); this.mDB.executeSimpleSQL( "CREATE INDEX IF NOT EXISTS " + "idx_cal_event_item_id ON cal_events(cal_id, id);" ); this.mDB.executeSimpleSQL( "CREATE INDEX IF NOT EXISTS " + "idx_cal_attendees_item_id ON cal_attendees(cal_id,item_id);" ); this.mDB.executeSimpleSQL( "CREATE INDEX IF NOT EXISTS " + "idx_cal_recurrence_item_id ON cal_recurrence(cal_id,item_id);" ); this.mDB.executeSimpleSQL( "CREATE INDEX IF NOT EXISTS " + "idx_cal_metadata_item_id ON cal_metadata(cal_id,item_id);" ); this.mDB.executeSimpleSQL( "CREATE INDEX IF NOT EXISTS " + "idx_cal_metadata_cal_id ON cal_metadata(cal_id);" ); this.mDB.executeSimpleSQL( "CREATE INDEX IF NOT EXISTS " + "idx_cal_attendees_recurrence_id ON cal_attendees(cal_id,item_id,recurrence_id);" ); this.mDB.executeSimpleSQL( "CREATE INDEX IF NOT EXISTS " + "idx_cal_properties_recurrence_id ON cal_properties(cal_id,item_id,recurrence_id);" ); this.mDB.executeSimpleSQL( "CREATE INDEX IF NOT EXISTS " + "idx_cal_events_recurrence_id ON cal_events(cal_id,id,recurrence_id);" ); Reproducible: Always

Yeah, we need these. I've always procrastinated on this, thinking we could just do this with the storage2 improvements, but maybe we should do this sooner. Even though this could well be done outside of the updater, I think we should spend a dedicated storage db version for this type of upgrade.

This patch takes care. I've used a slightly different set of indexes, based on our SELECT queries. There is still room for optimization, we could take a closer look at our queries with EXPLAIN. Especially the long ranged queries from cal_events and cal_todos might be optimizable when adding indexes to event_start/event_end, but I'd like to take a closer look at that some other time, since its not crystal clear which fields to index over best.

Comment on attachment 414007 [details] [diff] [review] Fix - v1 Argh, aside from forgetting to remove creating the old index, this patch doesn't create indexes on new profiles. Back to the drawing board. I'd still appreciate if you could take a look to see if I'm going in the right direction.

This takes care. I've re-used the table-data object to contain the index data too. Just relying on the prefix of the table name (i.e "idx_") might not be the best solution, but it fits in well with the current implementation, and its quite unlikely that someone will name his table idx_*. I've also documented this info.

One downside may be that someone upgrading between 0.9 and 1.0pre with a lot of events might expierence a one-time slow startup. We should either relnote this or add some debug output like "Indexing calendar data. This may take a while, but will not happen again". The latter won't be localized though, and it might not be worth it either.

Comment on attachment 414027 [details] [diff] [review] Fix - v2 >diff --git a/calendar/lightning/install.rdf b/calendar/lightning/install.rdf >--- a/calendar/lightning/install.rdf >+++ b/calendar/lightning/install.rdf [...] Changing this file seems to be wrong for this patch! >diff --git a/calendar/providers/storage/calStorageCalendar.js b/calendar/providers/storage/calStorageCalendar.js >--- a/calendar/providers/storage/calStorageCalendar.js >+++ b/calendar/providers/storage/calStorageCalendar.js [...] We should restrict the SELECT at http://mxr.mozilla.org/comm-central/source/calendar/providers/storage/calStorageCalendar.js#926 to |icalString| to opitmize the query. >diff --git a/calendar/providers/storage/calStorageUpgrade.jsm b/calendar/providers/storage/calStorageUpgrade.jsm >--- a/calendar/providers/storage/calStorageUpgrade.jsm >+++ b/calendar/providers/storage/calStorageUpgrade.jsm [...] >+/** >+ * Helper function to create an index on the database if it doesn't already >+ * exist. >+ * >+ * @param db The database to create the index on. >+ * @param idxName The index name. >+ * @param idxOn The part after the ON in CREATE INDEX ... ON. >+ */ >+function createIndex(tblData, tblName, colNameArray, db) { The |@param|s don't match the arguments of the function! >+ // Alarms, Attachments, Attendees, Relations >+ for each (let tblName in ["alarms", "attachments", "attendees", "relations"]) { >+ createIndex(tbl, "cal_" + tblName, ["cal_id", "item_id", "recurrence_id"], db); >+ createIndex(tbl, "cal_" + tblName, ["cal_id", >+ "item_id", >+ "recurrence_id", >+ "recurrence_id_tz"], db); >+ } The second index should be enough because it contains the first one (after reading http://www.sqlite.org/optoverview.html). Please, also fix the alignment! >+ // Metadata >+ createIndex(tbl, "cal_metadata", ["cal_id"], db); >+ createIndex(tbl, "cal_metadata", ["item_id", "cal_id"], db); After changing the order in WHERE for http://mxr.mozilla.org/comm-central/source/calendar/providers/storage/calStorageCalendar.js#909, we can use |createIndex(tbl, "cal_metadata", ["cal_id", "item_id"], db);| and need only one index. >+ // Properties. To align naming, we first delete the old index here. >+ executeSimpleSQL(db, "DROP INDEX IF EXISTS idx_cal_properies_item_id"); >+ createIndex(tbl, "cal_properties", ["cal_id", "item_id"], db); >+ createIndex(tbl, "cal_properties", ["cal_id", >+ "item_id", >+ "recurrence_id", >+ "recurrence_id_tz"], db); The second index should be enough because it contains the first one. Please, also fix the alignment! r=mschroeder... without much testing (maybe I make up for it tomorrow), but we need it for beta (where we could get the best testing ;) ).

All comments taken care of and did another round of testing. Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/bb7e22c8d601> and comm-1.9.1 <http://hg.mozilla.org/releases/comm-1.9.1/rev/b6886ae54d66> -> FIXED

These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".