mpogue2 / SquareDesk

Fully-featured music player and sequence designer, designed for square dance callers
10 stars 4 forks source link

Investigate QTableModel/QTableView for performance #769

Open danlyke opened 1 year ago

danlyke commented 1 year ago

We're currently using a MyTableWidget for the song table, a subclass of QTableWidget, and modifying its model, rather than our own model.

We have a number of performance issues, especially on slower machines, but even on modern M1 machines, as in issue #714. I suspect that we can address a lot of these by decoupling the current song list from the current subset of filtered items, and make those a QTableModel.

I have a vague sense that I started on this once before.

danlyke commented 1 year ago

And it looks like I started with songlistmodel.{cpp,h}

mpogue2 commented 1 year ago

I like this idea...

Here's a question for you: could this be prototyped with a quick performance test, before rewriting all the songTable code? I think we're already turning off most of what the songTable internal model is doing to make it slow (the re-sort on every change), so I'm wondering how much time would it be saving to switch to the explicit model. I believe it's potentially a lot, I would just hate to see it only be a few percent after putting in all that work... Or, does it basically require doing most of the work to get it going in the first place, just to test it (and so a performance prototype would not make sense)?

BTW, in #714, I suspect that most of the slowness there is due to the fact that it's an X86 program being emulated by the M1 processor with Rosetta 2. The M1 version of SquareDesk doesn't seem to have this performance problem -- I have about 1200 files with no slowness, but I admit that I have not tested it with 2900 files myself -- I should probably do that, and I do have an MP3 filename generator laying around somewhere for this purpose....

danlyke commented 1 year ago

I think some of it is all the way there, but I also think that separating out the data model gives us some other potential advantages for reloading on disk changes, so it's good to do generally. Just never been high enough priority.

But it's also the sort of cleaning up code that I'm not getting to do in my day job these days, so it's the sort of clean-up that'll feel good. Do holler when you've hit a point with breaking up mainwindow.cpp, because I think this is the sort of change that could be tough to track across files.

mpogue2 commented 1 year ago

Sounds good. I'll move the "split up main window.cpp" task up a bit in priority then. I just did a quick look-see pass a couple days ago, and there's some code that can/should move. I'll also see if I can do a quick test with 3000 files, and see where we are...

danlyke commented 1 year ago

Or move the "split mainwindow.cpp" task down in priority, and be happy with whatever subdivisions I end up doing... 😜 Message ID: @.***>

mpogue2 commented 1 year ago

LOL -- now the pressure is on to get this done like today! :-)

mpogue2 commented 1 year ago

OK, I did some code reorganization, and the details are over at #649 (which I then closed).
You should have a cleaner surgical field in the operating room now... :-)

mpogue2 commented 1 year ago

FYI -- as per my other action item, I just ran a test with about 3300 MP3 files, and there is no appreciable lag on an M1 Air for any songTable operation that I tried.

So, I think that the lag in #714 was probably because he was using the X86 version of SquareDesk on an M1 machine (the X86 version is definitely slower on an M1, but it has the advantage that it will run on both X86 and M1 processors).