Skip to content

Commit

Permalink
Faster string comparison functions in StringUtils
Browse files Browse the repository at this point in the history 
The methods EqualsNoCase(), StartsWith() and EndsWith() required quite a lot
of unncessary creation and destruction of std::strings, whether it be the
creation of a temporary copy that could be forced to lower-case, a temporary
copy that was a substring of one of the inputs, or just marshalling a
string literal argument into a std::string.

These functions don't appear to be used all that much at the moment; when I
profiled opening the songs library, I saw only a 1% improvement, which was
within the sampling noise threshold. But with PR #3225 and PR #3290 coming
along, that looks set to change. Once the functions are being called millions
of times, those heap operations really start to get noticeable.

Also, split StartsWith() and EndsWith() into multiple separately-named
functions, according to case sensitivity. Formerly, there was an optional
parameter (default off) to indicate that these operations were
case-*sensitive*, which is actually computationally simpler to perform. Now
the naming convention is consistent with EqualsNoCase:
  StartsWith       - case-sensitive
  EndsWith         - case-sensitive
  StartsWithNoCase - case-insensitive
  EndsWithNoCase   - case-insensitive

With the case-sensitive versions now easier to type, it will hopefully
encourage future developers to use them in preference.
  • Loading branch information
@bavison
bavison committed Sep 27, 2013
1 parent 92169a6 commit 02fd1ab
Show file tree
Hide file tree
Showing 23 changed files with 194 additions and 99 deletions.
6 changes: 3 additions & 3 deletions xbmc/Application.cpp
View file
Expand Up @@ -1586,7 +1586,7 @@ void CApplication::OnSettingChanged(const CSetting *setting)
}
else if (settingId == "lookandfeel.skinzoom")
g_windowManager.SendMessage(GUI_MSG_NOTIFY_ALL, 0, 0, GUI_MSG_WINDOW_RESIZE);
else if (StringUtils::StartsWith(settingId, "audiooutput."))
else if (StringUtils::StartsWithNoCase(settingId, "audiooutput."))
{
if (settingId == "audiooutput.guisoundmode")
{
Expand Down Expand Up @@ -2225,13 +2225,13 @@ bool CApplication::OnKey(const CKey& key)
m_idleTimer.StartZero();
bool processKey = AlwaysProcess(action);

if (StringUtils::StartsWith(action.GetName(),"CECToggleState") || StringUtils::StartsWith(action.GetName(),"CECStandby"))
if (StringUtils::StartsWithNoCase(action.GetName(),"CECToggleState") || StringUtils::StartsWithNoCase(action.GetName(),"CECStandby"))
{
bool ret = true;

CLog::Log(LOGDEBUG, "%s: action %s [%d], toggling state of playing device", __FUNCTION__, action.GetName().c_str(), action.GetID());
// do not wake up the screensaver right after switching off the playing device
if (StringUtils::StartsWith(action.GetName(),"CECToggleState"))
if (StringUtils::StartsWithNoCase(action.GetName(),"CECToggleState"))
ret = CApplicationMessenger::Get().CECToggleState();
else
ret = CApplicationMessenger::Get().CECStandby();
Expand Down
24 changes: 12 additions & 12 deletions xbmc/CueDocument.cpp
View file
Expand Up @@ -103,7 +103,7 @@ bool CCueDocument::Parse(const CStdString &strFile)
{
if (!ReadNextLine(strLine))
break;
if (StringUtils::StartsWith(strLine,"INDEX 01"))
if (StringUtils::StartsWithNoCase(strLine,"INDEX 01"))
{
if (bCurrentFileChanged)
{
Expand All @@ -124,7 +124,7 @@ bool CCueDocument::Parse(const CStdString &strFile)
if (m_iTotalTracks >= 0)
m_Track[m_iTotalTracks].iStartTime = time; // start time of the next track
}
else if (StringUtils::StartsWith(strLine,"TITLE"))
else if (StringUtils::StartsWithNoCase(strLine,"TITLE"))
{
if (m_iTotalTracks == -1) // No tracks yet
ExtractQuoteInfo(strLine, m_strAlbum);
Expand All @@ -140,14 +140,14 @@ bool CCueDocument::Parse(const CStdString &strFile)
}
}
}
else if (StringUtils::StartsWith(strLine,"PERFORMER"))
else if (StringUtils::StartsWithNoCase(strLine,"PERFORMER"))
{
if (m_iTotalTracks == -1) // No tracks yet
ExtractQuoteInfo(strLine, m_strArtist);
else // New Artist for this track
ExtractQuoteInfo(strLine, m_Track[m_iTotalTracks].strArtist);
}
else if (StringUtils::StartsWith(strLine,"TRACK"))
else if (StringUtils::StartsWithNoCase(strLine,"TRACK"))
{
int iTrackNumber = ExtractNumericInfo(strLine.Mid(5));

Expand All @@ -164,13 +164,13 @@ bool CCueDocument::Parse(const CStdString &strFile)

bCurrentFileChanged = false;
}
else if (StringUtils::StartsWith(strLine,"REM DISCNUMBER"))
else if (StringUtils::StartsWithNoCase(strLine,"REM DISCNUMBER"))
{
int iDiscNumber = ExtractNumericInfo(strLine.Mid(14));
if (iDiscNumber > 0)
m_iDiscNumber = iDiscNumber;
}
else if (StringUtils::StartsWith(strLine,"FILE"))
else if (StringUtils::StartsWithNoCase(strLine,"FILE"))
{
// already a file name? then the time computation will be changed
if(strCurrentFile.size() > 0)
Expand All @@ -182,13 +182,13 @@ bool CCueDocument::Parse(const CStdString &strFile)
if (strCurrentFile.length() > 0)
ResolvePath(strCurrentFile, strFile);
}
else if (StringUtils::StartsWith(strLine,"REM DATE"))
else if (StringUtils::StartsWithNoCase(strLine,"REM DATE"))
{
int iYear = ExtractNumericInfo(strLine.Mid(8));
if (iYear > 0)
m_iYear = iYear;
}
else if (StringUtils::StartsWith(strLine,"REM GENRE"))
else if (StringUtils::StartsWithNoCase(strLine,"REM GENRE"))
{
if (!ExtractQuoteInfo(strLine, m_strGenre))
{
Expand All @@ -201,13 +201,13 @@ bool CCueDocument::Parse(const CStdString &strFile)
}
}
}
else if (StringUtils::StartsWith(strLine,"REM REPLAYGAIN_ALBUM_GAIN"))
else if (StringUtils::StartsWithNoCase(strLine,"REM REPLAYGAIN_ALBUM_GAIN"))
m_replayGainAlbumGain = (float)atof(strLine.Mid(26));
else if (StringUtils::StartsWith(strLine,"REM REPLAYGAIN_ALBUM_PEAK"))
else if (StringUtils::StartsWithNoCase(strLine,"REM REPLAYGAIN_ALBUM_PEAK"))
m_replayGainAlbumPeak = (float)atof(strLine.Mid(26));
else if (StringUtils::StartsWith(strLine,"REM REPLAYGAIN_TRACK_GAIN") && m_iTotalTracks >= 0)
else if (StringUtils::StartsWithNoCase(strLine,"REM REPLAYGAIN_TRACK_GAIN") && m_iTotalTracks >= 0)
m_Track[m_iTotalTracks].replayGainTrackGain = (float)atof(strLine.Mid(26));
else if (StringUtils::StartsWith(strLine,"REM REPLAYGAIN_TRACK_PEAK") && m_iTotalTracks >= 0)
else if (StringUtils::StartsWithNoCase(strLine,"REM REPLAYGAIN_TRACK_PEAK") && m_iTotalTracks >= 0)
m_Track[m_iTotalTracks].replayGainTrackPeak = (float)atof(strLine.Mid(26));
}

Expand Down
2 changes: 1 addition & 1 deletion xbmc/addons/Addon.cpp
View file
Expand Up @@ -314,7 +314,7 @@ bool CAddon::MeetsVersion(const AddonVersion &version) const
// if the addon is one of xbmc's extension point definitions (addonid starts with "xbmc.")
// and the minversion is "0.0.0" i.e. no <backwards-compatibility> tag has been specified
// we need to assume that the current version is not backwards-compatible and therefore check against the actual version
if (StringUtils::StartsWith(m_props.id, "xbmc.") &&
if (StringUtils::StartsWithNoCase(m_props.id, "xbmc.") &&
(strlen(m_props.minversion.c_str()) == 0 || StringUtils::EqualsNoCase(m_props.minversion.c_str(), "0.0.0")))
return m_props.version == version;

Expand Down
2 changes: 1 addition & 1 deletion xbmc/cores/AudioEngine/Sinks/AESinkDirectSound.cpp
View file
Expand Up @@ -149,7 +149,7 @@ bool CAESinkDirectSound::Initialize(AEAudioFormat &format, std::string &device)
std::string deviceFriendlyName;
DirectSoundEnumerate(DSEnumCallback, &DSDeviceList);

if(StringUtils::EndsWith(device, std::string("default")))
if(StringUtils::EndsWithNoCase(device, std::string("default")))
strDeviceGUID = GetDefaultDevice();

for (std::list<DSDevice>::iterator itt = DSDeviceList.begin(); itt != DSDeviceList.end(); ++itt)
Expand Down
2 changes: 1 addition & 1 deletion xbmc/cores/AudioEngine/Sinks/AESinkWASAPI.cpp
View file
Expand Up @@ -231,7 +231,7 @@ bool CAESinkWASAPI::Initialize(AEAudioFormat &format, std::string &device)
hr = pEnumDevices->GetCount(&uiCount);
EXIT_ON_FAILURE(hr, __FUNCTION__": Retrieval of audio endpoint count failed.")

if(StringUtils::EndsWith(device, std::string("default")))
if(StringUtils::EndsWithNoCase(device, std::string("default")))
bdefault = true;

if(!bdefault)
Expand Down
20 changes: 10 additions & 10 deletions xbmc/filesystem/test/TestRarFile.cpp
View file
Expand Up @@ -210,7 +210,7 @@ TEST(TestRarFile, StoredRAR)
* an uncompressed RAR archive. See TestRarFile.Read test case.
*/
strpathinrar = itemlist[1]->GetPath();
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/reffile.txt", true));
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/reffile.txt"));
EXPECT_EQ(0, XFILE::CFile::Stat(strpathinrar, &stat_buffer));
EXPECT_TRUE((stat_buffer.st_mode & S_IFMT) | S_IFREG);

Expand Down Expand Up @@ -255,7 +255,7 @@ TEST(TestRarFile, StoredRAR)

/* /testsymlink -> testdir/reffile.txt */
strpathinrar = itemlist[2]->GetPath();
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testsymlink", true));
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testsymlink"));
EXPECT_EQ(0, XFILE::CFile::Stat(strpathinrar, &stat_buffer));
EXPECT_TRUE((stat_buffer.st_mode & S_IFMT) | S_IFLNK);

Expand All @@ -270,7 +270,7 @@ TEST(TestRarFile, StoredRAR)

/* /testsymlinksubdir -> testdir/testsubdir/reffile.txt */
strpathinrar = itemlist[3]->GetPath();
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testsymlinksubdir", true));
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testsymlinksubdir"));
EXPECT_EQ(0, XFILE::CFile::Stat(strpathinrar, &stat_buffer));
EXPECT_TRUE((stat_buffer.st_mode & S_IFMT) | S_IFLNK);

Expand All @@ -280,7 +280,7 @@ TEST(TestRarFile, StoredRAR)

/* /testdir/ */
strpathinrar = itemlist[0]->GetPath();
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testdir/", true));
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testdir/"));
EXPECT_EQ(0, XFILE::CFile::Stat(strpathinrar, &stat_buffer));
EXPECT_TRUE((stat_buffer.st_mode & S_IFMT) | S_IFDIR);

Expand Down Expand Up @@ -345,7 +345,7 @@ TEST(TestRarFile, StoredRAR)

/* FIXME: This directory appears a second time as a file */
strpathinrar = itemlist[3]->GetPath();
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testdir/testsubdir", true));
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testdir/testsubdir"));

/* /testdir/testsymlink -> testsubdir/reffile.txt */
strpathinrar = itemlist[4]->GetPath();
Expand Down Expand Up @@ -432,7 +432,7 @@ TEST(TestRarFile, NormalRAR)

/* /reffile.txt */
strpathinrar = itemlist[1]->GetPath();
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/reffile.txt", true));
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/reffile.txt"));
EXPECT_EQ(0, XFILE::CFile::Stat(strpathinrar, &stat_buffer));
EXPECT_TRUE((stat_buffer.st_mode & S_IFMT) | S_IFREG);

Expand Down Expand Up @@ -477,7 +477,7 @@ TEST(TestRarFile, NormalRAR)

/* /testsymlink -> testdir/reffile.txt */
strpathinrar = itemlist[2]->GetPath();
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testsymlink", true));
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testsymlink"));
EXPECT_EQ(0, XFILE::CFile::Stat(strpathinrar, &stat_buffer));
EXPECT_TRUE((stat_buffer.st_mode & S_IFMT) | S_IFLNK);

Expand All @@ -492,7 +492,7 @@ TEST(TestRarFile, NormalRAR)

/* /testsymlinksubdir -> testdir/testsubdir/reffile.txt */
strpathinrar = itemlist[3]->GetPath();
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testsymlinksubdir", true));
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testsymlinksubdir"));
EXPECT_EQ(0, XFILE::CFile::Stat(strpathinrar, &stat_buffer));
EXPECT_TRUE((stat_buffer.st_mode & S_IFMT) | S_IFLNK);

Expand All @@ -502,7 +502,7 @@ TEST(TestRarFile, NormalRAR)

/* /testdir/ */
strpathinrar = itemlist[0]->GetPath();
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testdir/", true));
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testdir/"));
EXPECT_EQ(0, XFILE::CFile::Stat(strpathinrar, &stat_buffer));
EXPECT_TRUE((stat_buffer.st_mode & S_IFMT) | S_IFDIR);

Expand Down Expand Up @@ -567,7 +567,7 @@ TEST(TestRarFile, NormalRAR)

/* FIXME: This directory appears a second time as a file */
strpathinrar = itemlist[3]->GetPath();
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testdir/testsubdir", true));
ASSERT_TRUE(StringUtils::EndsWith(strpathinrar, "/testdir/testsubdir"));

/* /testdir/testsymlink -> testsubdir/reffile.txt */
strpathinrar = itemlist[4]->GetPath();
Expand Down
2 changes: 1 addition & 1 deletion xbmc/interfaces/generic/ScriptInvocationManager.cpp
View file
Expand Up @@ -122,7 +122,7 @@ void CScriptInvocationManager::RegisterLanguageInvocationHandler(ILanguageInvoca

string ext = extension;
StringUtils::ToLower(ext);
if (!StringUtils::StartsWith(ext, "."))
if (!StringUtils::StartsWithNoCase(ext, "."))
ext = "." + ext;

CSingleLock lock(m_critSection);
Expand Down
2 changes: 1 addition & 1 deletion xbmc/music/windows/GUIWindowMusicBase.cpp
View file
Expand Up @@ -1154,7 +1154,7 @@ bool CGUIWindowMusicBase::CheckFilterAdvanced(CFileItemList &items) const

bool CGUIWindowMusicBase::CanContainFilter(const CStdString &strDirectory) const
{
return StringUtils::StartsWith(strDirectory, "musicdb://");
return StringUtils::StartsWithNoCase(strDirectory, "musicdb://");
}

void CGUIWindowMusicBase::OnInitWindow()
Expand Down
6 changes: 3 additions & 3 deletions xbmc/music/windows/GUIWindowMusicNav.cpp
View file
Expand Up @@ -447,7 +447,7 @@ void CGUIWindowMusicNav::GetContextButtons(int itemNumber, CContextButtons &butt
{
if (!item->m_bIsFolder) // music video
buttons.Add(CONTEXT_BUTTON_INFO, 20393);
if (StringUtils::StartsWith(item->GetPath(), "videodb://musicvideos/artist/") && item->m_bIsFolder)
if (StringUtils::StartsWithNoCase(item->GetPath(), "videodb://musicvideos/artist/") && item->m_bIsFolder)
{
long idArtist = m_musicdatabase.GetArtistByName(m_vecItems->Get(itemNumber)->GetLabel());
if (idArtist > - 1)
Expand Down Expand Up @@ -565,7 +565,7 @@ bool CGUIWindowMusicNav::OnContextButton(int itemNumber, CONTEXT_BUTTON button)
return CGUIWindowMusicBase::OnContextButton(itemNumber,button);

// music videos - artists
if (StringUtils::StartsWith(item->GetPath(), "videodb://musicvideos/artists/"))
if (StringUtils::StartsWithNoCase(item->GetPath(), "videodb://musicvideos/artists/"))
{
long idArtist = m_musicdatabase.GetArtistByName(item->GetLabel());
if (idArtist == -1)
Expand All @@ -582,7 +582,7 @@ bool CGUIWindowMusicNav::OnContextButton(int itemNumber, CONTEXT_BUTTON button)
}

// music videos - albums
if (StringUtils::StartsWith(item->GetPath(), "videodb://musicvideos/albums/"))
if (StringUtils::StartsWithNoCase(item->GetPath(), "videodb://musicvideos/albums/"))
{
long idAlbum = m_musicdatabase.GetAlbumByName(item->GetLabel());
if (idAlbum == -1)
Expand Down
2 changes: 1 addition & 1 deletion xbmc/network/WebServer.cpp
View file
Expand Up @@ -965,7 +965,7 @@ int64_t CWebServer::ParseRangeHeader(const std::string &rangeHeaderValue, int64_
firstPosition = 0;
lastPosition = totalLength - 1;

if (rangeHeaderValue.empty() || !StringUtils::StartsWith(rangeHeaderValue, "bytes="))
if (rangeHeaderValue.empty() || !StringUtils::StartsWithNoCase(rangeHeaderValue, "bytes="))
return totalLength;

int64_t rangesLength = 0;
Expand Down
18 changes: 9 additions & 9 deletions xbmc/network/upnp/UPnPServer.cpp
View file
Expand Up @@ -547,7 +547,7 @@ CUPnPServer::OnBrowseMetadata(PLT_ActionReference& action,

// attempt to determine the parent of this item
CStdString parent;
if (URIUtils::IsVideoDb((const char*)id) || URIUtils::IsMusicDb((const char*)id) || StringUtils::StartsWith((const char*)id, "library://video/")) {
if (URIUtils::IsVideoDb((const char*)id) || URIUtils::IsMusicDb((const char*)id) || StringUtils::StartsWithNoCase((const char*)id, "library://video/")) {
if (!URIUtils::GetParentPath((const char*)id, parent)) {
parent = "0";
}
Expand All @@ -560,11 +560,11 @@ CUPnPServer::OnBrowseMetadata(PLT_ActionReference& action,
// however this is quicker to implement and subsequently purge when a
// better solution presents itself
CStdString child_id((const char*)id);
if (StringUtils::StartsWith(child_id, "special://musicplaylists/")) parent = "musicdb://";
else if (StringUtils::StartsWith(child_id, "special://videoplaylists/")) parent = "library://video/";
else if (StringUtils::StartsWith(child_id, "sources://video/")) parent = "library://video/";
else if (StringUtils::StartsWith(child_id, "special://profile/playlists/music/")) parent = "special://musicplaylists/";
else if (StringUtils::StartsWith(child_id, "special://profile/playlists/video/")) parent = "special://videoplaylists/";
if (StringUtils::StartsWithNoCase(child_id, "special://musicplaylists/")) parent = "musicdb://";
else if (StringUtils::StartsWithNoCase(child_id, "special://videoplaylists/")) parent = "library://video/";
else if (StringUtils::StartsWithNoCase(child_id, "sources://video/")) parent = "library://video/";
else if (StringUtils::StartsWithNoCase(child_id, "special://profile/playlists/music/")) parent = "special://musicplaylists/";
else if (StringUtils::StartsWithNoCase(child_id, "special://profile/playlists/video/")) parent = "special://videoplaylists/";
else parent = "sources://video/"; // this can only match video sources
}

Expand Down Expand Up @@ -729,13 +729,13 @@ CUPnPServer::BuildResponse(PLT_ActionReference& action,
NPT_Reference<CThumbLoader> thumb_loader;

if (URIUtils::IsVideoDb(items.GetPath()) ||
StringUtils::StartsWith(items.GetPath(), "library://video/") ||
StringUtils::StartsWith(items.GetPath(), "special://profile/playlists/video/")) {
StringUtils::StartsWithNoCase(items.GetPath(), "library://video/") ||
StringUtils::StartsWithNoCase(items.GetPath(), "special://profile/playlists/video/")) {

thumb_loader = NPT_Reference<CThumbLoader>(new CVideoThumbLoader());
}
else if (URIUtils::IsMusicDb(items.GetPath()) ||
StringUtils::StartsWith(items.GetPath(), "special://profile/playlists/music/")) {
StringUtils::StartsWithNoCase(items.GetPath(), "special://profile/playlists/music/")) {

thumb_loader = NPT_Reference<CThumbLoader>(new CMusicThumbLoader());
}
Expand Down
4 changes: 2 additions & 2 deletions xbmc/settings/SettingDependency.cpp
View file
Expand Up @@ -131,12 +131,12 @@ bool CSettingDependencyCondition::setTarget(const std::string &target)
bool CSettingDependencyCondition::setOperator(const std::string &op)
{
size_t length = 0;
if (StringUtils::EndsWith(op, "is"))
if (StringUtils::EndsWithNoCase(op, "is"))
{
m_operator = SettingDependencyOperatorEquals;
length = 2;
}
else if (StringUtils::EndsWith(op, "contains"))
else if (StringUtils::EndsWithNoCase(op, "contains"))
{
m_operator = SettingDependencyOperatorContains;
length = 8;
Expand Down
2 changes: 1 addition & 1 deletion xbmc/settings/SettingsManager.cpp
View file
Expand Up @@ -476,7 +476,7 @@ bool CSettingsManager::GetBool(const std::string &id) const
if (setting == NULL || setting->GetType() != SettingTypeBool)
{
// Backward compatibility (skins use this setting)
if (setting == NULL && StringUtils::EqualsNoCase(id.c_str(), "lookandfeel.enablemouse"))
if (setting == NULL && StringUtils::EqualsNoCase(id, "lookandfeel.enablemouse"))
return GetBool("input.enablemouse");

return false;
Expand Down
4 changes: 2 additions & 2 deletions xbmc/settings/SkinSettings.cpp
View file
Expand Up @@ -175,13 +175,13 @@ void CSkinSettings::Reset()
// clear all the settings and strings from this skin.
for (map<int, CSkinBool>::iterator it = m_bools.begin(); it != m_bools.end(); ++it)
{
if (StringUtils::StartsWith(it->second.name, currentSkin))
if (StringUtils::StartsWithNoCase(it->second.name, currentSkin))
it->second.value = false;
}

for (map<int, CSkinString>::iterator it = m_strings.begin(); it != m_strings.end(); ++it)
{
if (StringUtils::StartsWith(it->second.name, currentSkin))
if (StringUtils::StartsWithNoCase(it->second.name, currentSkin))
it->second.value.clear();
}

Expand Down

0 comments on commit 02fd1ab

Please sign in to comment.