ubuntu-touch-coreapps-reviewers team mailing list archive

[Roman Shchekin <mrqtros@xxxxxxxxx>]

Roman Shchekin has proposed merging lp:~mrqtros/ubuntu-rssreader-app/ubuntu-rssreader-app-startup-time into lp:ubuntu-rssreader-app.

Commit message:
Startup optimization.

Requested reviews:
  Ubuntu RSS Feed Reader Developers (ubuntu-rssreader-dev)

For more details, see:
https://code.launchpad.net/~mrqtros/ubuntu-rssreader-app/ubuntu-rssreader-app-startup-time/+merge/253769

Gentlemen, some time ago Joey noted, that startup time of our app is slow when database become big (2k+ records in his case). So I reworked it, and here is some hard data:

1. I achieved reduction (~10 times) of startup time by means of delayed DB access for separate topic (tabs). Memory usage is also reduced significantly. The cost of this change - first access to any topic become little bit slower (50 msec).
2. Improved initialization of "Shorts" tab in list mode (from 1396 msec to 56 msec, more than 20 times faster).

Someone should test it on a device.
Large DB can be found here (I hope there is nothing personal):
https://drive.google.com/open?id=0B07jhyL8-HtcNGpJMW9IcFo0R2VrUmwyYTlKQ1ZvZ25jZ3ZN&authuser=0 

-- 
Your team Ubuntu RSS Feed Reader Developers is requested to review the proposed merge of lp:~mrqtros/ubuntu-rssreader-app/ubuntu-rssreader-app-startup-time into lp:ubuntu-rssreader-app.

=== modified file 'BaseTab.qml'
--- BaseTab.qml	2015-02-04 20:49:10 +0000
+++ BaseTab.qml	2015-03-22 09:54:54 +0000
@@ -39,19 +39,26 @@
         }
     }
 
