-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LibraryFeature's cannot support drop of multiple tracks. Repeated dropping can stall mixxx. #6105
Comments
Commented by: shetyeakash I found a very efficient way to do bulk inserts on the database, the same approach may be extended to include other DAO functionalities that suffer from huge overhead due to frequent commits. The patch uses transactions and the bulk inserts within the transaction to overcome the need to auto-commit by committing at the end only. |
Commented by: rryan Hi Akash, Thanks for the patch! On the whole it looks good and this will surely speed up drag-and-dropping and adding to crates by a lot. Here are my comments:
For example, in your line where you change the method declaration, you could have written:
See http://doc.qt.nokia.com/4.7-snapshot/implicit-sharing.html for more info about implicit sharing.
|
Commented by: shetyeakash Thank you for the time Ryan, I'll resubmit a patch with the suggested changes. I'll make it a point to comment code, it does help, esp. if you are new to code. I happen to use pretty basic stuff for development and things are getting unmanageable. I am planning this big PC format and Ubuntu re-installation after my midterms. Letting you know that I'll be refining all my contributions and more after 16th of this month. I had no idea that emitting within sql is problematic, thanks for all the info, super-helpful. |
Commented by: shetyeakash Hey guys, I am working on a branch to improve the mass drop n load functionalities of Mixxx including drops from external file managers. The current patch only addresses the performance issue with the 'add to playlist or crate' from the library. I'll put a link of the bzr branch as I develop. Need a little time on this one. |
Commented by: rryan I just merged Akash's branch (thanks Akash!). This is a good first step as it adds APIs for multiple-adds to the DAOs and uses them from WTrackTableView. There is one remaining aspect of this problem which is that LibraryFeature doesn't support dropping of multiple tracks. The DAO APIs are there but the LibraryFeature's can't use them since their drop-callbacks only get a single track. |
Commented by: shetyeakash Hey Ryan, I am working on that, just that the semester is drawing to an end On Fri, May 4, 2012 at 11:11 AM, RJ Ryan wrote:
|
Commented by: kain88-de I've tuned the drag and drop performance of the sidebar model today. Before we didn't check if the source of the DropEvent was mixxx or some other programm. Luckely Qt can tell us which is the case, DropEvent.source() will return 0 if the event does not originate from within mixxx. I used that information and now when ever a drop event occurs inside of mixxx we just look up the trackIds in the database. Writing the patch I noticed that dropAccept(...) and dropAcceptChild(...) are reimplemented in a lot of classes but are doing nothing expect for a few exceptions (mixxxlibraryfeature,cratefeature,autodj,playlists). I would find the code easier to understand if we define these functions in LibraryFeature and only reimplement them if we want the subclass to actually use the function. |
Commented by: rryan Thanks Max! The patch looks good except for the manual escaping of the track locations in the SQL query. Please use the FieldEscaper class instead from src/library/queryutil.h and then just join the QStringList by , |
Commented by: kain88-de Here is the updated patch using the Fieldescaper method |
Commented by: kain88-de Here is a version that also removes function from the features when they are not using them. I like this version better because now it is obvious from libraryfeature.h what is absolutely required to subclass it and what features are optional. Also this way it is easy to see from the *feature.h file what operations are intended to be supported by it. |
Commented by: rryan Awesome -- thanks. Please commit it! |
Issue closed with status Fix Released. |
Reported by: toomuch
Date: 2011-11-13T13:24:47Z
Status: Fix Released
Importance: Medium
Launchpad Issue: lp889825
Tags: library, performance
Attachments: [Patch that fixes this issue.](https://bugs.launchpad.net/bugs/889825/+attachment/2835356/+files/Patch that fixes this issue.), dragdrop.patch, dragdrop_v2.patch, dragdrop_plus.patch
As Mixxx currently cannot filter columns (https://bugs.launchpad.net/mixxx/+bug/675057) I wanted to create crates to access my genres faster. When choosing larger amounts of files it takes forever to finish. In this time, Mixxx is not usable.
Win7 x64, Intel Pentium Dual Core T2390, 3GB RAM (Asus X51L laptop)
Mixxx 1.10.0beta1 x64
======
The reason for this is that LibraryFeature/SidebarModel only support a single drop method for children. dropAcceptChild(QModelIndex, QUrl). The problem with this is that WLibrarySidebar delivers each dropped item one at a time. In the case of toomuch's drop of 1000 tracks to a crate, this generates 1000 SQL queries instead of a single query to add the tracks in bulk.
The text was updated successfully, but these errors were encountered: