Faster sorting of CFileItemLists, step 2

Each time a CFileItemList is sorted, multiple SortItem objects are
constructed and destructed. Each object is a std::map ,implemented as a
red-black tree with one node for each attribute (Field) upon which the
CFileItemList could conceivably have been sorted, even though for any given
SortBy, very few attributes are actually examined by SortUtils::Sort(). Worse
still, many of those nodes involve creating an associated fresh copy of a
string, with all the associated heap thrashing that entails.

This patch changes the various ToSortable methods to limit the number of
items in each SortItem's std::map to those actually required for performing
the chosen sort.

Benchmarks for CFileItemList::Sort() on a Raspberry Pi with a library of
600 movies:
Before patch: 1.1 seconds
After patch:  0.36 seconds

diff --git a/xbmc/FileItem.cpp b/xbmc/FileItem.cpp
index 25c22c3..ceba484 100644 (file)
--- a/xbmc/FileItem.cpp
+++ b/xbmc/FileItem.cpp
@@ -699,45 +699,57 @@ void CFileItem::Serialize(CVariant& value) const
     (*m_pictureInfoTag).Serialize(value["pictureInfoTag"]);
 }
 
-void CFileItem::ToSortable(SortItem &sortable)
-{
-  sortable[FieldPath] = m_strPath;
-  sortable[FieldDate] = (m_dateTime.IsValid()) ? m_dateTime.GetAsDBDateTime() : "";
-  sortable[FieldSize] = m_dwSize;
-  sortable[FieldDriveType] = m_iDriveType;
-  sortable[FieldStartOffset] = m_lStartOffset;
-  sortable[FieldEndOffset] = m_lEndOffset;
-  sortable[FieldProgramCount] = m_iprogramCount;
-  sortable[FieldBitrate] = m_dwSize;
-  sortable[FieldTitle] = m_strTitle;
-  sortable[FieldSortSpecial] = m_specialSort;
-  sortable[FieldFolder] = m_bIsFolder;
-
+void CFileItem::ToSortable(SortItem &sortable, Field field) const
+{
+  switch (field)
+  {
+  case FieldPath:         sortable[FieldPath] = m_strPath; break;
+  case FieldDate:         sortable[FieldDate] = (m_dateTime.IsValid()) ? m_dateTime.GetAsDBDateTime() : ""; break;
+  case FieldSize:         sortable[FieldSize] = m_dwSize; break;
+  case FieldDriveType:    sortable[FieldDriveType] = m_iDriveType; break;
+  case FieldStartOffset:  sortable[FieldStartOffset] = m_lStartOffset; break;
+  case FieldEndOffset:    sortable[FieldEndOffset] = m_lEndOffset; break;
+  case FieldProgramCount: sortable[FieldProgramCount] = m_iprogramCount; break;
+  case FieldBitrate:      sortable[FieldBitrate] = m_dwSize; break;
+  case FieldTitle:        sortable[FieldTitle] = m_strTitle; break;
+  case FieldSortSpecial:  sortable[FieldSortSpecial] = m_specialSort; break;
+  case FieldFolder:       sortable[FieldFolder] = m_bIsFolder; break;
   // If there's ever a need to convert more properties from CGUIListItem it might be
   // worth to make CGUIListItem  implement ISortable as well and call it from here
-  sortable[FieldLabel] = GetLabel();
+  default: break;
+  }
 
   if (HasMusicInfoTag())
-    GetMusicInfoTag()->ToSortable(sortable);
-    
+    GetMusicInfoTag()->ToSortable(sortable, field);
+
   if (HasVideoInfoTag())
   {
-    GetVideoInfoTag()->ToSortable(sortable);
+    GetVideoInfoTag()->ToSortable(sortable, field);
 
     if (GetVideoInfoTag()->m_type == "tvshow")
     {
-      if (HasProperty("totalepisodes"))
+      if (field == FieldNumberOfEpisodes && HasProperty("totalepisodes"))
         sortable[FieldNumberOfEpisodes] = GetProperty("totalepisodes");
-      if (HasProperty("unwatchedepisodes"))
+      if (field == FieldNumberOfWatchedEpisodes && HasProperty("unwatchedepisodes"))
         sortable[FieldNumberOfWatchedEpisodes] = GetProperty("unwatchedepisodes");
     }
   }
-    
+
   if (HasPictureInfoTag())
-    GetPictureInfoTag()->ToSortable(sortable);
+    GetPictureInfoTag()->ToSortable(sortable, field);
 
   if (HasPVRChannelInfoTag())
-    GetPVRChannelInfoTag()->ToSortable(sortable);
+    GetPVRChannelInfoTag()->ToSortable(sortable, field);
+}
+
+void CFileItem::ToSortable(SortItem &sortable, const Fields &fields) const
+{
+  Fields::const_iterator it;
+  for (it = fields.begin(); it != fields.end(); it++)
+    ToSortable(sortable, *it);
+
+  /* It seems SortUtils assumes we have this one even when we're not sorting by it */
+  sortable[FieldLabel] = GetLabel();
 }
 
 bool CFileItem::Exists(bool bUseCache /* = true */) const
@@ -1887,11 +1899,12 @@ void CFileItemList::Sort(SortDescription sortDescription)
   if (m_sortIgnoreFolders)
     sortDescription.sortAttributes = (SortAttribute)((int)sortDescription.sortAttributes | SortAttributeIgnoreFolders);
 
+  const Fields fields = SortUtils::GetFieldsForSorting(sortDescription.sortBy);
   SortItems sortItems((size_t)Size());
   for (int index = 0; index < Size(); index++)
   {
     sortItems[index] = boost::shared_ptr<SortItem>(new SortItem);
-    m_items[index]->ToSortable(*sortItems[index]);
+    m_items[index]->ToSortable(*sortItems[index], fields);
     (*sortItems[index])[FieldId] = index;
   }
 

diff --git a/xbmc/FileItem.h b/xbmc/FileItem.h
index 3d9c565..4113dba 100644 (file)
--- a/xbmc/FileItem.h
+++ b/xbmc/FileItem.h
@@ -113,7 +113,8 @@ public:
   const CFileItem& operator=(const CFileItem& item);
   virtual void Archive(CArchive& ar);
   virtual void Serialize(CVariant& value) const;
-  virtual void ToSortable(SortItem &sortable);
+  virtual void ToSortable(SortItem &sortable, Field field) const;
+  void ToSortable(SortItem &sortable, const Fields &fields) const;
   virtual bool IsFileItem() const { return true; };
 
   bool Exists(bool bUseCache = true) const;

diff --git a/xbmc/music/tags/MusicInfoTag.cpp b/xbmc/music/tags/MusicInfoTag.cpp
index 1a3767a..77ca384 100644 (file)
--- a/xbmc/music/tags/MusicInfoTag.cpp
+++ b/xbmc/music/tags/MusicInfoTag.cpp
@@ -590,22 +590,26 @@ void CMusicInfoTag::Serialize(CVariant& value) const
   value["compilationartist"] = m_bCompilation;
 }
 
