Rockbox

Tasklist

FS#11723 - Tagcache returns stale numeric (runtime statistics) data

Attached to Project: Rockbox
Opened by sideral (sideral) - Thursday, 04 November 2010, 18:49 GMT
Last edited by Jonathan Gordon (jdgordon) - Tuesday, 23 November 2010, 00:15 GMT
Task Type Bugs
Category Database
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

The problem is that tagcache_update_numeric queues database updates using tagcache.c's command_queue, whereas tagcache_get_numeric consults the database directly without consulting the command_queue. This can lead to lost updates when pending changes have not yet been processed and committed.

I was able to reproduce this problem with my Sansa ClipV2, but not with the simulator, presumably because of a different thread schedule due to my PC's better horsepower.

To reproduce, I have been jumping back and forth between two tracks of a playlist. The following slightly edited logf output shows the buffer and unbuffer events (“be:”, “ube:”) logged by tagtree_buffer_event and tagtree_track_finish_event, respectively, along with the playcount read from and written to the tagcache by these functions:

be:/PODCASTS/foo.mp3
-> 3
[...]
ube:/PODCASTS/foo.mp3
-> 4
[...]
be:/PODCASTS/bar.mp3
-> 4
[...]
ube:/PODCASTS/bar.mp3
-> 5
[...]
be:/PODCASTS/foo.mp3
-> 3
[...]
ube:/PODCASTS/foo.mp3
-> 4
[...]
be:/PODCASTS/bar.mp3
-> 4
[...]
ube:/PODCASTS/bar.mp3
-> 5
[...]

As you can see, tagtree_track_finish_event increments the playcount each time, but the next tagtree_buffer_event for the same track reads the old, stale playcount again.

I have considered three solutions to this problem:

1. Flush the command queue in tagcache_get_numeric with a call to run_command_queue(true).

2. Update the in-memory database synchronously in tagcache_update_numeric, and queue only updates to the on-disk database in the command_queue.

3. Store-to-load forwarding: In tagcache_get_numeric, consult the command queue for uncommitted updates to the tag being read.

Solution 1, while simple, also is the most inefficient one as it delays all tagcache reads until all writes have been committed. Also, I encountered the occasional lockup during background database updates – not quote sure why, though.

I was able to implement Solution 2 after some refactoring. Unfortunately, this solution is incomplete: It depends on the in-memory database, and it can fail if the tag being updated has not been loaded into memory yet.

I ended up with Solution 3 (patch attached). It may not be the shortest solution, but it is the most elegant one, as it does not alter the tagcache's update policy and is complete nonetheless.

(You may wonder why I am so interested in tagcache accuracy (see also  FS#11721  and  FS#11722 ) – after all, this is only statistics data? The answer is that I am working on another use case for the tag cache that requires accuracy: I am using the tagcache as the ultimate resume database, which allows resuming every track at its last-played position, independent of the navigation mechanism used to access the track (no file browser / bookmarking system needed).)
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Tuesday, 23 November 2010, 00:15 GMT
Reason for closing:  Accepted
Additional comments about closing:  r28645.
Comment by Jonathan Gordon (jdgordon) - Tuesday, 16 November 2010, 13:03 GMT
please fix tabs -> 4 spaces and 80 char line widths, otherwise looks good
Comment by sideral (sideral) - Tuesday, 16 November 2010, 21:30 GMT
Thanks for your comment! I've updated the patch as advised.

Loading...