Merge lp://staging/~josharenson/unity-scopes-shell/fix-overview-results-sorting into lp://staging/unity-scopes-shell
Status: | Approved |
---|---|
Approved by: | Paweł Stołowski |
Approved revision: | 355 |
Proposed branch: | lp://staging/~josharenson/unity-scopes-shell/fix-overview-results-sorting |
Merge into: | lp://staging/unity-scopes-shell |
Diff against target: |
44 lines (+19/-14) 1 file modified
src/Unity/overviewresults.cpp (+19/-14) |
To merge this branch: | bzr merge lp://staging/~josharenson/unity-scopes-shell/fix-overview-results-sorting |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Andrea Cimitan (community) | Approve | ||
Paweł Stołowski (community) | Approve | ||
Review via email:
|
Commit message
Fix the list sorting of overviewresults
When the results for the model were set, the loop responsible for sorting them
was being executed hundreds of thousands of times. It has been replaced with a
simple insertion sort.
Description of the change
Fix the list sorting of overviewresults
When the results for the model were set, the loop responsible for sorting them
was being executed hundreds of thousands of times. It has been replaced with a
simple insertion sort.
Unmerged revisions
- 356. By Josh Arenson
-
Clean syntax
- 355. By Josh Arenson
-
Fix the list sorting of overviewresults
When the results for the model were set, the loop responsible for sorting them
was being executed hundreds of thousands of times. It has been replaced with a
simple insertion sort.
Looks good to me, thanks!
As discussed on IRC, it's not clear what made the previous implementation so slow (although the fact the 'i' wasn't incremented on every iteration could be part of the problem), but what you propose make sense and is an improvement, the tests are passing and I haven't found any regression with this change, so +1.
One remark only, could you please remove the extra line before we land this:
32 + for (int i = 1; i < m_results.size(); i++) {
33 +