-void CMusicInfoTag::ToSortable(SortItem& sortable)
-{
-  sortable[FieldTitle] = m_strTitle;
-  sortable[FieldArtist] = m_artist;
-  sortable[FieldAlbum] = m_strAlbum;
-  sortable[FieldAlbumArtist] = FieldAlbumArtist;
-  sortable[FieldGenre] = m_genre;
-  sortable[FieldTime] = m_iDuration;
-  sortable[FieldTrackNumber] = m_iTrack;
-  sortable[FieldYear] = m_dwReleaseDate.wYear;
-  sortable[FieldComment] = m_strComment;
-  sortable[FieldRating] = (float)(m_rating - '0');
-  sortable[FieldPlaycount] = m_iTimesPlayed;
-  sortable[FieldLastPlayed] = m_lastPlayed.IsValid() ? m_lastPlayed.GetAsDBDateTime() : StringUtils::EmptyString;
-  sortable[FieldListeners] = m_listeners;
-  sortable[FieldId] = (int64_t)m_iDbId;
+void CMusicInfoTag::ToSortable(SortItem& sortable, Field field) const
+{
+  switch (field)
+  {
+  case FieldTitle:       sortable[FieldTitle] = m_strTitle; break;
+  case FieldArtist:      sortable[FieldArtist] = m_artist; break;
+  case FieldAlbum:       sortable[FieldAlbum] = m_strAlbum; break;
+  case FieldAlbumArtist: sortable[FieldAlbumArtist] = FieldAlbumArtist; break;
+  case FieldGenre:       sortable[FieldGenre] = m_genre; break;
+  case FieldTime:        sortable[FieldTime] = m_iDuration; break;
+  case FieldTrackNumber: sortable[FieldTrackNumber] = m_iTrack; break;
+  case FieldYear:        sortable[FieldYear] = m_dwReleaseDate.wYear; break;
+  case FieldComment:     sortable[FieldComment] = m_strComment; break;
+  case FieldRating:      sortable[FieldRating] = (float)(m_rating - '0'); break;
+  case FieldPlaycount:   sortable[FieldPlaycount] = m_iTimesPlayed; break;
+  case FieldLastPlayed:  sortable[FieldLastPlayed] = m_lastPlayed.IsValid() ? m_lastPlayed.GetAsDBDateTime() : StringUtils::EmptyString; break;
+  case FieldListeners:   sortable[FieldListeners] = m_listeners; break;
+  case FieldId:          sortable[FieldId] = (int64_t)m_iDbId; break;
+  default: break;
+  }
 }
 
 void CMusicInfoTag::Archive(CArchive& ar)

diff --git a/xbmc/music/tags/MusicInfoTag.h b/xbmc/music/tags/MusicInfoTag.h
index f5bb5c5..0b76d22 100644 (file)
--- a/xbmc/music/tags/MusicInfoTag.h
+++ b/xbmc/music/tags/MusicInfoTag.h
@@ -173,7 +173,7 @@ public:
 
   virtual void Archive(CArchive& ar);
   virtual void Serialize(CVariant& ar) const;
-  virtual void ToSortable(SortItem& sortable);
+  virtual void ToSortable(SortItem& sortable, Field field) const;
 
   void Clear();
 protected:

diff --git a/xbmc/pictures/PictureInfoTag.cpp b/xbmc/pictures/PictureInfoTag.cpp
index 1a6d0de..99a3079 100644 (file)
--- a/xbmc/pictures/PictureInfoTag.cpp
+++ b/xbmc/pictures/PictureInfoTag.cpp
@@ -269,9 +269,9 @@ void CPictureInfoTag::Serialize(CVariant& value) const
   value["imagetype"] = CStdString(m_iptcInfo.ImageType);
 }
 
