Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

jsonrpc: optimize handling of properties requested by the client

Up until now FillDetails, which contains a loop to go through all
the properties requested by the client in a request has been called
at least twice (once for the CFileItem and once for the CFooInfoTag)
for every media item matching the client's request. With n items
and m properties this resulted in n * 2 * m loops which can result
in longer response times. By removing the already matched properties
from the list of requested properties we get down to n * m loops.
Especially for "expensive" properties like "thumbnail" and "fanart"
which require additional SQL queries avoiding doing the same work
twice which can have a huge impact.
In addition moving the logic to retrieve any requested properties
directly from the serialization of the media item instead of first
checking and handling any special cases results in additional
speed-up by not having to do the extra string comparisons and map-
lookups required for the special cases.
  • Loading branch information...
commit ef7cae0e6e9947d7230bc8562feb80e19f86d50f 1 parent e5bbb0f
Sascha Montellese authored
242 xbmc/interfaces/json-rpc/FileItemHandler.cpp
View
@@ -41,111 +41,125 @@ using namespace MUSIC_INFO;
using namespace JSONRPC;
using namespace XFILE;
-void CFileItemHandler::FillDetails(ISerializable* info, CFileItemPtr item, const CVariant& fields, CVariant &result, CThumbLoader *thumbLoader /* = NULL */)
+bool CFileItemHandler::GetField(const std::string &field, const CVariant &info, const CFileItemPtr &item, CVariant &result, bool &fetchedArt, CThumbLoader *thumbLoader /* = NULL */)
{
- if (info == NULL || fields.size() == 0)
- return;
-
- CVariant serialization;
- info->Serialize(serialization);
+ if (result.isMember(field) && !result[field].empty())
+ return true;
- bool fetchedArt = false;
-
- for (unsigned int i = 0; i < fields.size(); i++)
+ if (info.isMember(field) && !info[field].isNull())
{
- CStdString field = fields[i].asString();
+ result[field] = info[field];
+ return true;
+ }
- if (item)
+ if (item)
+ {
+ if (item->IsAlbum())
{
- if (item->IsAlbum() && field.Equals("albumlabel"))
- field = "label";
- if (item->IsAlbum())
+ if (field == "albumlabel")
{
- if (field == "label")
- {
- result["albumlabel"] = item->GetProperty("album_label");
- continue;
- }
- if (item->HasProperty("album_" + field + "_array"))
- {
- result[field] = item->GetProperty("album_" + field + "_array");
- continue;
- }
- if (item->HasProperty("album_" + field))
- {
- result[field] = item->GetProperty("album_" + field);
- continue;
- }
+ result[field] = item->GetProperty("album_label");
+ return true;
}
-
- if (item->HasProperty("artist_" + field + "_array"))
+ if (item->HasProperty("album_" + field + "_array"))
{
- result[field] = item->GetProperty("artist_" + field + "_array");
- continue;
+ result[field] = item->GetProperty("album_" + field + "_array");
+ return true;
}
- if (item->HasProperty("artist_" + field))
+ if (item->HasProperty("album_" + field))
{
- result[field] = item->GetProperty("artist_" + field);
- continue;
+ result[field] = item->GetProperty("album_" + field);
+ return true;
}
-
- if (field == "thumbnail")
+ }
+
+ if (item->HasProperty("artist_" + field + "_array"))
+ {
+ result[field] = item->GetProperty("artist_" + field + "_array");
+ return true;
+ }
+ if (item->HasProperty("artist_" + field))
+ {
+ result[field] = item->GetProperty("artist_" + field);
+ return true;
+ }
+
+ if (field == "thumbnail")
+ {
+ if (thumbLoader != NULL && !item->HasThumbnail() && !fetchedArt &&
+ ((item->HasVideoInfoTag() && item->GetVideoInfoTag()->m_iDbId > -1) || (item->HasMusicInfoTag() && item->GetMusicInfoTag()->GetDatabaseId() > -1)))
{
- if (thumbLoader != NULL && !item->HasThumbnail() && !fetchedArt &&
- ((item->HasVideoInfoTag() && item->GetVideoInfoTag()->m_iDbId > -1) || (item->HasMusicInfoTag() && item->GetMusicInfoTag()->GetDatabaseId() > -1)))
- {
- thumbLoader->FillLibraryArt(*item);
- fetchedArt = true;
- }
- else if (item->HasPictureInfoTag() && !item->HasThumbnail())
- item->SetThumbnailImage(CTextureCache::GetWrappedThumbURL(item->GetPath()));
-
- if (item->HasThumbnail())
- result["thumbnail"] = CTextureCache::GetWrappedImageURL(item->GetThumbnailImage());
- else
- result["thumbnail"] = "";
- continue;
+ thumbLoader->FillLibraryArt(*item);
+ fetchedArt = true;
}
-
- if (field == "fanart")
+ else if (item->HasPictureInfoTag() && !item->HasThumbnail())
+ item->SetThumbnailImage(CTextureCache::GetWrappedThumbURL(item->GetPath()));
+
+ if (item->HasThumbnail())
+ result["thumbnail"] = CTextureCache::GetWrappedImageURL(item->GetThumbnailImage());
+ else
+ result["thumbnail"] = "";
+
+ return true;
+ }
+
+ if (field == "fanart")
+ {
+ if (thumbLoader != NULL && !item->HasProperty("fanart_image") && !fetchedArt &&
+ ((item->HasVideoInfoTag() && item->GetVideoInfoTag()->m_iDbId > -1) || (item->HasMusicInfoTag() && item->GetMusicInfoTag()->GetDatabaseId() > -1)))
{
- if (thumbLoader != NULL && !item->HasProperty("fanart_image") && !fetchedArt &&
- ((item->HasVideoInfoTag() && item->GetVideoInfoTag()->m_iDbId > -1) || (item->HasMusicInfoTag() && item->GetMusicInfoTag()->GetDatabaseId() > -1)))
- {
- thumbLoader->FillLibraryArt(*item);
- fetchedArt = true;
- }
-
- if (item->HasProperty("fanart_image"))
- result["fanart"] = CTextureCache::GetWrappedImageURL(item->GetProperty("fanart_image").asString());
- else
- result["fanart"] = "";
- continue;
+ thumbLoader->FillLibraryArt(*item);
+ fetchedArt = true;
}
-
- if (item->HasVideoInfoTag() && item->GetVideoContentType() == VIDEODB_CONTENT_TVSHOWS)
+
+ if (item->HasProperty("fanart_image"))
+ result["fanart"] = CTextureCache::GetWrappedImageURL(item->GetProperty("fanart_image").asString());
+ else
+ result["fanart"] = "";
+
+ return true;
+ }
+
+ if (item->HasVideoInfoTag() && item->GetVideoContentType() == VIDEODB_CONTENT_TVSHOWS)
+ {
+ if (item->GetVideoInfoTag()->m_iSeason < 0 && field == "season")
{
- if (item->GetVideoInfoTag()->m_iSeason < 0 && field == "season")
- {
- result[field] = (int)item->GetProperty("totalseasons").asInteger();
- continue;
- }
- if (field == "watchedepisodes")
- {
- result[field] = (int)item->GetProperty("watchedepisodes").asInteger();
- continue;
- }
+ result[field] = (int)item->GetProperty("totalseasons").asInteger();
+ return true;
}
-
- if (field == "lastmodified" && item->m_dateTime.IsValid())
+ if (field == "watchedepisodes")
{
- result[field] = item->m_dateTime.GetAsLocalizedDateTime();
- continue;
+ result[field] = (int)item->GetProperty("watchedepisodes").asInteger();
+ return true;
}
}
+
+ if (field == "lastmodified" && item->m_dateTime.IsValid())
+ {
+ result[field] = item->m_dateTime.GetAsLocalizedDateTime();
+ return true;
+ }
+ }
+
+ return false;
+}
+
+void CFileItemHandler::FillDetails(const ISerializable *info, const CFileItemPtr &item, std::set<std::string> &fields, CVariant &result, CThumbLoader *thumbLoader /* = NULL */)
+{
+ if (info == NULL || fields.size() == 0)
+ return;
+
+ CVariant serialization;
+ info->Serialize(serialization);
+
+ bool fetchedArt = false;
+
+ std::set<std::string> originalFields = fields;
- if (serialization.isMember(field) && !serialization[field].isNull() && (!result.isMember(field) || result[field].empty()))
- result[field] = serialization[field];
+ for (std::set<std::string>::const_iterator fieldIt = originalFields.begin(); fieldIt != originalFields.end(); fieldIt++)
+ {
+ if (GetField(*fieldIt, serialization, item, result, fetchedArt, thumbLoader))
+ fields.erase(*fieldIt);
}
}
@@ -179,11 +193,18 @@ void CFileItemHandler::HandleFileItemList(const char *ID, bool allowFile, const
thumbLoader->Initialize();
}
+ std::set<std::string> fields;
+ if (parameterObject.isMember("properties") && parameterObject["properties"].isArray())
+ {
+ for (CVariant::const_iterator_array field = parameterObject["properties"].begin_array(); field != parameterObject["properties"].end_array(); field++)
+ fields.insert(field->asString());
+ }
+
for (int i = start; i < end; i++)
{
CVariant object;
CFileItemPtr item = items.Get(i);
- HandleFileItem(ID, allowFile, resultname, item, parameterObject, parameterObject["properties"], result, true, thumbLoader);
+ HandleFileItem(ID, allowFile, resultname, item, parameterObject, fields, result, true, thumbLoader);
}
delete thumbLoader;
@@ -191,28 +212,37 @@ void CFileItemHandler::HandleFileItemList(const char *ID, bool allowFile, const
void CFileItemHandler::HandleFileItem(const char *ID, bool allowFile, const char *resultname, CFileItemPtr item, const CVariant &parameterObject, const CVariant &validFields, CVariant &result, bool append /* = true */, CThumbLoader *thumbLoader /* = NULL */)
{
+ std::set<std::string> fields;
+ if (parameterObject.isMember("properties") && parameterObject["properties"].isArray())
+ {
+ for (CVariant::const_iterator_array field = parameterObject["properties"].begin_array(); field != parameterObject["properties"].end_array(); field++)
+ fields.insert(field->asString());
+ }
+
+ HandleFileItem(ID, allowFile, resultname, item, parameterObject, fields, result, append, thumbLoader);
+}
+
+void CFileItemHandler::HandleFileItem(const char *ID, bool allowFile, const char *resultname, CFileItemPtr item, const CVariant &parameterObject, const std::set<std::string> &validFields, CVariant &result, bool append /* = true */, CThumbLoader *thumbLoader /* = NULL */)
+{
CVariant object;
- bool hasFileField = false;
+ std::set<std::string> fields(validFields.begin(), validFields.end());
if (item.get())
{
- for (unsigned int i = 0; i < validFields.size(); i++)
- {
- CStdString field = validFields[i].asString();
-
- if (field == "file")
- hasFileField = true;
- }
-
- if (allowFile && hasFileField)
+ std::set<std::string>::const_iterator fileField = fields.find("file");
+ if (fileField != fields.end())
{
- if (item->HasVideoInfoTag() && !item->GetVideoInfoTag()->GetPath().IsEmpty())
- object["file"] = item->GetVideoInfoTag()->GetPath().c_str();
- if (item->HasMusicInfoTag() && !item->GetMusicInfoTag()->GetURL().IsEmpty())
- object["file"] = item->GetMusicInfoTag()->GetURL().c_str();
+ if (allowFile)
+ {
+ if (item->HasVideoInfoTag() && !item->GetVideoInfoTag()->GetPath().IsEmpty())
+ object["file"] = item->GetVideoInfoTag()->GetPath().c_str();
+ if (item->HasMusicInfoTag() && !item->GetMusicInfoTag()->GetURL().IsEmpty())
+ object["file"] = item->GetMusicInfoTag()->GetURL().c_str();
- if (!object.isMember("file"))
- object["file"] = item->GetPath().c_str();
+ if (!object.isMember("file"))
+ object["file"] = item->GetPath().c_str();
+ }
+ fields.erase(fileField);
}
if (ID)
@@ -260,14 +290,14 @@ void CFileItemHandler::HandleFileItem(const char *ID, bool allowFile, const char
}
}
- FillDetails(item.get(), item, validFields, object, thumbLoader);
-
if (item->HasVideoInfoTag())
- FillDetails(item->GetVideoInfoTag(), item, validFields, object, thumbLoader);
+ FillDetails(item->GetVideoInfoTag(), item, fields, object, thumbLoader);
if (item->HasMusicInfoTag())
- FillDetails(item->GetMusicInfoTag(), item, validFields, object, thumbLoader);
+ FillDetails(item->GetMusicInfoTag(), item, fields, object, thumbLoader);
if (item->HasPictureInfoTag())
- FillDetails(item->GetPictureInfoTag(), item, validFields, object, thumbLoader);
+ FillDetails(item->GetPictureInfoTag(), item, fields, object, thumbLoader);
+
+ FillDetails(item.get(), item, fields, object, thumbLoader);
if (deleteThumbloader)
delete thumbLoader;
6 xbmc/interfaces/json-rpc/FileItemHandler.h
View
@@ -19,6 +19,8 @@
*
*/
+#include <set>
+
#include "JSONRPC.h"
#include "JSONUtils.h"
#include "FileItem.h"
@@ -30,13 +32,15 @@ namespace JSONRPC
class CFileItemHandler : public CJSONUtils
{
protected:
- static void FillDetails(ISerializable* info, CFileItemPtr item, const CVariant& fields, CVariant &result, CThumbLoader *thumbLoader = NULL);
+ static void FillDetails(const ISerializable *info, const CFileItemPtr &item, std::set<std::string> &fields, CVariant &result, CThumbLoader *thumbLoader = NULL);
static void HandleFileItemList(const char *ID, bool allowFile, const char *resultname, CFileItemList &items, const CVariant &parameterObject, CVariant &result, bool sortLimit = true);
static void HandleFileItemList(const char *ID, bool allowFile, const char *resultname, CFileItemList &items, const CVariant &parameterObject, CVariant &result, int size, bool sortLimit = true);
static void HandleFileItem(const char *ID, bool allowFile, const char *resultname, CFileItemPtr item, const CVariant &parameterObject, const CVariant &validFields, CVariant &result, bool append = true, CThumbLoader *thumbLoader = NULL);
+ static void HandleFileItem(const char *ID, bool allowFile, const char *resultname, CFileItemPtr item, const CVariant &parameterObject, const std::set<std::string> &validFields, CVariant &result, bool append = true, CThumbLoader *thumbLoader = NULL);
static bool FillFileItemList(const CVariant &parameterObject, CFileItemList &list);
private:
static void Sort(CFileItemList &items, const CVariant& parameterObject);
+ static bool GetField(const std::string &field, const CVariant &info, const CFileItemPtr &item, CVariant &result, bool &fetchedArt, CThumbLoader *thumbLoader = NULL);
};
}
Please sign in to comment.
Something went wrong with that request. Please try again.