Code review comment for lp:~mhr3/unity-lens-applications/secondary-sorting

Revision history for this message
Michal Hruby (mhr3) wrote :

> kamstrup wrote 4 minutes ago:
> 52 + if (popularities_dirty)
> 53 + yield update_popularities ();
>
> Check for cancellation after here?

8 minutes ago
Make sure we check the cancellable after updating popularities

Received the tachyons ;)

> Should the decrement not be moved to the outermost for-loop? If one event has many apps they should all rank alike, no?

I suppose you're right, at least I don't have to use 256 event limit and magical 512 constant five lines later.

> Can you add a comment explaining the sorting please (also noting the rounding trick)? For the uninitiated this will look like dark magic.

Fixed.

> I was wondering if it would be a significant speed increase if you simply iterated the results GSList but then inserted the hits sorted in a GSequence. I don't know the performance profile of sorting a GSList, but I'd assume it to be weak... But this prolly clashes with code elsewhere expecting the glist..?

I was thinking about this as well, but since there are usually just a few hundred apps (and only a few dozen search hits), the time spent building a sequence and sorting it would probably be higher than just sorting the linked list. (moreover looking at glib's sources it seems to be using mergesort which is O(N logN) therefore a non-issue really).

« Back to merge proposal