-void CPictureInfoTag::ToSortable(SortItem& sortable)
+void CPictureInfoTag::ToSortable(SortItem& sortable, Field field) const
 {
-  if (m_dateTimeTaken.IsValid())
+  if (field == FieldDateTaken && m_dateTimeTaken.IsValid())
     sortable[FieldDateTaken] = m_dateTimeTaken.GetAsDBDateTime();
 }
 

diff --git a/xbmc/pictures/PictureInfoTag.h b/xbmc/pictures/PictureInfoTag.h
index 500316d..c47b8e7 100644 (file)
--- a/xbmc/pictures/PictureInfoTag.h
+++ b/xbmc/pictures/PictureInfoTag.h
@@ -96,7 +96,7 @@ public:
   void Reset();
   virtual void Archive(CArchive& ar);
   virtual void Serialize(CVariant& value) const;
-  virtual void ToSortable(SortItem& sortable);
+  virtual void ToSortable(SortItem& sortable, Field field) const;
   const CPictureInfoTag& operator=(const CPictureInfoTag& item);
   const CStdString GetInfo(int info) const;
 

diff --git a/xbmc/pvr/channels/PVRChannel.cpp b/xbmc/pvr/channels/PVRChannel.cpp
index 042efac..dfe74ee 100644 (file)
--- a/xbmc/pvr/channels/PVRChannel.cpp
+++ b/xbmc/pvr/channels/PVRChannel.cpp
@@ -707,10 +707,13 @@ void CPVRChannel::SetCachedChannelNumber(unsigned int iChannelNumber)
   m_iCachedChannelNumber = iChannelNumber;
 }
 