-    onIsListModeChanged: {
-        if (topicId == -1) // Ignore first undefined state.
+    onIsListModeChanged: modeChangeHandler()
+
+    function modeChangeHandler() {
+        if (topicId == -1 || !__isActive) // Ignore first undefined state.
             return
 
-        reload()
-    }
-
-    function reload() {
+        reloadInternal()
+    }
+
+    function reloadTab() {
+
+    }
+
+    function reloadInternal() {
         // console.log('baseTab reload', topicId)
         clear()
 
         var articlesByTopic = []
         var topicArticles = DB.loadArticles({"tagId" : topicId})
+        //.var topicArticles = { rows : { length : 0 } }
 
         for (var j = 0; j < topicArticles.rows.length; j++) {
             var ca = topicArticles.rows.item(j)
@@ -99,10 +106,11 @@
     }
 
     function showContent() {
-        //.console.log("showContent: ", topicId, isActive)
+        console.log("showContent: ", topicId, __isActive)
         if (__isActive)
             return
 
+        reloadInternal()
         __isActive = true
         if (isListMode)
             listPage.reload()

=== modified file 'SavedTab.qml'
--- SavedTab.qml	2015-02-04 20:49:10 +0000
+++ SavedTab.qml	2015-03-22 09:54:54 +0000
@@ -8,18 +8,17 @@
 BaseTab {
     id: aSavedTab
 
-    function reload() {
+    function reloadTab() {
         clear()
 
         var articlesByTopic = []
         var topicArticles = DB.loadFavouriteArticles()
-        var feedTags = DB.loadFeedTags()
 
         for (var j = 0; j < topicArticles.rows.length; j++) {
             var ca = topicArticles.rows.item(j)
 
             var anObj = {"tagName" : "",
-                "tagId" : getTagByFeed(feedTags, ca.feed_id),
+                "tagId" : ca.tag_id,
                 "title" : ca.title,
                 "content" : ca.content,
                 "link" : ca.link,
@@ -63,16 +62,6 @@
         else removeArticle(article)
     }
 
-    function getTagByFeed(feedTags, feedId) {
-        for (var j = 0; j < feedTags.rows.length; j++) {
-            var ft = feedTags.rows.item(j)
-            if (ft.feed_id == feedId)
-                return ft.tag_id
-        }
-
-        console.log("ERROR: getTagByFeed, feed without tag, feedId:", feedId)
-    }
-
     function removeArticle(article) {
         var commonModel = getModel()
         var articleId = article.id

=== modified file 'ShortsTab.qml'
--- ShortsTab.qml	2015-02-04 20:49:10 +0000
+++ ShortsTab.qml	2015-03-22 09:54:54 +0000
@@ -10,7 +10,14 @@
 
     readonly property int __listModeCount: 8
 
-    function reload() {
+    function modeChangeHandler() {
+        if (topicId == -1) // Ignore first undefined state.
+            return
+
+        reloadTab()
+    }
+
+    function reloadTab() {
         // console.time("Shorts model update")
         clear()
 
@@ -56,36 +63,27 @@
     }
 
     function reloadList() {
-        var tagArray = loadTags()
-
-        for (var i = 0; i < tagArray.length; i++) {
-            var curTag = tagArray[i]
-
-            var tagArticles = DB.loadArticles({"tagId" : curTag.id})
-
-            for (var j = 0; j < tagArticles.rows.length; j++) {
-                var ca = tagArticles.rows.item(j)
-
-                var articleObj = {"tagName" : curTag.name,
-                    "tagId" : curTag.id,
-                    "title" : ca.title,
-                    "content" : ca.content,
-                    "link" : ca.link,
-                    "author": ca.author,
-                    "description" : ca.description,
-                    "pubdate" : ca.pubdate,
-                    "status" : ca.status, // ?
-                    "favourite" : ca.favourite,
-                    "feedId" : ca.feed_id,
-                    "image" : ca.image,
-                    "feed_name" : ca.feed_name,
-                    "media_groups" : ca.media_groups,
-                    "id" : ca.id }
-
-                commonModelList.append(articleObj)
-                if (j >= __listModeCount)
-                    break
-            }
+        var tagArticles = DB.loadTagHighlights(__listModeCount)
+        for (var j = 0; j < tagArticles.rows.length; j++) {
+            var ca = tagArticles.rows.item(j)
+
+            var articleObj = {"tagName" : ca.tag_name,
+                "tagId" : ca.tag_id,
+                "title" : ca.title,
+                "content" : ca.content,
+                "link" : ca.link,
+                "author": ca.author,
+                "description" : ca.description,
+                "pubdate" : ca.pubdate,
+                "status" : ca.status, // ?
+                "favourite" : ca.favourite,
+                "feedId" : ca.feed_id,
+                "feed_name" : ca.feed_name,
+                "image" : ca.image,
+                "media_groups" : ca.media_groups,
+                "id" : ca.id }
+
+            commonModelList.append(articleObj)
         }
     }
 

=== modified file 'databasemodule_v2.js'
--- databasemodule_v2.js	2015-02-04 20:49:10 +0000
+++ databasemodule_v2.js	2015-03-22 09:54:54 +0000
@@ -79,7 +79,7 @@
     db.transaction(function (tx) {
         /* Check uniqueness.
          */
-        dbResult = tx.executeSql("SELECT * FROM feed WHERE source=?", [source])
+        dbResult = tx.executeSql("SELECT 1 FROM feed WHERE source=?", [source])
         if (dbResult.rows.length > 0) {
             console.log("Database, addFeed: already exist feed with source: ", source, "ID", dbResult.rows.item(0).id)
             dbResult = {"error": true, "exist": true}
@@ -240,6 +240,8 @@
     var db = openStdDataBase()
     var dbResult
 
+    console.log("loadArticles", JSON.stringify(params))
+
     db.transaction(function(tx) {
         if (params == undefined || params.isAll) // miss params
             dbResult = tx.executeSql('SELECT article.*, feed.title as feed_name FROM article inner join feed on article.feed_id = feed.id ORDER BY article.pubdate DESC')
@@ -291,7 +293,7 @@
     var dbResult
 
     db.transaction(function(tx) {
-        dbResult = tx.executeSql('SELECT article.*, feed.title as feed_name FROM article inner join feed on article.feed_id = feed.id WHERE article.favourite = "1" ORDER BY article.pubdate DESC' )
+        dbResult = tx.executeSql('SELECT article.*, feed.title as feed_name, feed_tag.tag_id FROM article inner join feed on article.feed_id = feed.id join feed_tag on feed_tag.feed_id = feed.id WHERE article.favourite = "1" ORDER BY article.pubdate DESC' )
     }
     )
     //console.log("loadFavouriteArticles", dbResult.rows.length)
@@ -299,6 +301,52 @@
     return dbResult;
 }
 
+// Load top for tag.
+function loadTagHighlights(size) {
+    var db = openStdDataBase()
+    var dbResult
+
+    db.transaction(function(tx) {
+        dbResult = tx.executeSql(  'select a.id, f.id as feed_id, ft.tag_id \
+                                    from article a \
+                                    inner join feed f on f.id = a.feed_id \
+                                    inner join feed_tag ft on ft.feed_id = f.id \
+                                    order by ft.tag_id, a.pubdate desc' )
+
+        var idArray = []
+
+        var curTagId = -1
+        var count = 0
+        for (var i = 0; i < dbResult.rows.length; i++) {
+            var c = dbResult.rows.item(i).tag_id
+
+            if (c != curTagId) {
+                count = 0
+                curTagId = c
+            }
+
+            if (count < size) {
+                idArray.push(dbResult.rows.item(i).id)
+                count++
+            }
+        }
+
+        var param = idArray.length ? idArray.join() : "-1" // Empty array guard.
+        dbResult = tx.executeSql('select a.*, f.id as feed_id, f.title as feed_name, t.id as tag_id, t.name as tag_name \
+                                 from article a \
+                                 inner join feed f on a.feed_id = f.id \
+                                 inner join feed_tag ft on ft.feed_id = f.id \
+                                 inner join tag t on t.id = ft.tag_id
+                                 where a.id in (%1) \
+                                 order by t.id, a.pubdate desc'.arg(param)) // Don't know why, but it doesn't work with commas in param
+
+        //.console.log(idArray.length, idArray)
+    }
+    )
+    //.console.log("loadTagHighlights", dbResult.rows.length)
+    return dbResult
+}
+
 // insert
 function addArticle(title, content, link, description, pubdate, guid, feed_id)
 {
@@ -309,7 +357,7 @@
 
         /* Check uniqueness.
          */
-        dbResult = tx.executeSql("SELECT * FROM article WHERE guid=?", [guid])
+        dbResult = tx.executeSql("SELECT 1 FROM article WHERE guid=?", [guid])
         if (dbResult.rows.length > 0) {
             console.log("Database, add article: already exist article with guid: ", guid)
             dbResult = {"error": true, "exist": true}
@@ -343,19 +391,19 @@
         for (var i = 0; i < model.length; i++) {
 
             article = model[i]
-            var title =  article.title == undefined ? "" : article.title
-            var guid =  article.guid == undefined ? Qt.md5(title) : article.guid
-            var link =  article.link == undefined ? "" : article.link
-            var pubDate =  article.pubDate == undefined ? "" : article.pubDate
-            var description =  article.description == undefined ? "" : article.description
-            var content =  article.content == undefined ? "" : article.content
-            var image =  article.image == undefined ? "" : article.image
-            var media_groups = article.media_groups == undefined ? "" : JSON.stringify(article.media_groups)
-            var author = article.author == undefined ? "" : JSON.stringify(article.author)
+            var title =  article.title ? article.title : ""
+            var guid =  article.guid ? article.guid : Qt.md5(title)
+            var link =  article.link ? article.link : ""
+            var pubDate =  article.pubDate ? article.pubDate : ""
+            var description =  article.description ? article.description : ""
+            var content =  article.content ? article.content : ""
+            var image =  article.image ? article.image : ""
+            var media_groups = article.media_groups ? JSON.stringify(article.media_groups) : ""
+            var author = article.author ? JSON.stringify(article.author) : ""
 
             /* Check uniqueness.
              */
-            dbResult = tx.executeSql("SELECT * FROM article WHERE guid=? AND feed_id=?", [guid, feed_id])
+            dbResult = tx.executeSql("SELECT 1 FROM article WHERE guid=? AND feed_id=?", [guid, feed_id])
             if (dbResult.rows.length > 0) {
                 // console.log("Database, add article: already exist article with guid: ", guid)
                 continue;
@@ -503,7 +551,7 @@
 
         /* Check uniqueness.
          */
-        dbResult = tx.executeSql("SELECT * FROM tag WHERE name=?", [name])
+        dbResult = tx.executeSql("SELECT 1 FROM tag WHERE name=?", [name])
         if (dbResult.rows.length > 0) {
             console.log("Database, add tag: already exist tag with source: ", name)
             dbResult = {"error": true, "exist": true}
@@ -593,7 +641,7 @@
 
         /* Check uniqueness.
          */
-        dbResult = tx.executeSql("SELECT * FROM feed_tag WHERE feed_id=? AND tag_id=? ", [feed_id, tag_id])
+        dbResult = tx.executeSql("SELECT 1 FROM feed_tag WHERE feed_id=? AND tag_id=? ", [feed_id, tag_id])
         if (dbResult.rows.length > 0) {
             console.log("Database, add feed_tag: already exist feed_tag with source: ", feed_id, tag_id)
             return {"error": true, "exist": true};

=== modified file 'shorts-app.qml'
--- shorts-app.qml	2015-01-21 19:01:20 +0000
+++ shorts-app.qml	2015-03-22 09:54:54 +0000
@@ -36,7 +36,6 @@
     footerColor: "#9b616c"
 
     Component.onCompleted: {
-
         // Update database if the db scheme is old.
         var dbv = parseFloat(optionsKeeper.dbVersion())
         console.log("db version: " , dbv)
@@ -74,8 +73,8 @@
             /* Update if "isListView" set will be done automatically.
              */
             if (!pageStack.isListView) {
-                shortsTab.reload()
-                savedTab.reload()
+                shortsTab.reloadTab()
+                savedTab.reloadTab()
             }
         }
     }
@@ -112,10 +111,10 @@
         tagId = tagId || 0
 
         if (tagId == 0 || !repeater.containsTopic(tagId)) {
-            shortsTab.reload()
+            shortsTab.reloadTab()
             repeater.reloadTabs()
         } else {
-            shortsTab.reload()
+            shortsTab.reloadTab()
             repeater.reloadOneTopic(tagId)
         }
 
@@ -124,7 +123,7 @@
 
     // refresh "Saved" Tab
     function refreshSavedTab() {
-        savedTab.reload()
+        savedTab.reloadTab()
     }
 
     function showArticle(model, index) {      //   go to single article page
@@ -364,7 +363,7 @@
                     for (var i = 0; i< tags.rows.length; i++) {
                         repeater.itemAt(i).title = tags.rows.item(i).name
                         repeater.itemAt(i).topicId = tags.rows.item(i).id
-                        repeater.itemAt(i).reload()
+                        repeater.itemAt(i).reloadTab()
                     }
                 }
 
@@ -380,7 +379,7 @@
                 function reloadOneTopic(topicId) {
                     for(var i = 0; i < repeater.count; i++) {
                         if (repeater.itemAt(i).topicId == topicId) {
-                            repeater.itemAt(i).reload()
+                            repeater.itemAt(i).reloadTab()
                             break
                         }
                     }
@@ -602,7 +601,7 @@
 
         onFeedEdit: {
             console.log("onFeedEdit: ", topicId)
-            shortsTab.reload()
+            shortsTab.reloadTab()
             repeater.reloadOneTopic(topicId)
             tabstabs.selectTopic(topicId)
         }