Performance hit when editing sorted lists

Bug #1303746 reported by David Planella
22
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Ubuntu Notes app
Fix Released
Critical
Unassigned
qtdeclarative-opensource-src (Ubuntu)
Fix Released
Undecided
Timo Jyrinki
Trusty
Fix Released
Undecided
Timo Jyrinki

Bug Description

[Impact]
Performance regression from saucy in sorted lists, due to lacking a proper way to support layoutChange.

[Test Case]
We first noticed this after the migration to Qt 5.2 and using the Reminders app: in summary, when editing a note the application becomes unresponsive on the phone and the CPU goes to 100% on the desktop (there it's not too noticeable).

[Regression Potential]
No regressions spotted in utopic with the same patch. List views could be the place where also potential regression would be seen most likely.

---
We first noticed this after the migration to Qt 5.2 and using the Reminders app: in summary, when editing a note the application becomes unresponsive on the phone and the CPU goes to 100% on the desktop (there it's not too noticeable).

After a chat with mzanetti, this is related to an upstream patch that should have probably been implemented differently, and it's now reported in the upstream tracker [1].

This will affect every app using a sorted list on the phone

[1] https://bugreports.qt-project.org/browse/QTBUG-37983

Related branches

David Planella (dpm)
Changed in reminders-app:
status: New → Triaged
importance: Undecided → Critical
Zoltan Balogh (bzoltan)
Changed in qtdeclarative-opensource-src (Ubuntu):
assignee: nobody → Zsombor Egri (zsombi)
Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in qtdeclarative-opensource-src (Ubuntu):
status: New → Confirmed
Revision history for this message
David Planella (dpm) wrote :

There's a merge proposal upstream with a fix we'll want to backport:

- https://codereview.qt-project.org/#change,83580

Changed in qtdeclarative-opensource-src (Ubuntu):
assignee: Zsombor Egri (zsombi) → Timo Jyrinki (timo-jyrinki)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package qtdeclarative-opensource-src - 5.2.1-5ubuntu1

---------------
qtdeclarative-opensource-src (5.2.1-5ubuntu1) utopic; urgency=medium

  * Resync with Debian including renamed QML packages (LP: #1313547)
  * Depend conditionally on libgl1 (LP: #1278871)
  * debian/patches/Fix-marking-of-prototype-objects-in-chain.patch
    - Fix a crasher with deleted QQmlCompiledData as suggested by upstream
      (LP: #1304248)
  * debian/patches/QQuickItemView-QQuickPathView-Fix-creation-of-delega.patch:
    - Fix "Carousel doesn't re-render properly after scrolling"
      (LP: #1307578)
  * debian/patches/Implement-proper-support-for-layoutChange-in-QQmlDel.patch:
    - Fix a performance hit when editing sorted lists (LP: #1303746)
  * debian/patches/V4-regalloc-fix-register-spill-choice-under-high-pre.patch:
    - Fix differing JS results on AMD64/i386 (LP: #1312571)
  * Updated symbols
 -- Timo Jyrinki <email address hidden> Mon, 28 Apr 2014 07:35:22 +0000

Changed in qtdeclarative-opensource-src (Ubuntu):
status: Confirmed → Fix Released
Revision history for this message
Niklas Wenzel (nikwen) wrote :

Will this be backported to Ubuntu 14.04? It does not help if it's fixed in Utopic Unicorn while everyone still uses Trusty.

In my opinion it's a quite critical bug because it prevents useful scrolling in an app I'm working on (http://cantata.googlecode.com).

Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote :

The trusty SDK support has moved to PPA https://launchpad.net/~ubuntu-sdk-team/+archive/ppa so that can (and will) be used to offer backports in the future.

Revision history for this message
Niklas Wenzel (nikwen) wrote :

Yes, I've been using that PPA since Saucy. However, my version is still 5.2.1-3ubuntu15. How long do you think will it take until we get a backport?
This bug really keeps us from creating anything productive and releasing a new version.

Revision history for this message
Pat McGowan (pat-mcgowan) wrote :

@Timo seems like this should be an SRU for Qt in 14.04 archive for folks who are not interested in the SDK

Revision history for this message
Niklas Wenzel (nikwen) wrote :

@Pat: Yep, you're right. It does not seem to be related to the SDK. Running my app on my Nexus 4 which has the most recent 14.10 builds works fine but scrolling is broken on my Trusty desktop. Both versions were compiled using the same SDK.
Since Trusty is a LTS release, it wouldn't be great if the app I'm working on does not work properly on it. What's about backporting the fix?

Changed in qtdeclarative-opensource-src (Ubuntu Trusty):
assignee: nobody → Timo Jyrinki (timo-jyrinki)
status: New → In Progress
description: updated
description: updated
Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote :

The 14.04 LTS SRU fix for this is now published and currently resides in the unapproved queue of SRU uploads. After the SRU team has looked at it, it will be copied to proposed updates and at that time this bug will be updated to ask for yet another round of testing.

Revision history for this message
David Planella (dpm) wrote :

Thanks a lot Timo! As discussed this morning, this fix has already landed in a promoted utopic phone image (#2) and on the desktop it's pending the SRU to appear in trusty-updates in the archive.

Revision history for this message
Niklas Wenzel (nikwen) wrote :

Big thanks from me as well. :)

David Planella (dpm)
Changed in reminders-app:
status: Triaged → Fix Committed
Revision history for this message
Chris Halse Rogers (raof) wrote : Please test proposed package

Hello David, or anyone else affected,

Accepted qtdeclarative-opensource-src into trusty-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/qtdeclarative-opensource-src/5.2.1-3ubuntu15.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in qtdeclarative-opensource-src (Ubuntu Trusty):
status: In Progress → Fix Committed
tags: added: verification-needed
Revision history for this message
Chris Halse Rogers (raof) wrote :

I've accepted this into trusty-proposed. I note that a symbol has disappeared from the library, which would be an ABI break, but by my understanding this is in private code so is not a problem.

If my understanding is wrong, please pipe up :)

Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote :

Both of them are private code. The only omission is that in the trusty SRU the latter wasn't marked as such (with the " 1") in the .symbols file unlike in the utopic upload, but that shouldn't hurt.

Revision history for this message
Niklas Wenzel (nikwen) wrote :

This fixed it. Thanks a lot. Hoping that the SRU will roll out soon. :)

Version: 5.2.1-3ubuntu15.1 on amd64

tags: added: verification-done
removed: verification-needed
Revision history for this message
David Planella (dpm) wrote :

I can also confirm the packages in trusty-proposed fix the bug for me.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package qtdeclarative-opensource-src - 5.2.1-3ubuntu15.1

---------------
qtdeclarative-opensource-src (5.2.1-3ubuntu15.1) trusty; urgency=medium

  * debian/patches/QQuickItemView-QQuickPathView-Fix-creation-of-delega.patch:
    - Fix "Carousel doesn't re-render properly after scrolling"
      (LP: #1307578)
  * debian/patches/Implement-proper-support-for-layoutChange-in-QQmlDel.patch:
    - Fix a performance hit when editing sorted lists (LP: #1303746)
  * Update libqt5qml5.symbols (two symbols from the latter patch)
 -- Timo Jyrinki <email address hidden> Fri, 02 May 2014 07:31:28 +0300

Changed in qtdeclarative-opensource-src (Ubuntu Trusty):
status: Fix Committed → Fix Released
Revision history for this message
Scott Kitterman (kitterman) wrote : Update Released

The verification of the Stable Release Update for qtdeclarative-opensource-src has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

David Planella (dpm)
Changed in reminders-app:
status: Fix Committed → Fix Released
Revision history for this message
Niklas Wenzel (nikwen) wrote :

I'm experiencing this issue on my Nexus 4 again (utopic, r91).

Revision history for this message
Niklas Wenzel (nikwen) wrote :

Addition: This package has been updated in that version: http://people.canonical.com/~ogra/touch-image-stats/91.changes

Revision history for this message
David Planella (dpm) wrote :

Niklas, could you please file a separate bug for what you're seeing? It might be that the symptoms are the same, but it's easier to track the causes separately, as the present bug was already marked as fixed, and from what I understand what you're describing now affects only utopic, unlike the present bug, which also affected trusty.

Thanks!

Revision history for this message
David Planella (dpm) wrote :

I've reported the follow-up as separate bug 1334177

Revision history for this message
Niklas Wenzel (nikwen) wrote :

David, thanks for creating a separate bug report. I've been quite busy this week so I couldn't do it earlier.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.