-void CPVRChannel::ToSortable(SortItem& sortable) const
+void CPVRChannel::ToSortable(SortItem& sortable, Field field) const
 {
-  CSingleLock lock(m_critSection);
-  sortable[FieldChannelName] = m_strChannelName;
+  if (field == FieldChannelName)
+  {
+    CSingleLock lock(m_critSection);
+    sortable[FieldChannelName] = m_strChannelName;
+  }
 }
 
 int CPVRChannel::ChannelID(void) const

diff --git a/xbmc/pvr/channels/PVRChannel.h b/xbmc/pvr/channels/PVRChannel.h
index 2c0a912..74fd087 100644 (file)
--- a/xbmc/pvr/channels/PVRChannel.h
+++ b/xbmc/pvr/channels/PVRChannel.h
@@ -43,7 +43,7 @@ namespace PVR
   typedef boost::shared_ptr<PVR::CPVRChannel> CPVRChannelPtr;
 
   /** PVR Channel class */
-  class CPVRChannel : public Observable, public ISerializable
+  class CPVRChannel : public Observable, public ISerializable, public ISortable
   {
     friend class CPVRDatabase;
     friend class CPVRChannelGroupInternal;
@@ -320,7 +320,7 @@ namespace PVR
      */
     CStdString Path(void) const;
 
-    void ToSortable(SortItem& sortable) const;
+    virtual void ToSortable(SortItem& sortable, Field field) const;
 
     /*!
      * @brief Update the path after the channel number in the internal group changed.

diff --git a/xbmc/utils/ISortable.h b/xbmc/utils/ISortable.h
index 468bb64..21bc712 100644 (file)
--- a/xbmc/utils/ISortable.h
+++ b/xbmc/utils/ISortable.h
@@ -27,5 +27,5 @@ class ISortable
 {
 public:
   virtual ~ISortable() { }
-  virtual void ToSortable(SortItem& sortable) = 0;
+  virtual void ToSortable(SortItem& sortable, Field field) const = 0;
 };

diff --git a/xbmc/video/VideoInfoTag.cpp b/xbmc/video/VideoInfoTag.cpp
index 621bc88..2517fb8 100644 (file)
--- a/xbmc/video/VideoInfoTag.cpp
+++ b/xbmc/video/VideoInfoTag.cpp
@@ -484,57 +484,61 @@ void CVideoInfoTag::Serialize(CVariant& value) const
   value["seasonid"] = m_iIdSeason;
 }
 
-void CVideoInfoTag::ToSortable(SortItem& sortable)
+void CVideoInfoTag::ToSortable(SortItem& sortable, Field field) const
 {
-  sortable[FieldDirector] = m_director;
-  sortable[FieldWriter] = m_writingCredits;
-  sortable[FieldGenre] = m_genre;
-  sortable[FieldCountry] = m_country;
-  sortable[FieldTagline] = m_strTagLine;
-  sortable[FieldPlotOutline] = m_strPlotOutline;
-  sortable[FieldPlot] = m_strPlot;
-  sortable[FieldTitle] = m_strTitle;
-  sortable[FieldVotes] = m_strVotes;
-  sortable[FieldStudio] = m_studio;
-  sortable[FieldTrailer] = m_strTrailer;
-  sortable[FieldSet] = m_strSet;
-  sortable[FieldTime] = GetDuration();
-  sortable[FieldFilename] = m_strFile;
-  sortable[FieldMPAA] = m_strMPAARating;
-  sortable[FieldPath] = m_strFileNameAndPath;
-  sortable[FieldSortTitle] = m_strSortTitle;
-  sortable[FieldTvShowStatus] = m_strStatus;
-  sortable[FieldProductionCode] = m_strProductionCode;
-  sortable[FieldAirDate] = m_firstAired.IsValid() ? m_firstAired.GetAsDBDate() : (m_premiered.IsValid() ? m_premiered.GetAsDBDate() : StringUtils::EmptyString);
-  sortable[FieldTvShowTitle] = m_strShowTitle;
-  sortable[FieldAlbum] = m_strAlbum;
-  sortable[FieldArtist] = m_artist;
-  sortable[FieldPlaycount] = m_playCount;
-  sortable[FieldLastPlayed] = m_lastPlayed.IsValid() ? m_lastPlayed.GetAsDBDateTime() : StringUtils::EmptyString;
-  sortable[FieldTop250] = m_iTop250;
-  sortable[FieldYear] = m_iYear;
-  sortable[FieldSeason] = m_iSeason;
-  sortable[FieldEpisodeNumber] = m_iEpisode;
-  sortable[FieldEpisodeNumberSpecialSort] = m_iSpecialSortEpisode;
-  sortable[FieldSeasonSpecialSort] = m_iSpecialSortSeason;
-  sortable[FieldRating] = m_fRating;
-  sortable[FieldId] = m_iDbId;
-  sortable[FieldTrackNumber] = m_iTrack;
-  sortable[FieldTag] = m_tags;
-
-  sortable[FieldVideoResolution] = m_streamDetails.GetVideoHeight();
-  sortable[FieldVideoAspectRatio] = m_streamDetails.GetVideoAspect();
-  sortable[FieldVideoCodec] = m_streamDetails.GetVideoCodec();
-  
-  sortable[FieldAudioChannels] = m_streamDetails.GetAudioChannels();
-  sortable[FieldAudioCodec] = m_streamDetails.GetAudioCodec();
-  sortable[FieldAudioLanguage] = m_streamDetails.GetAudioLanguage();
-  
-  sortable[FieldSubtitleLanguage] = m_streamDetails.GetSubtitleLanguage();
-
-  sortable[FieldInProgress] = m_resumePoint.IsPartWay();
-  sortable[FieldDateAdded] = m_dateAdded.IsValid() ? m_dateAdded.GetAsDBDateTime() : StringUtils::EmptyString;
-  sortable[FieldMediaType] = DatabaseUtils::MediaTypeFromString(m_type);
+  switch (field)
+  {
+  case FieldDirector:                 sortable[FieldDirector] = m_director; break;
+  case FieldWriter:                   sortable[FieldWriter] = m_writingCredits; break;
+  case FieldGenre:                    sortable[FieldGenre] = m_genre; break;
+  case FieldCountry:                  sortable[FieldCountry] = m_country; break;
+  case FieldTagline:                  sortable[FieldTagline] = m_strTagLine; break;
+  case FieldPlotOutline:              sortable[FieldPlotOutline] = m_strPlotOutline; break;
+  case FieldPlot:                     sortable[FieldPlot] = m_strPlot; break;
+  case FieldTitle:                    sortable[FieldTitle] = m_strTitle; break;
+  case FieldVotes:                    sortable[FieldVotes] = m_strVotes; break;
+  case FieldStudio:                   sortable[FieldStudio] = m_studio; break;
+  case FieldTrailer:                  sortable[FieldTrailer] = m_strTrailer; break;
+  case FieldSet:                      sortable[FieldSet] = m_strSet; break;
+  case FieldTime:                     sortable[FieldTime] = GetDuration(); break;
+  case FieldFilename:                 sortable[FieldFilename] = m_strFile; break;
+  case FieldMPAA:                     sortable[FieldMPAA] = m_strMPAARating; break;
+  case FieldPath:                     sortable[FieldPath] = m_strFileNameAndPath; break;
+  case FieldSortTitle:                sortable[FieldSortTitle] = m_strSortTitle; break;
+  case FieldTvShowStatus:             sortable[FieldTvShowStatus] = m_strStatus; break;
+  case FieldProductionCode:           sortable[FieldProductionCode] = m_strProductionCode; break;
+  case FieldAirDate:                  sortable[FieldAirDate] = m_firstAired.IsValid() ? m_firstAired.GetAsDBDate() : (m_premiered.IsValid() ? m_premiered.GetAsDBDate() : StringUtils::EmptyString); break;
+  case FieldTvShowTitle:              sortable[FieldTvShowTitle] = m_strShowTitle; break;
+  case FieldAlbum:                    sortable[FieldAlbum] = m_strAlbum; break;
+  case FieldArtist:                   sortable[FieldArtist] = m_artist; break;
+  case FieldPlaycount:                sortable[FieldPlaycount] = m_playCount; break;
+  case FieldLastPlayed:               sortable[FieldLastPlayed] = m_lastPlayed.IsValid() ? m_lastPlayed.GetAsDBDateTime() : StringUtils::EmptyString; break;
+  case FieldTop250:                   sortable[FieldTop250] = m_iTop250; break;
+  case FieldYear:                     sortable[FieldYear] = m_iYear; break;
+  case FieldSeason:                   sortable[FieldSeason] = m_iSeason; break;
+  case FieldEpisodeNumber:            sortable[FieldEpisodeNumber] = m_iEpisode; break;
+  case FieldEpisodeNumberSpecialSort: sortable[FieldEpisodeNumberSpecialSort] = m_iSpecialSortEpisode; break;
+  case FieldSeasonSpecialSort:        sortable[FieldSeasonSpecialSort] = m_iSpecialSortSeason; break;
+  case FieldRating:                   sortable[FieldRating] = m_fRating; break;
+  case FieldId:                       sortable[FieldId] = m_iDbId; break;
+  case FieldTrackNumber:              sortable[FieldTrackNumber] = m_iTrack; break;
+  case FieldTag:                      sortable[FieldTag] = m_tags; break;
+
+  case FieldVideoResolution:          sortable[FieldVideoResolution] = m_streamDetails.GetVideoHeight(); break;
+  case FieldVideoAspectRatio:         sortable[FieldVideoAspectRatio] = m_streamDetails.GetVideoAspect(); break;
+  case FieldVideoCodec:               sortable[FieldVideoCodec] = m_streamDetails.GetVideoCodec(); break;
+
+  case FieldAudioChannels:            sortable[FieldAudioChannels] = m_streamDetails.GetAudioChannels(); break;
+  case FieldAudioCodec:               sortable[FieldAudioCodec] = m_streamDetails.GetAudioCodec(); break;
+  case FieldAudioLanguage:            sortable[FieldAudioLanguage] = m_streamDetails.GetAudioLanguage(); break;
+
+  case FieldSubtitleLanguage:         sortable[FieldSubtitleLanguage] = m_streamDetails.GetSubtitleLanguage(); break;
+
+  case FieldInProgress:               sortable[FieldInProgress] = m_resumePoint.IsPartWay(); break;
+  case FieldDateAdded:                sortable[FieldDateAdded] = m_dateAdded.IsValid() ? m_dateAdded.GetAsDBDateTime() : StringUtils::EmptyString; break;
+  case FieldMediaType:                sortable[FieldMediaType] = DatabaseUtils::MediaTypeFromString(m_type); break;
+  default: break;
+  }
 }
 
 const CStdString CVideoInfoTag::GetCast(bool bIncludeRole /*= false*/) const

diff --git a/xbmc/video/VideoInfoTag.h b/xbmc/video/VideoInfoTag.h
index ed8dd2f..dc22747 100644 (file)
--- a/xbmc/video/VideoInfoTag.h
+++ b/xbmc/video/VideoInfoTag.h
@@ -64,7 +64,7 @@ public:
   bool Save(TiXmlNode *node, const CStdString &tag, bool savePathInfo = true, const TiXmlElement *additionalNode = NULL);
   virtual void Archive(CArchive& ar);
   virtual void Serialize(CVariant& value) const;
-  virtual void ToSortable(SortItem& sortable);
+  virtual void ToSortable(SortItem& sortable, Field field) const;
   const CStdString GetCast(bool bIncludeRole = false) const;
   bool HasStreamDetails() const;
   bool IsEmpty() const;