openlp-core team mailing list archive

[matysek <mzibricky@xxxxxxxxx>]

matysek has proposed merging lp:~mzibricky/openlp/bug-687638 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~mzibricky/openlp/bug-687638/+merge/129586

This merge contains some speed improvements for the locale aware sorting of songs. I did some performance testing for song sorting. Some timing in sec follows. Sorting songs:

- without locale support  0.012
- old implemntation:      0.115
- pyicu:                  0.013
- proposed changes:       0.013

According to numbers, we should be now with performance close to icu implementation.
-- 
https://code.launchpad.net/~mzibricky/openlp/bug-687638/+merge/129586
Your team OpenLP Core is requested to review the proposed merge of lp:~mzibricky/openlp/bug-687638 into lp:openlp.

=== modified file 'openlp/core/utils/__init__.py'
--- openlp/core/utils/__init__.py	2012-10-12 11:55:06 +0000
+++ openlp/core/utils/__init__.py	2012-10-14 11:24:26 +0000
@@ -498,8 +498,13 @@
     """
     # Function locale.strcol() from standard Python library does not work
     # properly on Windows and probably somewhere else.
-    return int(QtCore.QString.localeAwareCompare(
-        QtCore.QString(string1).toLower(), QtCore.QString(string2).toLower()))
+    return QtCore.QString.localeAwareCompare(string1.lower(), string2.lower())
+
+
+# For performance reasons provide direct reference to compare function
+# without wrapping it in another function making te string lowercase.
+# This is needed for sorting songs.
+locale_direct_compare = QtCore.QString.localeAwareCompare
 
 
 from languagemanager import LanguageManager
@@ -508,4 +513,5 @@
 __all__ = [u'AppLocation', u'get_application_version', u'check_latest_version',
     u'add_actions', u'get_filesystem_encoding', u'LanguageManager',
     u'ActionList', u'get_web_page', u'get_uno_command', u'get_uno_instance',
-    u'delete_file', u'clean_filename', u'format_time', u'locale_compare']
+    u'delete_file', u'clean_filename', u'format_time', u'locale_compare',
+    u'locale_direct_compare']

=== modified file 'openlp/plugins/songs/forms/songexportform.py'
--- openlp/plugins/songs/forms/songexportform.py	2012-10-12 11:55:06 +0000
+++ openlp/plugins/songs/forms/songexportform.py	2012-10-14 11:24:26 +0000
@@ -37,6 +37,7 @@
     create_separated_list
 from openlp.core.lib.ui import UiStrings, critical_error_message_box
 from openlp.core.ui.wizard import OpenLPWizard, WizardStrings
+from openlp.core.utils import locale_direct_compare
 from openlp.plugins.songs.lib.db import Song
 from openlp.plugins.songs.lib.openlyricsexport import OpenLyricsExport
 
@@ -251,7 +252,8 @@
         # Load the list of songs.
         Receiver.send_message(u'cursor_busy')
         songs = self.plugin.manager.get_all_objects(Song)
-        songs.sort()
+        songs.sort(
+            cmp=locale_direct_compare, key=lambda song: song.sort_string)
         for song in songs:
             # No need to export temporary songs.
             if song.temporary:

=== modified file 'openlp/plugins/songs/lib/db.py'
--- openlp/plugins/songs/lib/db.py	2012-10-12 13:00:56 +0000
+++ openlp/plugins/songs/lib/db.py	2012-10-14 11:24:26 +0000
@@ -31,11 +31,11 @@
 """
 
 from sqlalchemy import Column, ForeignKey, Table, types
-from sqlalchemy.orm import mapper, relation
+from sqlalchemy.orm import mapper, relation, reconstructor
 from sqlalchemy.sql.expression import func
+from PyQt4 import QtCore
 
 from openlp.core.lib.db import BaseModel, init_db
-from openlp.core.utils import locale_compare
 
 class Author(BaseModel):
     """
@@ -64,14 +64,22 @@
     """
     Song model
     """
-    # By default sort the songs by its title considering language specific
-    # characters.
-    def __lt__(self, other):
-        r = locale_compare(self.title, other.title)
-        return True if r < 0 else False
-
-    def __eq__(self, other):
-        return 0 == locale_compare(self.title, other.title)
+    def __init__(self):
+        self.sort_string = ''
+
+    # This decorator tells sqlalchemy to call this method everytime
+    # any data on this object are updated.
+    @reconstructor
+    def init_on_load(self):
+        """
+        Precompute string to be used for sorting.
+
+        Song sorting is performance sensitive operation.
+        To get maximum speed lets precompute the string
+        used for comparison.
+        """
+        # Avoid the overhead of converting string to lowercase and to QString
+        self.sort_string = QtCore.QString(self.title.lower())
 
 
 class Topic(BaseModel):

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2012-10-12 11:55:06 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2012-10-14 11:24:26 +0000
@@ -39,7 +39,7 @@
     check_directory_exists
 from openlp.core.lib.ui import UiStrings, create_widget_action
 from openlp.core.lib.settings import Settings
-from openlp.core.utils import AppLocation
+from openlp.core.utils import AppLocation, locale_direct_compare
 from openlp.plugins.songs.forms import EditSongForm, SongMaintenanceForm, \
     SongImportForm, SongExportForm
 from openlp.plugins.songs.lib import OpenLyrics, SongXML, VerseType, \
@@ -259,7 +259,8 @@
         log.debug(u'display results Song')
         self.saveAutoSelectId()
         self.listView.clear()
-        searchresults.sort()
+        searchresults.sort(
+            cmp=locale_direct_compare, key=lambda song: song.sort_string)
         for song in searchresults:
             # Do not display temporary songs
             if song.temporary: