Speedup FileItem::IsSamePath

[cg110]

IsSamePath has been found to be expensive if PR #4301 is applied and
used. This is due to the high number of update events generated.

If the user is in the movie view, then the CFileItemList is scanned for every updated item, and the way the code detects it's a matching movie is to use IsSamePath.

By optimizing the IsSamePath code, the stall/overhead of processing the events (depending on collection size) is reduced.

See this post/thread for more details:
http://forum.xbmc.org/showthread.php?tid=184411&pid=1641764#pid1641764

[jmarshallnz]

Thanks. Looks fine. Ideally m_strPath would be just be a CURL anyway, which would make that change redundant, but that can be fixed if/when that change is made anyway.

[jmarshallnz]

On second thought, I don't really like the reversing of those conditionals. The Has_InfoTag() ones are pure safety against Get_InfoTag() returning NULL, and shouldn't be required at all anyway. Plus, I suspect that the cases where this code is run a lot is where you're updating videodb or musicdb items anyway, and the key speedup will be the changes to IsMusicDb(), not the reversing of the conditionals, right?

[cg110]

The first speed change was the order switch. As the Has_InfoTag is dirt cheap to run, being that it's a != NULL test (in the same class). And so avoided the Curl compare Is_Db was doing. But that wasn't enough on it's own.

In theory the string compare is far cheaper than the curl, so could be enough on it's own. In general the order change means that only 2 calls to Is*DB happen, rather than 6 (most cases of the calls recurse, as the object from the FileItemList tends to be in videodb::/ format)

Mix that with a worst case of 300 movie updates, while on the movies list view and so FileItemList::UpdateItem is called for each update, we end up with O(N^2) IsSamePath calls, so 90,000 calls, so the order change means 180,000 vs 540,000 string compares.

I did have a version that checked if both items had a videotag, and compared them using the DbId and FileId, which is a very quick test, but I don't know enough about the tags to be sure that was a correct test in all cases.

[jmarshallnz]

The string compares are fast though, right? The ones that don't hit (which you are eliminating with the reorder) will normally not hit at the first character, so the expense is primarily the function call. Thus, you'd expect a similar speedup just by making the IsMusicDb() code a lot faster than it currently is. Not quite as fast, but I'm guessing that it may not make all that much difference?

The problem with the new order is that it obfuscates what the code is doing. i.e. the test that has to be satisfied is the test for IsMusicDb() - the test for the tag is then done just to be sure the tag is non-NULL before use, which shouldn't be required, but is there for sanity.

And yes, there are better, faster, ways to do the comparison rather than a path compare, by matching on the database id and type (unfortunately id's alone aren't unique - e.g. movies vs tvshows) in the case you have two videodb paths. This could also save a recurse for example. Ofcourse, you still need to check the case you have a db item and a non-db item, whereby a path compare is the way to go.

[cg110]

I've updated the commit/PR to remove the && reordering. It's now just the string compare. (I've squashed it as well to avoid confusion with 2 commits)

For the more optimal testing the id and type would something like this work:
cg110@763a702
(and should it be part of this PR?)

[jmarshallnz]

Thanks - the current change looks fine. Will check your other stuff.

[cg110]

Having benchmarked the 3 versions:

• no change
• switching IsVideoDB from curl to string compare
• making use of videoinfotags.

The basic test was for 346 movies to compare all of them (when a movie isn't in the list)
original: ~30-40ms
string compare: ~10ms
videoinfotags: <1ms

Tested on an i7-920. I expect a pi and other low powered system will see larger gains.

Based on this I've also added to the PR a second commit for optimizing the video info tag case.

[jmarshallnz]

Thanks. jenkins build this please