Opened 13 years ago

Closed 13 years ago

#1287 closed patch (fixed)

Performance and clean patches for mythmusic

Reported by: eskil <myth@…> Owned by: Isaac Richards
Priority: minor Milestone: unknown
Component: mythmusic Version:
Severity: medium Keywords:
Cc: Ticket locked: no

Description

Performance and cleanup patches for mythmusic.

The main part here is the performance, based off revision 8915, I added some code (1-perfmeasure.patch) to measure the time and cpu cycles spend on loading the metadata and creating the tree with various tree layout settings, using my db with about 29000 metadata entries.

Here's the "before" results :

  • Tree build of 'directory' took 37865 Mcycles, wall time 23.74s
  • Tree build of 'artist album title' took 13539 Mcycles, wall time 8.49s
  • Tree build of 'splitartist artist album title' took 14606 Mcycles, wall time 9.16s
  • Tree build of 'splitartist1 artist album title' took 16328 Mcycles, wall time 10.24s

After making all the changes that oprofile suggested, these are the new times :

  • Tree build of 'directory' took 9586 Mcycles, wall time 6.01s
  • Tree build of 'artist album title' took 10544 Mcycles, wall time 6.61s
  • Tree build of 'splitartist artist album title' took 10845 Mcycles, wall time 6.80s
  • Tree build of 'splitartist1 artist album title' took 10660 Mcycles, wall time 6.68s

Noticeable is the time for "directory", is about 4 times faster. The other ones are a few seconds faster which is nice and stuff. Since loading the metadata from the db takes about 5 seconds, I declare success.

Since there's a ton of changes, I've split up the patches to make it easier to look through (but the split out patches don't nessecarily apply cleanly, compile or work, they're just for reviewing all the changes).

Attachments (10)

0-changetree.patch (811 bytes) - added by eskil <myth@…> 13 years ago.
allow dynamic change of tree layout
1-perfmeasure.patch (3.6 KB) - added by eskil <myth@…> 13 years ago.
code that measures performance (not in final patch)
2-regexps.patch (725 bytes) - added by eskil <myth@…> 13 years ago.
remove superflous QRegExp instances
3-lessstartdir.patch (7.9 KB) - added by eskil <myth@…> 13 years ago.
always use AllMusic::startdir instead of passing startdir as argument
4-artistalbumtitle.patch (1.6 KB) - added by eskil <myth@…> 13 years ago.
empty artist/album/title becomes "Unknown Artist/Album/Title?", looks better in the tree
5-musiccompare.patch (1.2 KB) - added by eskil <myth@…> 13 years ago.
Metadata and MusicNode? uses locale aware comparisons
6-playcount.patch (1.7 KB) - added by eskil <myth@…> 13 years ago.
initialise playcount, shuts up valgrind
7-lesscode.patch (8.6 KB) - added by eskil <myth@…> 13 years ago.
Remove duplicate code from the old treebuilding
8-trees.patch (24.3 KB) - added by eskil <myth@…> 13 years ago.
move treebuilding to a object and make it fast
treebuilding.patch (36.7 KB) - added by eskil <myth@…> 13 years ago.
all of the above in 1 big patch against svn 8960

Download all attachments as: .zip

Change History (12)

Changed 13 years ago by eskil <myth@…>

Attachment: 0-changetree.patch added

allow dynamic change of tree layout

Changed 13 years ago by eskil <myth@…>

Attachment: 1-perfmeasure.patch added

code that measures performance (not in final patch)

Changed 13 years ago by eskil <myth@…>

Attachment: 2-regexps.patch added

remove superflous QRegExp instances

Changed 13 years ago by eskil <myth@…>

Attachment: 3-lessstartdir.patch added

always use AllMusic::startdir instead of passing startdir as argument

Changed 13 years ago by eskil <myth@…>

Attachment: 4-artistalbumtitle.patch added

empty artist/album/title becomes "Unknown Artist/Album/Title?", looks better in the tree

Changed 13 years ago by eskil <myth@…>

Attachment: 5-musiccompare.patch added

Metadata and MusicNode? uses locale aware comparisons

Changed 13 years ago by eskil <myth@…>

Attachment: 6-playcount.patch added

initialise playcount, shuts up valgrind

Changed 13 years ago by eskil <myth@…>

Attachment: 7-lesscode.patch added

Remove duplicate code from the old treebuilding

Changed 13 years ago by eskil <myth@…>

Attachment: 8-trees.patch added

move treebuilding to a object and make it fast

Changed 13 years ago by eskil <myth@…>

Attachment: treebuilding.patch added

all of the above in 1 big patch against svn 8960

comment:1 Changed 13 years ago by eskil <myth@…>

0-changetree.patch::

Small cleanup to let AllMusic? change it's sorting and resync the db. Needed by the perfmeasure patch.

1-perfmeasure.patch::

Code that measures performance when entering mythmusic, not part of the final patch, but submitted since it might be of interest.

2-regexps.patch::

Remove unnessecary QRegExp objects, this alone shaved off 1-2 seconds of all the tests.

3-lessstartdir.patch::

Don't pass around the startdir since it's already set inside AllMusic?. Some parts used the member variable, other took it as argument, now they all used the member variable.

4-artistalbumtitle.patch::

When loading metadata from the db, if artist/album/title is empty, set it to sensible value ("Unknown Artist/Album/Title?").

5-musiccompare.patch::

Fix Metadata/MusicNode? comparison to use locale aware QString comparisions - is more correct for us folk with crazy alphabets (börkbörkbörk).

6-playcount.patch::

This just fixes initialising playcount, mostly to shut valgrind up.

7-lesscode.patch::

AllMusic?.intoTree and MusicNode?.intoTree were essentially the same, so eliminate one (I picked AllMusic::intoTree for now). So make it build the tree directly in root_node, this also eliminates the need for top_nodes, so they also go out the window. This greatly simplifies the code.

8-trees.patch::

And now that the code is simpler, we can finally move tree building logic out of Metadata (areYouFinished, getField and the splitartist handling) and toss it all in a object whose responsibility is to only treebuilding. During treebuilding, various stuff is cached in memory, ie. with 29000 records, there's a ca. 18mb spike in memory usage, but it's freed after the tree is built.

treebuilding.patch::

All of the above in one big hunking patch. This one was generated against svn revision 8960.

comment:2 Changed 13 years ago by Isaac Richards

Resolution: fixed
Status: newclosed

(In [8983]) eskil's big performance patch for mythmusic. Closes #1287.

Note: See TracTickets for help on using tickets.