Bug 9194 - Track::artist expensive; suggest cache result and avoid double calls
: Track::artist expensive; suggest cache result and avoid double calls
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Database
: 7.4.0
: PC Other
: -- normal (vote)
: 7.4.0
Assigned To: Alan Young
: new_schema
Depends on:
Blocks: 9189
  Show dependency treegraph
 
Reported: 2008-08-18 09:50 UTC by Alan Young
Modified: 2009-10-05 14:32 UTC (History)
4 users (show)

See Also:
Category: ---


Attachments
Cache primary artistid in Track column (2.39 KB, patch)
2009-05-06 02:45 UTC, Alan Young
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Young 2008-08-18 09:50:11 UTC
The Slim::Schema::Track::artist() call is reasonably expensive and called very often (for example from _addJiveSong - see bug 9189). It probably makes sense to add a new column to the Track table and either fill this as part of the scan or lazy-evaluate it as part of the call to artist().

Also, Track::artist() is often called in a manner such as:

    	if (blessed($obj->artist) && $obj->artist->name) {

			$track->creator($obj->artist->name);
	}

or

	blessed $track->artist ) ? $track->artist->id : undef

Sometimes there are multiple such calls in the same function. It would probably help to call such object-generating methods only once.

Similar comments apply to Track::album(). For example, 

	blessed $track->artist ? $track->artist->id : undef

could probably be replaced by:

	$track->albumid
Comment 1 Blackketter Dean 2008-08-20 12:14:56 UTC
Alan:  Seems like a reasonable optimization for 7.2.1, or were you thinking this is further out?
Comment 2 Alan Young 2008-08-20 13:19:32 UTC
Sre, it could go in 7.2.1. I would need some help from someone more familiar with the DB stuff.
Comment 3 Jim McAtee 2008-09-05 02:31:14 UTC
Are you thinking of a string containing the formatted list of artist names or a column containing contributor ids?  Either way, it will likely be challenging to maintain database integrity with respect to the contributor_track table.

If this is done (and proves to be useful), then it makes sense to also do it for albums, where retrieving the artist(s) for an album can be an expensive operation that has a lot of impact on performance when browsing albums in the web ui.
Comment 4 Alan Young 2008-09-05 03:02:45 UTC
I'm looking at the following from Schema/Track.pm:

sub artist {
	my $self = shift;

	# Bug 3824 - check for both types, in the case that an ALBUMARTIST was set.
	return $self->contributorsOfType('ARTIST')->single ||
	       $self->contributorsOfType('TRACKARTIST')->single;
}

This only ever returns a single Artist object, somehow designated the "primary" artist for the track. This is a very common query, where various UIs and status functions want "the" artist for a track (you could argue that this, in itself, is wrong but that is a separate issue).

It would seem to be efficient to cache, in the Track table, the resulting artist-id (primary key to Artist table), so that this (expensive) search need be done only once.
Comment 5 Chris Owens 2008-10-02 14:53:12 UTC
This doesn't seem likely to go into 7.2.1.
Comment 6 Alan Young 2009-05-02 07:26:02 UTC
Change 26358 makes the following improvements:
Add Track::artistName() as helper method and use when appropriate.
Use Track->albumid in favour of Track->album->id.
Remove other repeated indirection via Track->artist or Track->album.

Still want to consider adding a column to cache artistid in Track.
Comment 7 Alan Young 2009-05-06 02:45:05 UTC
Created attachment 5197 [details]
Cache primary artistid in Track column

I could not make this work by adding a schema_9_up.sql, so this change would require a wipe and rescan.
It makes a big difference to the speed of Jive playlist queries.
Comment 8 Andy Grundman 2009-05-06 05:26:48 UTC
For 7.4 I think a wipe is fine, as they will have to do a full rescan because of the move to SQLite anyway.
Comment 9 Alan Young 2009-05-15 06:58:11 UTC
Changes 26614 & 26615 implement the cached primary artist concept and some utility accessors.
Comment 10 James Richardson 2009-10-05 14:32:03 UTC
This bug has been marked as fixed in the 7.4.0 release version of SqueezeBox Server!
    * SqueezeCenter: 28672
    * Squeezebox 2 and 3: 130
    * Transporter: 80
    * Receiver: 65
    * Boom: 50
    * Controller: 7790
    * Radio: 7790  

Please see the Release Notes for all the details: http://wiki.slimdevices.com/index.php/Release_Notes

If you haven't already, please download and install the new version from http://www.logitechsqueezebox.com/support/download-squeezebox-server.html

If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.