Merge lp://staging/~michihenning/thumbnailer/fix-1531038 into lp://staging/thumbnailer/devel

Proposed by Michi Henning
Status: Merged
Approved by: Michi Henning
Approved revision: 328
Merged at revision: 328
Proposed branch: lp://staging/~michihenning/thumbnailer/fix-1531038
Merge into: lp://staging/thumbnailer/devel
Diff against target: 109 lines (+11/-12)
5 files modified
data/com.canonical.Unity.Thumbnailer.gschema.xml (+1/-1)
include/ratelimiter.h (+2/-2)
man/thumbnailer-settings.5 (+1/-1)
src/ratelimiter.cpp (+5/-6)
tests/settings/settings_test.cpp (+2/-2)
To merge this branch: bzr merge lp://staging/~michihenning/thumbnailer/fix-1531038
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Michi Henning (community) Approve
Review via email: mp+281813@code.staging.launchpad.net

Commit message

Changed default backlog to 10. Change RateLimiter to work on LIFO basis. Fixes lp:1531038

Description of the change

Changed default backlog to 10. This is necessary because, with a backlog of 20, we ended up never cancelling anything because the sliding window around a list view is smaller than this. In turn, this meant that we always ended up having fewer outstanding requests than the limit, so the cancel messages from the list view always arrived after the request had already gone out on the wire.

Changed the RateLimiter to work on a LIFO basis instead of FIFO. By dealing with the most request first, cancellation has a much better chance of actually cancelling something. In addition, this massively improves the user experience when scrolling through a large collection with a cold cache. Previously, when scrolling down quickly for, say, 150 songs) all the requests for songs that were scrolled over and disappeared off the top of the screen had to complete before the thumbnails for visible region (once scrolling stopped) became visible. With this change, we give priority to the most recent request, so the thumbnails for the visible region where scrolling stops fill in quickly.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michi Henning (michihenning) wrote :

Top-approving after lots of testing on the phone.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: